From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751528AbbCOIel (ORCPT ); Sun, 15 Mar 2015 04:34:41 -0400 Received: from mailsec110.isp.belgacom.be ([195.238.20.106]:56873 "EHLO mailsec110.isp.belgacom.be" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751334AbbCOIeg convert rfc822-to-8bit (ORCPT ); Sun, 15 Mar 2015 04:34:36 -0400 X-Cloudmark-SP-Filtered: true X-Cloudmark-SP-Result: v=1.1 cv=9tBgZ/ptsvfV0njmG4nrX7Xc2idz/SQEK7ZEWdRUbK0= c=1 sm=2 a=IkcTkHD0fZMA:10 a=MKtGQD3n3ToA:10 a=1oJP67jkp3AA:10 a=lOwDBDg6bkyudjRKzlUA:9 a=QEXdDO2ut3YA:10 a=QDGPdG6Cl44avU8-:21 a=j088gnYWzb5QtRTC:21 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A2CtDACWQwVV/9QU7sMaAUCDBoEsgwytfQEBAQEBAQaNBYt1AoEVTAEBAQEBAX2EDwEBAQMBIwRSBQsFBAIOCgICGA4CAlcGExEKiAwMkh9YnG6GU5NnAQEBBwIBH4EhhGmFDYQ+MweCaIFFBZs7hX+ECoUpg0cjg289MYJDAQEB Date: Sun, 15 Mar 2015 09:34:35 +0100 (CET) From: Fabian Frederick Reply-To: Fabian Frederick To: Jan Kara Cc: linux-kernel@vger.kernel.org Message-ID: <886970217.268830.1426408475073.open-xchange@webmail.nmp.proximus.be> In-Reply-To: <20150314065222.GD2888@quack.suse.cz> References: <1426020276-13609-1-git-send-email-fabf@skynet.be> <1426020276-13609-4-git-send-email-fabf@skynet.be> <20150314065222.GD2888@quack.suse.cz> Subject: Re: [PATCH 4/5 linux-next] udf: rename udf_get_filename() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT X-Priority: 3 Importance: Medium X-Mailer: Open-Xchange Mailer v7.2.2-Rev27 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > On 14 March 2015 at 07:52 Jan Kara wrote: > > > On Tue 10-03-15 21:44:34, Fabian Frederick wrote: > > udf_readdir(), udf_find_entry() and udf_pc_to_char() use > > udf_get_filename to obtain name length. Give that function > > an appropriate name. >   Hum, have you read what that function does? It actually converts the name > to a different format and returns converted length. So your name is IMHO > more confusing - it's as if sprintf() was called sprintf_length()... Not > applied. > >                                                               Honza Hi Jan, Ok for the name but AFAICS there's still a problem with error management in udf_get_filename(). We return 0 when not able to allocate filename and callsites don't seem to relate the real problem. Maybe we could either BUG_ON(!filename), BUG_ON(!unifilename) directly in udf_get_filename() or return -ENOMEM ?   udf_readdir() could return -ENOMEM instead of 0 but functions like udf_find_entry() would need some updates ... Another solution would be to have length as argument and return error... Regards, Fabian > > > > > Signed-off-by: Fabian Frederick > > --- > >  fs/udf/dir.c     | 3 ++- > >  fs/udf/namei.c   | 3 ++- > >  fs/udf/symlink.c | 7 ++++--- > >  fs/udf/udfdecl.h | 4 ++-- > >  fs/udf/unicode.c | 4 ++-- > >  5 files changed, 12 insertions(+), 9 deletions(-) > > > > diff --git a/fs/udf/dir.c b/fs/udf/dir.c > > index 05e90ed..edf4232 100644 > > --- a/fs/udf/dir.c > > +++ b/fs/udf/dir.c > > @@ -168,7 +168,8 @@ static int udf_readdir(struct file *file, struct > > dir_context *ctx) > >                     continue; > >             } > >  > > -           flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN); > > +           flen = udf_get_filename_length(sb, nameptr, lfi, fname, > > +                                          UDF_NAME_LEN); > >             if (!flen) > >                     continue; > >  > > diff --git a/fs/udf/namei.c b/fs/udf/namei.c > > index 33b246b..189b98b 100644 > > --- a/fs/udf/namei.c > > +++ b/fs/udf/namei.c > > @@ -234,7 +234,8 @@ static struct fileIdentDesc *udf_find_entry(struct inode > > *dir, > >             if (!lfi) > >                     continue; > >  > > -           flen = udf_get_filename(sb, nameptr, lfi, fname, UDF_NAME_LEN); > > +           flen = udf_get_filename_length(sb, nameptr, lfi, fname, > > +                                          UDF_NAME_LEN); > >             if (flen && udf_match(flen, fname, child->len, child->name)) > >                     goto out_ok; > >     } > > diff --git a/fs/udf/symlink.c b/fs/udf/symlink.c > > index ac10ca9..d986916a 100644 > > --- a/fs/udf/symlink.c > > +++ b/fs/udf/symlink.c > > @@ -80,9 +80,10 @@ static int udf_pc_to_char(struct super_block *sb, > > unsigned char *from, > >                     elen += pc->lengthComponentIdent; > >                     if (elen > fromlen) > >                             return -EIO; > > -                   comp_len = udf_get_filename(sb, pc->componentIdent, > > -                                               pc->lengthComponentIdent, > > -                                               p, tolen); > > +                   comp_len = udf_get_filename_length(sb, > > +                                           pc->componentIdent, > > +                                           pc->lengthComponentIdent, > > +                                           p, tolen); > >                     p += comp_len; > >                     tolen -= comp_len; > >                     if (tolen == 0) > > diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h > > index 47bb3f5..70dc260 100644 > > --- a/fs/udf/udfdecl.h > > +++ b/fs/udf/udfdecl.h > > @@ -211,8 +211,8 @@ udf_get_lb_pblock(struct super_block *sb, struct > > kernel_lb_addr *loc, > >  } > >  > >  /* unicode.c */ > > -extern int udf_get_filename(struct super_block *, uint8_t *, int, uint8_t > > *, > > -                       int); > > +extern int udf_get_filename_length(struct super_block *, uint8_t *, int, > > +                              uint8_t *, int); > >  extern int udf_put_filename(struct super_block *, const uint8_t *, uint8_t > >*, > >                         int); > >  extern int udf_build_ustr(struct ustr *, dstring *, int); > > diff --git a/fs/udf/unicode.c b/fs/udf/unicode.c > > index b84fee3..209c0c7 100644 > > --- a/fs/udf/unicode.c > > +++ b/fs/udf/unicode.c > > @@ -334,8 +334,8 @@ try_again: > >     return u_len + 1; > >  } > >  > > -int udf_get_filename(struct super_block *sb, uint8_t *sname, int slen, > > -                uint8_t *dname, int dlen) > > +int udf_get_filename_length(struct super_block *sb, uint8_t *sname, int > > slen, > > +                       uint8_t *dname, int dlen) > >  { > >     struct ustr *filename, *unifilename; > >     int len = 0; > > -- > > 1.9.1 > > > -- > Jan Kara > SUSE Labs, CR