On 31 January 2013 16:05, Laszlo Ersek <ler...@redhat.com> wrote:
> On 01/31/13 16:52, Peter Maydell wrote:
>> This looks like the wrong fix to this problem -- if the user passed
>> us a specific name to search for and we found it and it was full, then
>> we definitely want to stop here.
>
> You only skip the children, but not the siblings. When you return NULL
> here, the sibling loop one stack frame higher up continues anyway.

Then that's a bug in the caller -- it should actually stop on
error, not plough ahead. [that is, we need to distinguish
"not found" from "found and it won't work" from "found".]

> This function deserves more intrusive changes, but that's usually
> invitation for more bikeshedding, hence I wanted to avoid it. (For
> example, before commit 1395af6f there was no reason to set "match" to
> zero in two independent "if"s, then check "match" in a third "if".) I
> think that a rewrite from scratch would be justified (and frowned upon).

I think refactorings that make the code make more sense are
entirely reasonable.

-- PMM

Reply via email to