Re: Probable bug in file run-command.c function clear_child_for_cleanup
- From
Erik Faye-Lund <kusmabite@gmail.com>
- Date
- Sep 10, 2012, 13:58 UTC
- Message-ID
- <CABPQNSaufY9vUAFp9VL2i=6bNmO5_2+anzX6+omfq+UgP03kXg@mail.gmail.com>
- In-Reply-To
- <20120910134406.GA7496@sigill.intra.peff.net>
On Mon, Sep 10, 2012 at 3:44 PM, Jeff King <peff@peff.net> wrote:
Show 20 quoted lines
> On Sun, Sep 09, 2012 at 03:44:54PM +0100, David Gould wrote:
>> You want something like:
>>
>> for (... {
>> if (... {
>> ...
>> }
>> last = &p->next;
>> }
>>
>> or (probably clearer, but I haven't read your coding style guide, if
>> there is one, and some people don't like this approach)
>
> Yes, that's the correct fix. Care to submit a patch?
>
>> for (p = children_to_clean; p; last = &p->next, p = p->next) {
>> ...
>
> That is OK, too, but I think I prefer the first one.
>I feel like bikeshedding a bit today!
I tend to either prefer either the latter or something like this:
while (p) {
...last = p; p = p->next; }
because those approaches put all the iteration logic in the same place. The in-body traversal approach is a bit more explicit about the traversal details.
And to conclude my bikeshedding for the day: Shouldn't "last" ideally be called something like "prev" instead? It's the previously visited element, not the last element in the list.