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

[Lancelot SIX via Gdb-patches <gdb-patches@sourceware.org>]

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.
> 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.