Re: [RFC] Ada ARI fix for sprintf

Pedro Alves
2009-04-11T15:19:29Z

On Tuesday 07 April 2009 09:09:27, Pierre Muller wrote:
> The reason this is a RFC rather than a RFA
> is that I used another function than
> the one recommended on ARI page.

> Currently AR Index gives 153 failures for "sprint" rule.
> sprintf 153 Do not use sprintf, instead use xstrprintf
> But they seem to all have local static buffers
> whereas xstrprintf allocates memory globally.

Well, you don't have to follow those rules literally.
The important point is in not using a function that
can overflow, like sprintf.

> * ada-exp.y (convert_char_literal): Replace sprintf by xsnprintf.
> * ada-lang.c (add_angle_bracket): Ditto.
^ typo, add_angle_brackets.

> (ada_decode, find_old_style_renaming_symbol): Ditto.
> (ada_to_fixed_type_1, ada_enum_name): Ditto.
>

> Index: ada-lang.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/ada-lang.c,v
> retrieving revision 1.205
> diff -u -p -r1.205 ada-lang.c
> --- ada-lang.c 24 Mar 2009 02:07:06 -0000 1.205
> +++ ada-lang.c 7 Apr 2009 08:01:28 -0000
> @@ -335,11 +335,13 @@ static char *
> add_angle_brackets (const char *str)
> {
> static char *result = NULL;
> + int result_size;
>
> xfree (result);
> - result = (char *) xmalloc ((strlen (str) + 3) * sizeof (char));
> + result_size = (strlen (str) + 3) * sizeof (char);
> + result = (char *) xmalloc (result_size);
>
> - sprintf (result, "<%s>", str);
> + xsnprintf (result, result_size, "<%s>", str);
> return result;
> }

This function is already allocating the buffer from the heap, so,
if you used xstrprintf, the code would become simpler, as you
would get rid of the xmalloc, and the need to compute result_size
yourself. Something like:

static char *
add_angle_brackets (const char *str)
{
static char *result = NULL;

xfree (result);
result = xstrprintf ("<%s>", str);
return result;
}

Although, this function is used in the symbol completer, and
I don't know if there's a speed difference in using
xstrprintf over xmalloc+snprintf, and if it makes a difference
in this case. Probably not somethind to worry about
though... Joel?

All other cases looked good to me.

--
Pedro Alves