Re: [RFA] monitor.c fix for rom68k target boards

[Jeff Holcomb <jeffh@redhat.com>]

On Fri, 30 Mar 2001, Andrew Cagney wrote:

> So the old code would just return with ``p'' pointing to one char beyond
> the hex digit?

Yes, though p is a local variable in monitor_supply_register.  After the 
strtoul call p points to the location following the last valid digit.  The 
old code looked like this:

  val = strtoul (valstr, &p, 16);
  RDEBUG (("Supplying Register %d %s\n", regno, valstr));
 
  if (val == 0 && valstr == p)
    error ("monitor_supply_register (%d):  bad value from monitor: %s.",
           regno, valstr);

This seems like it was a valid check.  If the strtoul totally fails (no 
value was found and p hasn't changed), then it reports the error.

The new code looks like this:

  p = valstr;
  while (p && *p != '\0')
    {
      if (*p == '\r' || *p == '\n')
        {
          while (*p != '\0') 
              p++;
          break;
        }
      if (isspace (*p))
        {
          p++;
          continue;
        }
      if (!isxdigit (*p) && *p != 'x')
        {
          break;
        }
 
      val <<= 4;
      val += fromhex (*p++);
    }
  monitor_debug ("Supplying Register %d %s\n", regno, valstr);
 
  if (*p != '\0')
    error ("monitor_supply_register (%d):  bad value from monitor: %s.",
           regno, valstr);

This is testing something totally different.  If the next character is 
not the end of the string, then it reports the error.  The rom68k 
monitor outputs a text representation of the status register flags after 
the value.

A possibility would be to change the test back to the original version 
rather than removing the test, like:

  if (val == 0 && valstr == p)
     error ("monitor_supply_register (%d):  bad value from monitor: %s.",
           regno, valstr);

I'm willing to go either way, I just don't know what the intent was in 
changing the if test.

--
Jeff Holcomb
jeffh@redhat.com
GDB Engineering
Red Hat, Inc.