This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 4/6] Fetch and store GP registers by PTRACE_{G,S}ETREGSET


Doug Evans <dje@google.com> writes:

> The == 1 in this test hinders readability (to me anyway).
> [It occurs here and in 5/6, 6/6.]
> The name suggests the variable is a boolean, so I'm
> left wondering "Can it have values other than 0/1,
> and is the else clause correct for those other values?"
>
> Digging deeper the reader would find the variable is tri-state,
> but the initial -1 value should never be seen here (at least
> that's the intuitive choice).

Yes, this variable have three states, uninitialised (-1), true (1)
and false (0) and that is reason I check "have_ptrace_getregset == 1"
instead of "have_ptrace_getregset".

>
> If one wanted to add an assert that the value is not -1 here
> that'd be ok, though one could also argue it's overkill.
> I don't have a preference either way.
>
> But I suggest removing the "== 1" in the test.

I am OK to remove "== 1" from patches #4, #5 and #6.

-- 
Yao (éå)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]