From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932753AbbCQJIu (ORCPT ); Tue, 17 Mar 2015 05:08:50 -0400 Received: from cantor2.suse.de ([195.135.220.15]:34697 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754087AbbCQJIq (ORCPT ); Tue, 17 Mar 2015 05:08:46 -0400 Date: Tue, 17 Mar 2015 10:08:41 +0100 From: Jan Kara To: Fabian Frederick Cc: linux-kernel@vger.kernel.org, Jan Kara Subject: Re: [PATCH 1/1 linux-next] udf: improve udf_get_filename() error management Message-ID: <20150317090841.GA23155@quack.suse.cz> References: <1426537223-28087-1-git-send-email-fabf@skynet.be> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1426537223-28087-1-git-send-email-fabf@skynet.be> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon 16-03-15 21:20:23, Fabian Frederick wrote: > Let udf_get_filename() return errors instead of 0. > > udf_pc_to_char() now returns error accordingly. > udf_readdir() and udf_find_entry() process is done on > positive result. > > Note that error propagation has still to be done in the > later functions. > > Suggested-by: Jan Kara > Signed-off-by: Fabian Frederick ... > diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c > index b84fee3..4911c1d 100644 > --- a/fs/udf/unicode.c > +++ b/fs/udf/unicode.c > @@ -338,15 +338,17 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen, > uint8_t *dname, int dlen) > { > struct ustr *filename, *unifilename; > - int len = 0; > + int ret = 0; > > filename = kmalloc(sizeof(struct ustr), GFP_NOFS); > if (!filename) > - return 0; > + return -ENOMEM; > > unifilename = kmalloc(sizeof(struct ustr), GFP_NOFS); > - if (!unifilename) > + if (!unifilename) { > + ret = -ENOMEM; > goto out1; > + } > > if (udf_build_ustr_exact(unifilename, sname, slen)) > goto out2; > @@ -367,14 +369,14 @@ int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen, > } else > goto out2; > > - len = udf_translate_to_linux(dname, dlen, > + ret = udf_translate_to_linux(dname, dlen, > filename->u_name, filename->u_len, > unifilename->u_name, unifilename->u_len); > out2: > kfree(unifilename); > out1: > kfree(filename); > - return len; > + return ret; > } I think you need to do more work around udf_get_filename() if you want error handling to function correctly. The test in udf_build_ustr_exact() which can make it fail is just bogus - remove it. udf_CS0toUTF8() and udf_CS0toNLS() may fail as well - make them return error (-EINVAL?) and propagate that up. The else branch in udf_get_filename() should be a BUG() - mount code makes sure one of UDF_FLAG_NLS_MAP and UDF_FLAG_UTF8 is set. That should deal with all the error paths so we really return value < 0 if we fail and positive value if we succeed. Honza -- Jan Kara SUSE Labs, CR