This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [RFA] Factor out some functions in remote.c
- To: Michael Snyder <msnyder at cygnus dot com>
- Subject: Re: [RFA] Factor out some functions in remote.c
- From: Andrew Cagney <ac131313 at cygnus dot com>
- Date: Thu, 10 May 2001 00:53:22 -0400
- Cc: gdb-patches at sources dot redhat dot com
- References: <3AF07FD4.62C23D84@cygnus.com>
> These are two little functions that factor out the conversion between
> ascii-encoded hex and binary, and vice versa, something that seems to
> be done inline over and over in remote.c (and something that I was
> about to do inline yet another time).
Mostly ok. Just some tweeks.
> + /* exported functions */
> +
this comment can be deleted.
> --- 1731,1738 ----
> getpkt (bufp, PBUFSIZ, 0);
> if (bufp[0] != 0)
> {
> ! n = min (strlen (bufp) / 2, sizeof (display_buf));
> ! display_buf [hex2bin (bufp, display_buf, n)] = '\0';
> return display_buf;
> }
I needed several takes on this one. Can you split the line:
display_buf [hex2bin (bufp, display_buf, n)] = '\0';
into two steps?
> }
> *************** remote_async_detach (char *args, int fro
> *** 2316,2322 ****
>
> /* Convert hex digit A to a number. */
>
> ! int
> fromhex (int a)
> {
> if (a >= '0' && a <= '9')
> --- 2309,2315 ----
>
> /* Convert hex digit A to a number. */
>
> ! static int
> fromhex (int a)
> {
> if (a >= '0' && a <= '9')
> *************** fromhex (int a)
> *** 2329,2334 ****
> --- 2322,2350 ----
> error ("Reply contains invalid hex digit %d", a);
> }
>
> + static int
> + hex2bin (char *hex, char *bin, int count)
suggest ``const char *hex''.
> + {
> + int i;
> +
> + /* May use a length, or a nul-terminated string as input. */
> + if (count == 0)
> + count = strlen (hex) / 2;
Could I suggest leaving this out. Looking through the code, this
doesn't appear to be necessary and it leaves the function open to a
buffer overrun.
> p = buf + strlen (buf);
> regp = register_buffer (regno);
> ! bin2hex (regp, p, REGISTER_RAW_SIZE (regno));
> remote_send (buf, PBUFSIZ);
perhaphs write this as:
p += bin2hex (regp, p, ...);
*p = '\0';
so it is clear that the buffer was null terminated.
nice cleanup,
Andrew