Avoid "format not a string literal" warnings

  On:

 $ uname -a
 NetBSD gcc70.fsffrance.org 5.1 NetBSD 5.1 (GENERIC) #0: Sat Nov  6 13:19:33 UTC 2010  builds@b6.netbsd.org:/home/builds/ab/netbsd-5-1-RELEASE/amd64/201011061943Z-obj/home/builds/ab/netbsd-5-1-RELEASE/src/sys/arch/amd64/compile/GENERIC amd64

With:

 $ g++ -v
 Using built-in specs.
 Target: x86_64--netbsd
 Configured with: /usr/src/tools/gcc/../../gnu/dist/gcc4/configure --enable-long-long --disable-multilib --enable-threads --disable-symvers --build=x86_64-unknown-netbsd4.99.72 --host=x86_64--netbsd --target=x86_64--netbsd --enable-__cxa_atexit
 Thread model: posix
 gcc version 4.1.3 20080704 prerelease (NetBSD nb2 20081120)

I saw:

 cc1plus: warnings being treated as errors
 ../../src/gdb/ctf.c: In function 'void ctf_save_metadata_header(trace_write_handler*)':
 ../../src/gdb/ctf.c:267: warning: format not a string literal, argument types not checked
 cc1plus: warnings being treated as errors
 ../../src/gdb/cli/cli-cmds.c: In function 'void alias_command(char*, int)':
 ../../src/gdb/cli/cli-cmds.c:1428: warning: format not a string literal and no format arguments
 ../../src/gdb/cli/cli-cmds.c:1457: warning: format not a string literal and no format arguments

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* cli/cli-cmds.c (alias_usage_error): New function.
	(alias_command): Use it.
	* ctf.c (ctf_save_metadata_header): Inline metadata_fmt local in
	ctf_save_write_metadata call.
---
 gdb/cli/cli-cmds.c | 13 ++++++++++---
 gdb/ctf.c          | 42 ++++++++++++++++++++----------------------
 2 files changed, 30 insertions(+), 25 deletions(-)
  

On Wednesday, April 13 2016, Pedro Alves wrote:

> On:
>
>  $ uname -a
>  NetBSD gcc70.fsffrance.org 5.1 NetBSD 5.1 (GENERIC) #0: Sat Nov  6 13:19:33 UTC 2010  builds@b6.netbsd.org:/home/builds/ab/netbsd-5-1-RELEASE/amd64/201011061943Z-obj/home/builds/ab/netbsd-5-1-RELEASE/src/sys/arch/amd64/compile/GENERIC amd64
>
> With:
>
>  $ g++ -v
>  Using built-in specs.
>  Target: x86_64--netbsd
>  Configured with: /usr/src/tools/gcc/../../gnu/dist/gcc4/configure --enable-long-long --disable-multilib --enable-threads --disable-symvers --build=x86_64-unknown-netbsd4.99.72 --host=x86_64--netbsd --target=x86_64--netbsd --enable-__cxa_atexit
>  Thread model: posix
>  gcc version 4.1.3 20080704 prerelease (NetBSD nb2 20081120)
>
> I saw:
>
>  cc1plus: warnings being treated as errors
>  ../../src/gdb/ctf.c: In function 'void ctf_save_metadata_header(trace_write_handler*)':
>  ../../src/gdb/ctf.c:267: warning: format not a string literal, argument types not checked
>  cc1plus: warnings being treated as errors
>  ../../src/gdb/cli/cli-cmds.c: In function 'void alias_command(char*, int)':
>  ../../src/gdb/cli/cli-cmds.c:1428: warning: format not a string literal and no format arguments
>  ../../src/gdb/cli/cli-cmds.c:1457: warning: format not a string literal and no format arguments
>
> gdb/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
>
> 	* cli/cli-cmds.c (alias_usage_error): New function.
> 	(alias_command): Use it.
> 	* ctf.c (ctf_save_metadata_header): Inline metadata_fmt local in
> 	ctf_save_write_metadata call.
> ---
>  gdb/cli/cli-cmds.c | 13 ++++++++++---
>  gdb/ctf.c          | 42 ++++++++++++++++++++----------------------
>  2 files changed, 30 insertions(+), 25 deletions(-)
>
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index ede4909..c60b1d3 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -1410,6 +1410,14 @@ valid_command_p (const char *command)
>    return *command == '\0';
>  }
>  
> +/* Called when "alias" was incorrectly used.  */
> +
> +static void
> +alias_usage_error (void)
> +{
> +  error (_("Usage: alias [-a] [--] ALIAS = COMMAND"));
> +}
> +
>  /* Make an alias of an existing command.  */
>  
>  static void
> @@ -1421,10 +1429,9 @@ alias_command (char *args, int from_tty)
>    char **alias_argv, **command_argv;
>    dyn_string_t alias_dyn_string, command_dyn_string;
>    struct cleanup *cleanup;
> -  static const char usage[] = N_("Usage: alias [-a] [--] ALIAS = COMMAND");
>  
>    if (args == NULL || strchr (args, '=') == NULL)
> -    error (_(usage));
> +    alias_usage_error ();
>  
>    args2 = xstrdup (args);
>    cleanup = make_cleanup (xfree, args2);
> @@ -1453,7 +1460,7 @@ alias_command (char *args, int from_tty)
>  
>    if (alias_argv[0] == NULL || command_argv[0] == NULL
>        || *alias_argv[0] == '\0' || *command_argv[0] == '\0')
> -    error (_(usage));
> +    alias_usage_error ();

I'd call error directly here and above (and yes, replicate the message),
but that is a matter of personal taste.

>  
>    for (i = 0; alias_argv[i] != NULL; ++i)
>      {
> diff --git a/gdb/ctf.c b/gdb/ctf.c
> index 25a4c79..795c365 100644
> --- a/gdb/ctf.c
> +++ b/gdb/ctf.c
> @@ -202,27 +202,6 @@ ctf_save_next_packet (struct trace_write_handler *handler)
>  static void
>  ctf_save_metadata_header (struct trace_write_handler *handler)
>  {
> -  const char metadata_fmt[] =
> -  "\ntrace {\n"
> -  "	major = %u;\n"
> -  "	minor = %u;\n"
> -  "	byte_order = %s;\n"		/* be or le */
> -  "	packet.header := struct {\n"
> -  "		uint32_t magic;\n"
> -  "	};\n"
> -  "};\n"
> -  "\n"
> -  "stream {\n"
> -  "	packet.context := struct {\n"
> -  "		uint32_t content_size;\n"
> -  "		uint32_t packet_size;\n"
> -  "		uint16_t tpnum;\n"
> -  "	};\n"
> -  "	event.header := struct {\n"
> -  "		uint32_t id;\n"
> -  "	};\n"
> -  "};\n";
> -
>    ctf_save_write_metadata (handler, "/* CTF %d.%d */\n",
>  			   CTF_SAVE_MAJOR, CTF_SAVE_MINOR);
>    ctf_save_write_metadata (handler,
> @@ -262,7 +241,26 @@ ctf_save_metadata_header (struct trace_write_handler *handler)
>  #define HOST_ENDIANNESS "le"
>  #endif
>  
> -  ctf_save_write_metadata (handler, metadata_fmt,
> +  ctf_save_write_metadata (handler,
> +			   "\ntrace {\n"
> +			   "	major = %u;\n"
> +			   "	minor = %u;\n"
> +			   "	byte_order = %s;\n"
> +			   "	packet.header := struct {\n"
> +			   "		uint32_t magic;\n"
> +			   "	};\n"
> +			   "};\n"
> +			   "\n"
> +			   "stream {\n"
> +			   "	packet.context := struct {\n"
> +			   "		uint32_t content_size;\n"
> +			   "		uint32_t packet_size;\n"
> +			   "		uint16_t tpnum;\n"
> +			   "	};\n"
> +			   "	event.header := struct {\n"
> +			   "		uint32_t id;\n"
> +			   "	};\n"
> +			   "};\n",
>  			   CTF_SAVE_MAJOR, CTF_SAVE_MINOR,
>  			   HOST_ENDIANNESS);
>    ctf_save_write_metadata (handler, "\n");
> -- 
> 2.5.5

LGTM.
  

On 04/13/2016 09:12 PM, Sergio Durigan Junior wrote:

>>    if (alias_argv[0] == NULL || command_argv[0] == NULL
>>        || *alias_argv[0] == '\0' || *command_argv[0] == '\0')
>> -    error (_(usage));
>> +    alias_usage_error ();
> 
> I'd call error directly here and above (and yes, replicate the message),
> but that is a matter of personal taste.

OOC, is there a reason for that preference?

Instead of replicating the message, I also thought of:

    error (("%s"), usage);

but it seems slightly nicer to me the way I wrote it ( obviously :-) ).

> LGTM.

Thanks,
Pedro Alves
  

On Wednesday, April 13 2016, Pedro Alves wrote:

> On 04/13/2016 09:12 PM, Sergio Durigan Junior wrote:
>
>>>    if (alias_argv[0] == NULL || command_argv[0] == NULL
>>>        || *alias_argv[0] == '\0' || *command_argv[0] == '\0')
>>> -    error (_(usage));
>>> +    alias_usage_error ();
>> 
>> I'd call error directly here and above (and yes, replicate the message),
>> but that is a matter of personal taste.
>
> OOC, is there a reason for that preference?

Just avoiding a one-line-function call, and the fact that the message is
replicated just twice and will likely not change.

> Instead of replicating the message, I also thought of:
>
>     error (("%s"), usage);
>
> but it seems slightly nicer to me the way I wrote it ( obviously :-) ).

For sure.  I'd say go ahead with your way, no reason to bikeshed over
this :-).

Thanks for the patch.

Cheers,
  

On 04/13/2016 10:03 PM, Sergio Durigan Junior wrote:

> For sure.  I'd say go ahead with your way, no reason to bikeshed over
> this :-).

Alright, now pushed.

Thanks,
Pedro Alves