Re: [PATCH 2/2] [GSOC] show_bisect_vars(): Use unsigned int instead of signed int for flags

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 2, 2017, 17:14 UTC
Message-ID: <xmqqmvbyspdl.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAJYcaSNJ0un1RgM3DNX=EOez5zP=Rko+BUt5SMeyBTb20K21oQ@mail.gmail.com>

Robert Stanca <robert.stanca7@gmail.com> writes:

> The question is : If the flags field of the structure is used in
> function calls should i update flags that deep?(there are other
> cases where the field is used in nested calls )

[administrivia: please don't top-post around here].

There won't be any fast and clear rule and you'd need to grow and use common sense and good taste, but it probably is helpful to go back and think from the first principle, i.e. why you are doing this conversion.

For example, in your PATCH 2/2 that we are discussing, you updated the local variable "flags" to unsigned in show_bisect_vars(), because it receives the value from "info->flags", which is becoming an "unsigned" because it is a collection of independent bits.

The function uses this "flags" (now unsigned) twice, and one is to pass it to filter_skipped() as its 3rd parameter. This helper function takes "int", but you didn't update it to "unsigned". And you made the right decision to stop there.

The reason why it is the right place to stop is because the function does not use its 3rd parameter as a collection of bits; it wants its callers to give Yes/No there--anything non-zero is yes. Because you know "flags & BISECT_SHOW_ALL", which is unsigned, would be passed as a non-zero "int" to filter_skipped(), iff flags has that bit set, you know you do not have to touch that function and you stop there.

There will be similar places in the callchain that stop propagating the "collection-of-independent-bits"-ness. And that is where you would stop--because beyond that point there is no "arrgh, we use signed int to represent a collection of bits?" problem, which is what you are cleaning up.