[PATCH] osutil.c: one more refactoring

Petr Kodl petrkodl at gmail.com
Thu Sep 11 13:51:11 CDT 2008


>
> Not sure why you decided to pull this out into its own function. I don't
> think it's an improvement. You've now got error handling in two places:
> to return an error here and to catch an error there.


Mostly to cut the indentation and splitting of every other expression into
three lines.

- and the Linux coding guide seems to suggest it is a good idea to keep
function shorted than two pages and # of variables smaller than 5.
It is actually nicely wrapped this way - the function takes
WIN32_FIND_DATAA* and returns one list item or NULL and error.


> Second, the whole goto style only makes sense when there's a stack of
> resources to unroll. Without that, you might as well just return when
> you hit an error


I'm aware of that. It was not meant to be error handling - the goto is just
to keep single exit point - was suggested as a good practice couple threads
before. Kind of make sense for debugging
with single breakpoint capturing every possible function return.

I am resending the patch with the goto inside the make_item removed.

The last two remaining initializations are required - the uninitialized
statobj crashes if optional parameter is missing and the retval has to be
set to zero somehow to indicate an error, so it might as well be initialied.


Petr
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://selenic.com/pipermail/mercurial-devel/attachments/20080911/935034d2/attachment.htm 


More information about the Mercurial-devel mailing list