From: Junio C Hamano <gitster@pobox•com>
To: Robert Stanca <robert.stanca7@gmail•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH 2/2] [GSOC] show_bisect_vars(): Use unsigned int instead of signed int for flags
Date: Sun, 02 Apr 2017 10:14:46 -0700 [thread overview]
Message-ID: <xmqqmvbyspdl.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <CAJYcaSNJ0un1RgM3DNX=EOez5zP=Rko+BUt5SMeyBTb20K21oQ@mail.gmail.com> (Robert Stanca's message of "Sun, 2 Apr 2017 16:18:56 +0300")
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.
next prev parent reply other threads:[~2017-04-02 17:15 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-01 15:30 [PATCH 1/2] [GSOC] Convert signed flags to unsigned Robert Stanca
2017-04-01 15:30 ` [PATCH 2/2] [GSOC] show_bisect_vars(): Use unsigned int instead of signed int for flags Robert Stanca
2017-04-01 19:13 ` Junio C Hamano
2017-04-01 19:19 ` Robert Stanca
2017-04-02 3:30 ` Junio C Hamano
2017-04-02 13:18 ` Robert Stanca
2017-04-02 17:14 ` Junio C Hamano [this message]
2017-04-01 19:12 ` [PATCH 1/2] [GSOC] Convert signed flags to unsigned Junio C Hamano
2017-04-01 19:30 ` Robert Stanca
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqmvbyspdl.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=robert.stanca7@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox