Re: Subtle problems with "info sharedlibrary" on MS-Windows

Simon Marchi via Gdb-patches
2021-04-08T13:57:39Z

On 2021-04-08 3:06 a.m., Eli Zaretskii wrote:
>> Cc: gdb-patches@sourceware.org
>> From: Simon Marchi <simon.marchi@polymtl.ca>
>> Date: Wed, 7 Apr 2021 17:18:08 -0400
>>
>>> OK to commit to master (with a suitable ChangeLog entry)?
>>
>> The patch LGTM, see minor comments below.
>
> Thanks for your prompt review (and for the other help in investigating
> this tricky problem).
>
>> According to our style guideline, we would use
>>
>> dll_name != nullptr
>
> Does this also mean the GDB style prefers, e.g.,
>
> if (load_addr == nullptr)
>
> to
>
> if (!load_addr)
>
> ? Because you didn't comment on those lines, only on those where the
> value is tested for NOT being null.

Exactly, sorry for not being clear. We always want explicit comparison
with nullptr for pointers, whether that is == or !=.

Same for integers that are not used as booleans, we want

if (item_count == 0)

and not

if (!item_count)

This is described here:

https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Comparison_With_nullptr_And_Zero

>>> + if (!(load_addr && mi.lpBaseOfDll != load_addr))
>>
>> Perhaps matter of personal preference, but I would understand it better
>> (less mental steps) as
>>
>> if (!load_addr || mi.lpBaseOfDll == load_addr)
>
> I feel the other way around, but maybe I'm the odd one out here.
> Pedro, Joel: what say you?
>
> In any case, I guess I could add a comment there explaining the logic
> in plain English, so everyone would understand the intent.

That's alright, leave it the way it is. But a comment explaining the
intent is always welcome, in my book.

Thanks,

Simon