laman@sdcsvax.UUCP (Mike Laman) (08/22/84)
I received a message from "ihnp4!pegasus!hansen (aka Tony Hansen)" saying
they didn't receive bug fix #1 of the six I posted on the network. So
I thought I would repost it, just in case others missed it. The enhancement
I am including follows the original fix posting. I think that the author
either 1) didn't expect the binary data to be >= 255, or 2) they wrote the
code accidentally test the low byte. I don't know, but I suspect #1.
My thanks to Tony Hansen for pointing out this problem. He lost a few hairs
with it. The enhancement following the original posting will handle this
problem.
The original posting:
There are two bugs in the "portable" routine "getsh()" in the file
screen/setupterm.c. The first bug is that the comparison "if (*p == 0377)"
is incorrect if characters are signed by default. If *p contained the
BYTE value '\377', sign extension would make it so *p != 0377 since 0377
is an integer and will NOT be sign extended.
The second bug is that "rv = *p++" will do sign extension (for systems with
characters being signed). What we want is to sign extend on ONLY the
"high part"; more precisely, we want to sign extend ONLY on the assignment
that follows which puts the high byte in and NOT on the assignment that puts
the low byte in.
[ I suspect that if this routine was used, it was used on a machine where
characters are unsigned. ]
The original getsh() follows:
getsh(p)
register char *p;
{
register int rv;
if(*p == 0377)
return -1;
rv = *p++;
rv += *p * 256;
return rv;
}
the following should do the trick:
getsh(p)
register char *p;
{
register int rv;
rv = *((unsigned char *) p);
if(rv == 0377)
return -1; /* This is a pretty common case */
return rv + (*++p * 256);
}
Instead of this though, I use the following macro (since this is for a machine
with signed characters):
#define getsh(ip) (*((unsigned char *) ip) | (*(ip+1) << 8))
Folks on machines with unsigned character implementations could use:
#define getsh(ip) ((*ip == 0377) ? -1 : (*ip | (*(ip+1) << 8))))
[ This should serve the purpose required as stated in the comment above
the macro definitions. ]
End of Original posting
Here is part of the message I received from Tony:
"Also, you missed the (obvious?) case where *p has the VALID value of 0377.
It's just not good enough to check *p for 0377, you also have to check
*(p+1) for 0377. Alternatively, you could get by with just checking *(p+1)
because THAT byte is the high order byte of the short int, not the other
byte.
"I had a dickens of a time tracking this problem down because we had a
terminfo entry here that actually had a valid value for *p of 0377. (The
string table was VERY big.) ..."
:
:
:
Tony Hansen
The macro I suggested earlier for signed machines should still handle Tony's
problem, but I would suggest the following macro for machines with characters
being unsigned.
#define getsh(ip) ((*(ip+1) == 0377) ? -1 : (*ip | (*(ip+1) << 8))))
^^^^^^
Used to be just "ip"
I don't think we will get entries of 0x7f00 in size.
Mike Laman, NCR @ Torrey Pines
UUCP: {ucbvax,philabs,sdcsla}!sdcsvax!laman