Re: [RFA] Factor out some functions in remote.c

Andrew Cagney
2001-05-10T08:33:00Z

> 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