Re: [PATCH 1/4] gdb: add lookup_cmd_exact to simplify a common pattern

[Luis Machado via Gdb-patches <gdb-patches@sourceware.org>]

On 1/17/21 4:02 PM, Lancelot SIX via Gdb-patches wrote:
> Le Sun, Jan 17, 2021 at 10:47:38AM +0000, Marco Barisione a écrit :
>> On 10 Jan 2021, at 00:06, Lancelot SIX <lsix@lancelotsix.com> wrote:
>>>
>>> Hi
>>>
>>> I just have a few style-related remarks above.
>>>
>>> On Fri, Jan 08, 2021 at 10:07:03AM +0000, Marco Barisione via Gdb-patches wrote:
>>>> +/* See command.h.  */
>>>> +
>>>> +struct cmd_list_element *
>>>> +lookup_cmd_exact (const char *name,
>>>> +		  struct cmd_list_element *list,
>>>> +		  bool ignore_help_classes)
>>>> +{
>>>> +  const char *tem = name;
>>>> +  struct cmd_list_element *cmd = lookup_cmd (&tem, list, "", NULL, -1,
>>>
>>> Probably s/NULL/nullptr/ ?
>>>
>>>> +					     ignore_help_classes);
>>>> +  if (cmd && strcmp (name, cmd->name) != 0)
>>>
>>> I think gdb prefers explicit comparison to check for null pointers:
>>>
>>> https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Comparison_With_NULL_And_Zero
>>>
>>> +  if (cmd != nullptr && strcmp (name, cmd->name) != 0)
>>
>> I tried (but I noticed I didn’t succeed!) to use nullptr and compare
>> with nullptr in new code or code which already looked like that.
>> Otherwise, I tried to stick to the style used in nearby code.
>>
>> That is, if there's code like this:
>>      foo *bar = NULL;
>> When adding a new variable on the next line, I would use NULL for
>> consistency.
>>
>> What is the approach used in GDB?
>> 1. Add new code using the current style even if inconsistent with nearby
>>     code.

You should use the new style. So nullptr should be used here and 
throughout your patch series, even if inconsistent with nearby code.

>> 2. Stick to the style of nearby code.
>> 3. Also update nearby code.
>>
>>
>> -- 
>> Marco Barisione
>>
> 
> My personal approach would be to migrate to nullptr slowly but surely,
> but this is quite personal.  I’ll let a more experienced gdb maintainer
> state what is the prefered way of doing things for the project.
> 
> Independently of that matter, cmd should be compared to something in
> this expression. I guess something like
> 
> +  if (cmd != NULL && strcmp (name, cmd->name) != 0)
> 
> should be ok to maintain consistency.
> 
> Lancelot.
>