On Thu, Sep 04, 2008 at 05:36:17PM -0400, Chuck Lever wrote:
> On Thu, Sep 4, 2008 at 4:23 PM, J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote:
> > Slightly irrelevant, but same comment as before--wouldn't it be easier
> > to follow the logic if instead of:
> >
> > p = kstrndup(...)
> > if (p) {
> > do stuff for successful case
> > ....
> > return 1;
> > }
> >
> > return 0;
> >
> > it were:
> >
> > p = kstrndup(...)
> > if (!p)
> > return 0;
> > do stuff for successful case
> > ...
> > return 1;
>
> I tried that. It made this patch about 3x larger than it needed to
> be, obscured the changes introduced by the patch, and made it harder
> to show that the new logic is correct. As we are not introducing new
> code here but repairing existing issues, such a clean up would need to
> be in a separate patch.
I agree.
> I generally prefer to use a structured language as it was intended,
> rather than to abuse "goto" as is the trend in the Linux kernel.
That's a well-established practice, not a trend, and in any case I
didn't add a goto.
> By creating named subroutines instead of using lambda functions
I don't see any difference over named subroutines here? Does c even
support lambda functions?
To me the second example is obviously more readable, partly since it
helps clear up the return convention ("oh, we're returning 0 on failure
of kstrndup! 0 must mean failure..."), while the 1st delays one of the
two branches of the if longer than necessary.
But I don't care enough to insist. Take it as an indicator of what'd be
clearer to me for code I have to read a lot, if you like.
--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
