Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
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	[thread overview]
Message-ID: <3AFA1EC2.3020902@cygnus.com> (raw)
In-Reply-To: <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



  reply	other threads:[~2001-05-10  8:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-05-02 14:45 Michael Snyder
2001-05-10  8:33 ` Andrew Cagney [this message]
2001-05-10 11:46   ` Michael Snyder
2001-05-10 11:59     ` Andrew Cagney
2001-05-10 12:08       ` Michael Snyder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3AFA1EC2.3020902@cygnus.com \
    --to=ac131313@cygnus.com \
    --cc=gdb-patches@sources.redhat.com \
    --cc=msnyder@cygnus.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox