Jeff King <peff@xxxxxxxx> writes: >> I agree that the patch as posted does not help but if this is >> originally an assertion, then it should never trigger in real life, >> so BUG() would be more appropriate than an error return, no? > > My thinking was that it doesn't make much sense as an assertion in the > first place. It is not a side effect of "let's make sure things are as > we expect while we're doing some other operation". The whole point of > the function is: is this data structure properly in order. Very true. Ah, so you mean the way this function is supposed to be used is to _call_ it, like so: if (!is_our_data_structure_healthy()) BUG(...); It makes it easier to reason about what the function is doing, I guess. > But I guess you could argue that calling the function is itself a form > of assertion. I don't really care that much either way, so whatever > Han-Wen prefers is fine with me (but I do think it is worth addressing > the warning Carlo found _somehow_). > > -Peff