This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: Flash support part 1: memory maps
On Thursday 20 July 2006 23:32, Eli Zaretskii wrote:
> > 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?
That sound like a good idea; I just did not want to create too many files when
there's just one user of those functions.
> > + /* 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.
Sorry, will fix.
> > + 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?
Ah, yea, we don't need this in C.
- Volodya