Re: [RFA-v2] ARI fixes: Remove some sprintf calls

[Joel Brobecker <brobecker at adacore dot com>]

> 2012-11-11  Pierre Muller  <muller@sourceware.org>
> 
>         ARI fixes: Avoid sprintf function use rule.
>         * charset.c (convert_between_encodings): Use xsnprintf.
>         * cli-out.c (cli_field_int): Likewise.
>         * cp-namespace.c (cp_lookup_nested_symbol): Likewise.
>         * expprint.c (op_name_standard): Likewise.
>         * frv-tdep.c (set_variant_num_gprs): Likewise.
>         (set_variant_num_fprs): Likewise.
>         * m68hc11-tdep.c (m68hc11_initialize_register_info): Likewise.
>         * nto-tdep.c (nto_find_and_open_solib): Likewise.
>         (nto_init_solib_absolute_prefix): Likewise.
>         * source.c (init_source_path): Likewise.
>         (print_source_lines_base): Likewise.
>         * valprint.c (print_wchar): Likewise.
>         * mi/mi-out.c (mi_field_int): Likewise.
>         windows-nat.c (windows_pid_to_exec_file): Likewise.
>         (windows_create_inferior): Likewise.
>         (_initialize_check_for_gdb_ini): Likewise.

This is a nice improvement.

I'm wondering if there is any way to provoke an error if we ever
use sprintf again... I am asking because I know that it's easy to
ignore the ARI. I kind of remember not being able to do that, but
perhaps wrong memory. #poison, maybe?

A few nits... I think it's good to go, after the trivial nits are
corrected.

Thanks for doing this!

> Index: nto-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/nto-tdep.c,v
> retrieving revision 1.46
> diff -u -p -r1.46 nto-tdep.c
> --- nto-tdep.c	9 Nov 2012 19:58:00 -0000	1.46
> +++ nto-tdep.c	11 Nov 2012 22:42:47 -0000
> @@ -89,7 +89,7 @@ nto_find_and_open_solib (char *solib, un
>    char *buf, *arch_path, *nto_root, *endian;
>    const char *base;
>    const char *arch;
> -  int ret;
> +  int archlen, len, ret;

Can we use "arch_len" instead if "archlen". This would be more in
line with "arch_path", and since both are related... It's also more
in line with the GNU Coding Style, I believe, where we use underscores
to separate words in identifier names.

> +  archlen = strlen (nto_root) + strlen (arch) + strlen (endian) + 2
> +	+ strlen (solib);

Another tiny nit. GCS require that we put the RHS expression inside
parentheses (to help tools format it correctly). Thus:

  arch_len = (strlen (nto_root) + strlen (arch) + strlen (endian) + 2
              + strlen (solib));


>        if (!noerror)
>  	{
> -	  char *name = alloca (strlen (s->filename) + 100);
> -	  sprintf (name, "%d\t%s", line, s->filename);
> +	  int len = strlen (s->filename) + 100;
> +	  char *name = alloca (len);
> +	  xsnprintf (name, len, "%d\t%s", line, s->filename);

Can you add an empty line between variable defs and the rest
of the code?

> +++ windows-nat.c	11 Nov 2012 22:51:49 -0000
> @@ -1895,7 +1895,7 @@ windows_pid_to_exec_file (int pid)
>    /* Try to find exe name as symlink target of /proc/<pid>/exe.  */
>    int nchars;
>    char procexe[sizeof ("/proc/4294967295/exe")];
> -  sprintf (procexe, "/proc/%u/exe", pid);
> +  xsnprintf (procexe, sizeof (procexe), "/proc/%u/exe", pid);

Same here, please?

>  #else
> -      cygallargs = (char *)
> -	alloca (sizeof (" -c 'exec  '") + strlen (exec_file)
> -				    + strlen (allargs) + 2);
> -      sprintf (cygallargs, " -c 'exec %s %s'", exec_file, allargs);
> +      len = sizeof (" -c 'exec  '") + strlen (exec_file)
> +	    + strlen (allargs) + 2;

Can you enclose the RSH expression between parentheses?

-- 
Joel