Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: "Metzger, Markus T" <markus.t.metzger@intel.com>,
	Tom Tromey <tom@tromey.com>,
	"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: [RFA 49/67] Constify some commands in btrace.c
Date: Thu, 21 Sep 2017 13:47:00 -0000	[thread overview]
Message-ID: <83de5929-b2f2-fbd5-effd-740c21cd2a98@redhat.com> (raw)
In-Reply-To: <A78C989F6D9628469189715575E55B2369607020@IRSMSX104.ger.corp.intel.com>

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


  reply	other threads:[~2017-09-21 13:47 UTC|newest]

Thread overview: 110+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-21  5:16 [RFA 00/67] Simple add_cmd constification Tom Tromey
2017-09-21  5:11 ` [RFA 05/67] Constify two functions in valprint.c Tom Tromey
2017-09-21  5:13 ` [RFA 09/67] Constify display_tib Tom Tromey
2017-09-21  5:13 ` [RFA 16/67] Constify some functions in memattr.c Tom Tromey
2017-09-21  5:13 ` [RFA 12/67] Constify maintenance_cplus_namespace Tom Tromey
2017-09-21  5:13 ` [RFA 07/67] Constify two functions in cp-abi.c Tom Tromey
2017-09-21  5:13 ` [RFA 03/67] Constify not_just_help_class_command Tom Tromey
2017-09-21  5:13 ` [RFA 25/67] Constify some commands in symfile.c Tom Tromey
2017-09-21  5:13 ` [RFA 18/67] Constify interpreter_exec_cmd Tom Tromey
2017-09-21  5:13 ` [RFA 10/67] Constify some functions in psymtab.c Tom Tromey
2017-09-21  5:13 ` [RFA 04/67] Constify info_probes_dtrace_command Tom Tromey
2017-09-21 12:48   ` Sergio Durigan Junior
2017-09-21  5:13 ` [RFA 28/67] Constify some commands in record-btrace.c Tom Tromey
2017-09-21  7:02   ` Metzger, Markus T
2017-09-21  5:13 ` [RFA 14/67] Constify core_file_command Tom Tromey
2017-09-21  5:15 ` [RFA 20/67] Constify some commands in cli-cmds.c Tom Tromey
2017-09-21  5:15 ` [RFA 15/67] Constify show_convenience Tom Tromey
2017-09-21  5:15 ` [RFA 21/67] Constify commands in cli-dump.c Tom Tromey
2017-09-21  5:16 ` [RFA 22/67] Constify user_defined_command Tom Tromey
2017-09-21  5:16 ` [RFA 02/67] Constify add_cmd gdb_bfd.c Tom Tromey
2017-09-21  5:16 ` [RFA 11/67] Constify first_component_command Tom Tromey
2017-09-21  5:16 ` [RFA 01/67] Add add_cmd function overloads Tom Tromey
2017-09-21  7:02   ` Metzger, Markus T
2017-09-21 10:25     ` Pedro Alves
2017-09-23  4:14       ` Tom Tromey
2017-09-23  4:14     ` Tom Tromey
2017-09-26 12:11       ` Pedro Alves
2017-09-27 14:44         ` Tom Tromey
2017-09-21 10:25   ` Pedro Alves
2017-09-23  4:18     ` Tom Tromey
2017-09-23  4:30     ` Tom Tromey
2017-09-23  4:49       ` Tom Tromey
2017-09-26 12:12         ` Pedro Alves
2017-09-27 14:42           ` Tom Tromey
2017-09-21  5:16 ` [RFA 26/67] Constify new_ui_command Tom Tromey
2017-09-21  5:17 ` [RFA 08/67] Constify two functions in linux-fork.c Tom Tromey
2017-09-21  5:17 ` [RFA 27/67] Constify some commands in symmisc.c Tom Tromey
2017-09-21  5:17 ` [RFA 29/67] Constify some commands in skip.c Tom Tromey
2017-09-21  5:17 ` [RFA 23/67] Constify some commands in cli-logging.c Tom Tromey
2017-09-21  5:17 ` [RFA 17/67] Constify cmd_record_full_restore Tom Tromey
2017-09-21  7:02   ` Metzger, Markus T
2017-09-21  5:17 ` [RFA 13/67] Constify maintenance_print_user_registers Tom Tromey
2017-09-21  5:17 ` [RFA 06/67] Constify dump_arc_instruction_command Tom Tromey
2017-09-21  5:31 ` [RFA 24/67] Constify some commands in spu-tdep.c Tom Tromey
2017-09-21  5:31 ` [RFA 19/67] Constify maintenance_print_target_stack Tom Tromey
2017-09-21  5:39 ` [RFA 52/67] Constify save_gdb_index_command Tom Tromey
2017-09-21  5:39 ` [RFA 33/67] Constify some commands in target-descriptions.c Tom Tromey
2017-09-21  5:39 ` [RFA 50/67] Constify unset_exec_wrapper_command Tom Tromey
2017-09-21  5:39 ` [RFA 32/67] Constify maintenance_print_dummy_frames Tom Tromey
2017-09-21  5:39 ` [RFA 31/67] Constify some commands in tui.c Tom Tromey
2017-09-21  5:39 ` [RFA 51/67] Constify info_probes_stap_command Tom Tromey
2017-09-21 12:48   ` Sergio Durigan Junior
2017-09-21  5:39 ` [RFA 48/67] Constify delete_bookmark_command Tom Tromey
2017-09-21  5:39 ` [RFA 49/67] Constify some commands in btrace.c Tom Tromey
2017-09-21  7:02   ` Metzger, Markus T
2017-09-21 11:11     ` Pedro Alves
2017-09-21 13:06       ` Metzger, Markus T
2017-09-21 13:47         ` Pedro Alves [this message]
2017-09-21 16:53           ` Metzger, Markus T
2017-09-23  4:28           ` Tom Tromey
2017-09-26  8:15             ` Metzger, Markus T
2017-09-21  5:40 ` [RFA 54/67] Constify some commands in compile.c Tom Tromey
2017-09-21  5:40 ` [RFA 46/67] Constify some commands in mips-tdep.c Tom Tromey
2017-09-21  5:40 ` [RFA 63/67] Constify some commands in regcache.c Tom Tromey
2017-09-21  5:40 ` [RFA 47/67] Constify some commands in remote.c Tom Tromey
2017-09-21  5:40 ` [RFA 37/67] Constify some commands in record.c Tom Tromey
2017-09-21  7:02   ` Metzger, Markus T
2017-09-23  4:05     ` Tom Tromey
2017-09-21 10:00   ` Pedro Alves
2017-09-23  4:05     ` Tom Tromey
2017-09-23  4:29     ` Tom Tromey
2017-09-21  5:40 ` [RFA 38/67] Constify some linespec functions Tom Tromey
2017-09-21 10:04   ` Pedro Alves
2017-09-23  4:03     ` Tom Tromey
2017-09-26 12:17       ` Pedro Alves
2017-09-21  5:40 ` [RFA 65/67] Constify some commands in symtab.c Tom Tromey
2017-09-21  5:40 ` [RFA 34/67] Constify unwind_command Tom Tromey
2017-09-21  5:40 ` [RFA 66/67] Constify some commands in ada-tasks.c Tom Tromey
2017-09-21  5:40 ` [RFA 44/67] Constify some commands in thread.c Tom Tromey
2017-09-21  5:40 ` [RFA 53/67] Constify maintenance_print_reggroups Tom Tromey
2017-09-21  5:40 ` [RFA 67/67] Constify find_command Tom Tromey
2017-09-21  5:40 ` [RFA 62/67] Constify some commands in printcmd.c Tom Tromey
2017-09-21  5:40 ` [RFA 56/67] Constify demangle_command Tom Tromey
2017-09-21  5:40 ` [RFA 64/67] Constify some commands in inferior.c Tom Tromey
2017-09-21  5:40 ` [RFA 35/67] Constify commands maint.c, plus maintenance_print_type Tom Tromey
2017-09-21 10:06   ` Pedro Alves
2017-09-23  4:32     ` Tom Tromey
2017-09-21  5:40 ` [RFA 55/67] Constify maintenance_info_program_spaces_command Tom Tromey
2017-09-21  5:40 ` [RFA 45/67] Constify cd_command Tom Tromey
2017-09-21 12:48   ` Sergio Durigan Junior
2017-09-21  5:40 ` [RFA 36/67] Constify some commands in source.c Tom Tromey
2017-09-21  5:41 ` [RFA 58/67] Constify some commands in i386-tdep.c Tom Tromey
2017-09-21  5:41 ` [RFA 60/67] Constify some commands in macrocmd.c Tom Tromey
2017-09-21  5:41 ` [RFA 59/67] Constify some commands in infcmd.c Tom Tromey
2017-09-21  9:56   ` Pedro Alves
2017-09-21  5:41 ` [RFA 43/67] Constify some commands in probes.c Tom Tromey
2017-09-21 12:47   ` Sergio Durigan Junior
2017-09-21  5:41 ` [RFA 41/67] Constify some commands in remote-fileio.c Tom Tromey
2017-09-21  5:41 ` [RFA 39/67] Constify some commands in ax-gdb.c Tom Tromey
2017-09-21  5:42 ` [RFA 42/67] Constify some commands in exec.c, plus symbol_file_command Tom Tromey
2017-09-21  5:42 ` [RFA 40/67] Constify some commands in tracepoint.c Tom Tromey
2017-09-21 10:03   ` Pedro Alves
2017-09-23  4:34     ` Tom Tromey
2017-09-21  5:42 ` [RFA 30/67] Constify tui_reg_command Tom Tromey
2017-09-21  5:42 ` [RFA 61/67] Constify some commands in breakpoint.c Tom Tromey
2017-09-21  9:56   ` Pedro Alves
2017-09-21 13:20     ` Patrick Monnerat
2017-09-21  5:42 ` [RFA 57/67] Constify add_symbol_file_from_memory_command Tom Tromey
2017-09-21 10:26 ` [RFA 00/67] Simple add_cmd constification Pedro Alves
2017-09-26 12:19   ` Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83de5929-b2f2-fbd5-effd-740c21cd2a98@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=markus.t.metzger@intel.com \
    --cc=tom@tromey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox