Small problem with Remote Protocol register fetching.

[Steven Johnson]

Michael, Andrew,

I dont have a problem with it if it is defined to be lower case, but I 
checked the spec, and it wasnt defined anywhere (that I could find) 
exactly what Hex Encoding is, and what characters are valid.  So I went 
and looked for the function that interprets Hex Encoded characters.

remote-utils.c contains a fromhex function, that only supports lower case.
gdbtk-cmds.c contains a fromhex function, that supports upper and lower 
case.
monitor.c contains a fromhex function, that supports upper and lower case.
remote-sds.c contains a fromhex function, that only supports lower case.
remote.c contains a fromhex function, that supports upper and lower 
case. (This is the only one that seems to be used by the standard remote 
protocol)

There being more Hex Decoding functions supporting Upper and Lower case, 
I thought supporting both cases was the intention.  If the intention is 
that only lower case is the way it is encoded, I dont have any problem 
with this, but there needs to be a section added to the Remote Serial 
Protocol defining what Hex Encoding is, and what values are valid.  
Supporting Upper case in this one function didnt seem like a big change, 
if any others come up, where people have a problem with upper case, it 
will show itself in time, and exists now anyway.

If anyone does what I did and look for the "from_hex" function in 
remote.c, which is used to decode hex encoded values, and if lower case 
is all that is intended. The ability to decode upper case should be 
removed from remote.c from_hex.  I would suggest still doing it for a 
couple of versions, but print a warning suggesting Upper Case hex 
encoding is deprecated, in case there are targets out there that use 
upper case hex encoding for things like data read packets, etc.

BTW, I checked for all use of fromhex in remote.c, and the only place I 
can find any pre-checking of the Hex values is the one that is patched.  
So I am pretty confident it would fix the only place where upper case 
letters arent fully supported.

If the consensus is it should be lower case only, I will work up a patch 
to deprecate upper case from "fromhex" and an addition to the 
documentation to specify.  If upper case is allowed, I will work up 
another patch. A patch to the GDB documentation to describe what hex 
encoding is. (Unless there already is one that ive missed.)

So whatever the consenus view is, ill go that way. I dont care either 
way, it just seemed inconsistent currently.

Steven.



Michael Snyder wrote:

> Steven Johnson wrote:
>
>> Registers in the remote protocol are Hex Encoded.  Hex encoded values 
>> can have (as far as I can tell, valid values of 
>> '0'-'9','a'-'f','A'-'F' and ('x' for registers). the problem is that 
>> register packets that have an upper case 'A'-'F' in the first 
>> location are junked as being bad packets, when their is nothing 
>> wrong.  And then GDB ends up in an infinite comms loop, trying to 
>> recover.
>>
>> The attached patch allows Hex Encoded values to include upper case 
>> letters (in the case of fetching registers) without causing the 
>> packet handling to fail.
>>
>> I wasnt sure if 'X' should also be allowable, seems like it should, 
>> but i dont know for sure, so havent changed it.
>>
>> Steven Johnson
>
>
> What Andrew said, but OTOH, this is certainly not an intrusive change.
> Steven, what remote target are you seeing this with?
>
> [Someone will say "but then we'll have to make sure that we consistantly
> accept upper case throughout, and I agree that's a little more intrusive]
>
>
>> ------------------------------------------------------------------------
>>
>> diff -p -u -r clean-src/insight-6.1/gdb/frame.c 
>> mod-src/insight-6.1/gdb/frame.c
>> diff -p -u -r clean-src/insight-6.1/gdb/remote.c 
>> mod-src/insight-6.1/gdb/remote.c
>> --- clean-src/insight-6.1/gdb/remote.c    2004-02-26 
>> 06:41:00.000000000 +1000
>> +++ mod-src/insight-6.1/gdb/remote.c    2004-06-15 16:36:53.000000000 
>> +1000
>> @@ -3278,6 +3278,7 @@ remote_fetch_registers (int regnum)
>>       and try to fetch another packet to read.  */
>>    while ((buf[0] < '0' || buf[0] > '9')
>>       && (buf[0] < 'a' || buf[0] > 'f')
>> +     && (buf[0] < 'A' || buf[0] > 'F')
>>       && buf[0] != 'x')    /* New: unavailable register value */
>>      {
>>        if (remote_debug)
>
>
>
>