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

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

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