On Thu, Mar 06, 2014 at 02:47:15PM -0500, Theodore Ts'o wrote:
> On Thu, Mar 06, 2014 at 11:06:22AM -0800, Darrick J. Wong wrote:
> > I noticed a few things while merging -next into my dev tree...
>
> Darrick, good catch!
>
> > > + retval = ext2fs_new_inode(current_fs, cwd, 010755, 0, &newfile);
> > > + if (retval) {
> > > + com_err(__func__, retval, 0);
> > > + close(fd);
> > > + return errno;
> >
> > Why return errno here, instead of retval?
>
> Yep, we should do s/errno/retval/ at the places that you noted.
>
> Robert, since these are in next already, could you submit a patch to
> fixt this up?
I'm already queuing up a bunch of (more) fixes... there's more weird things I
didn't notice. Such as, why is current_fs now defined in current_inode.h?
That really ought to have stayed in debugfs.c, and current_inode.h should have
'extern ext2_filsys current_fs;', no?
Also there's a no-return-value 'return;' if the inline data write fails later
down in that function, and I think it's time to pull in cppcheck on a make
C=1 build.
...I'll also respin the patchset I sent out a few days ago.
--D
>
> Thanks!!
>
> - Ted
>
> P.S. Fortunately, this isn't crypto code ala gnutls. :-)
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
