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

From: Junio C Hamano <[email-masked]>
Date: Sun, 02 Apr 2017 10:14:46 -0700

Robert Stanca <[email-masked]> 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.