Re: Flash support part 1: memory maps

Eli Zaretskii
2006-07-20T19:32:52Z

> 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?