Re: Flash support part 1: memory maps

[Eli Zaretskii <eliz at gnu dot org>]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Thu, 20 Jul 2006 13:41:33 +0400
> 
> this patch is part of my work to add flash memory programming support to gdb. 

Thanks; my comments below.  (Note that I'm not the responsible
maintainer for this are of GDB, though.)

> +/* Parse a field VALSTR that we expect to contain an integer value.
> +   The integer is returned in *VALP.
> +   The string is parsed with the strtoul rountine.
> +
> +   Returns 0 for success, -1 for error.  */
> +static int
> +xml_parse_unsigned_integer (const char *valstr, unsigned long *valp)

Why is this (and other xml_* functions) here?  They seem to be pretty
much unrelated to memory-map.c, and I'd guess that other features that
use XML will want them.  How about a separate file, say gdb-xml.c or
something?

> +  /* Expat interface does not guarantee that a single call to
> +     a handler will be made. Actually, one call for each line
> +     will be made, and character data can possibly span several
> +     lines.
> +
> +     Take care to realloc the data if needed.
> +  */  

This style of comments is not the one prescribed by the GNU coding
standards.

> +  if (!data->character_data)
> +    data->character_data = (char *)malloc (len + 1);
> +  else
> +    {
> +      current_size = strlen (data->character_data);
> +      data->character_data = (char *)realloc (data->character_data,
> +                                              current_size + len + 1);

Why do we need to cast the results of malloc and realloc?