On 31.08.2007 [13:43:18 +0200], Richard Knutsson wrote:
> Andi Drebes wrote:
> >This patch removes a variable from fs/cramfs/inode.c that is just used to
> >store
> >a return value which is immediately read afterwards.
> >
> >Tested on an i386 box.
> >Signed-off-by: Andi Drebes <lists-receive@xxxxxxxxxxxxxxxxxxx>
> >---
> >diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c
> >index 350680f..42d2cf8 100644
> >--- a/fs/cramfs/inode.c
> >+++ b/fs/cramfs/inode.c
> >@@ -372,7 +372,7 @@ static int cramfs_readdir(struct file *filp, void
> >*dirent, filldir_t filldir)
> > char *name;
> > ino_t ino;
> > mode_t mode;
> >- int namelen, error;
> >+ int namelen;
> >
> > mutex_lock(&read_mutex);
> > de = cramfs_read(sb, OFFSET(inode) + offset,
> > sizeof(*de)+CRAMFS_MAXPATHLEN);
> >@@ -398,8 +398,7 @@ static int cramfs_readdir(struct file *filp, void
> >*dirent, filldir_t filldir)
> > break;
> > namelen--;
> > }
> >- error = filldir(dirent, buf, namelen, offset, ino, mode >>
> >12);
> >- if (error)
> >+ if(filldir(dirent, buf, namelen, offset, ino, mode >> 12))
> >
> Maybe picky but please leave it as "if (".
Beyond that, I just don't like this change. I thought the preferred way
for these types of statements was the previous version. That is:
error = filldir(...);
if (error)
error stuff
Rather than a conditional with potential side-effects?
Thanks,
Nish
--
Nishanth Aravamudan <nacc@xxxxxxxxxx>
IBM Linux Technology Center
-
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
