Re: [RFA 49/67] Constify some commands in btrace.c

[Pedro Alves <palves@redhat.com>]

On 09/21/2017 08:02 AM, Metzger, Markus T wrote:

>> @@ -3213,13 +3214,16 @@ get_context_size (char **arg)
>>    if (!isdigit (*pos))
>>      error (_("Expected positive number, got: %s."), pos);
>>
>> -  return strtol (pos, arg, 10);
>> +  char *end;
>> +  long result = strtol (pos, &end, 10);
>> +  *arg = end;
>> +  return result;
>>  }
> 
> The rest of the declarations are at the beginning.  I'd prefer to keep it that way.

Interesting.  Out of curiosity, is there a particular reason for
that preference?  I ask because we haven't adjusted our guidelines
in this area yet, but I've been asking people to move
declarations nearer where they're initialized in patch reviews.
My reasoning for that has been that IMHO, declarations nearer to
where they're initialized help with a few things:

 - helps with reasoning about the code, since the type of
   the variable has more changes of being visible.  The
   top of the scope may be a good number of lines above.

 - helps prevent accidental used-uninitialized bugs.

 - avoid gratuitous pessimization when the variable is of
   non-trivial type, by avoiding separate default
   construction + assignment steps.  (though that's not the
   case here, since in this case all variables are scalars).

The point that I feel particularly strongly about is the
last one, about non-trivial types, though as said that
doesn't apply in this case.

For the first two points, in C89, we'd some times open a
extra scope, like in this case you could write:

    {
      char *end;
      long result = strtol (pos, &end, 10);

      *arg = end;
      return result;
    }

In C99/C++, we can write declarations in the middle of
blocks without forcing the extra scope.

It's really up to you the style to use for this code as
btrace maintainer, but I hope that at least this clarifies
things for those that end up being asked different
things from different people.  :-)

Thanks,
Pedro Alves