From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cagney <ac131313@cygnus.com>
To: Michael Snyder <msnyder@cygnus.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFA] Factor out some functions in remote.c
Date: Thu, 10 May 2001 08:33:00 -0000
Message-id: <3AFA1EC2.3020902@cygnus.com>
References: <3AF07FD4.62C23D84@cygnus.com>
X-SW-Source: 2001-05/msg00151.html

> 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