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

Pedro Alves
2017-09-21T13:47:02Z

On 09/21/2017 02:06 PM, Metzger, Markus T wrote:
>> From: Pedro Alves [mailto:palves@redhat.com]
>
> Hello Pedro,

Hi Markus!

> My reason is simply to not mix different styles.
>

Alright, that's certainly a valid reason.

My view is that declarations in the middle of blocks are
desirable, and enforcing top-of-block-only consistency too
strongly prevents incremental modernization a bit, since it
puts the bar higher (because either everything is converted,
or else nothing is). So personally I'm fine with mixed style,
and I try to push for declare-at-initialization when it
makes sense.

>
>> It's really up to you the style to use for this code as
>> btrace maintainer
>
> That shouldn't be the case. If GDB is moving from a separate
> declaration block to mixed-in declarations we should do it
> everywhere.

Ack. To me, if we need a simpler rationale, it could go
like this: middle-of-block declaration+initialization makes
sense with non-trivial types. And if we accept middle-of-block
declarations with non-trivial types, then there's no good
rationale for not allowing them with scalar types too.

>
> So please ignore my comments on declaration placement.

On the contrary, it's not my intention to make your comment
be ignored, rather that we all discuss this and end up on
the same page. So thanks for the discussion, and really sorry
if I sounded too nit picky.

-----

BTW, looking at the get_context_size function in question, it
seems like the 'number' variable is not used, meaning we could
apply something like this:

static int
get_context_size (char **arg)
{
- char *pos;
- int number;
-
- pos = skip_spaces (*arg);
+ char *pos = skip_spaces (*arg);

if (!isdigit (*pos))
error (_("Expected positive number, got: %s."), pos);

return strtol (pos, arg, 10);
}

Thanks,
Pedro Alves