ntpd patch

[T. William Wells]

The correct code for dealing with a plain char pointer is:

isspace(*(unsigned char *)ptr);

1) Though the defined type may, in fact, be a character, we are
treating it as an unsigned character and this code makes that
explicit.

2) This code works on ones complement machines; (unsigned
char)*ptr does not.

3) This code has a better chance of generating decent code when
optimization is turned off. (unsigned char)*ptr has an implicit
conversion to int, then the explicit conversion to unsigned
char, followed by an implicit conversion to unsigned int.
*(unsigned char *)ptr has only the implicit conversion to an
unsigned int.

To Unsubscribe: send mail to majo...@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message

[Matt Dillon]

Ummm.. I think both ways are 'correct'. First of all, there is
nothing wrong type-casting a signed char into an unsigned char.
I really doubt any FreeBSD utility would actually run on a 1's
complement machine anyway, so there is no particular reason to
try to support it, and gcc will have no problem optimizing
(unsigned char)*ptr verses *(unsigned char *)ptr... in fact,
there is a very good chance that both would produce exactly the
same code as a result, even without using any optimization flags.
Casts are one of the easiest optimizations a C compiler can make.

-Matt

:The correct code for dealing with a plain char pointer is:

[T. William Wells]

> I really doubt any FreeBSD utility would actually run on a 1's
> complement machine anyway, so there is no particular reason to
> try to support it,

Odds are, this is true. OTOH, suppose someone else took up the
patch and applied it elsewhere? It would be nice if it actually
did work right. And, who knows? Maybe someone will try to port
*BSD to a Univac someday. :)

> try to support it, and gcc will have no problem optimizing
> (unsigned char)*ptr verses *(unsigned char *)ptr...

One would hope.

> there is a very good chance that both would produce exactly the
> same code as a result, even without using any optimization flags.
> Casts are one of the easiest optimizations a C compiler can make.

You'd think. However, back when I did a lot of portable
programming, one thing I found is that compilers, especially when
run without optimization, could produce some really gawdawful
code. For example, I've seen (unsigned char)(int_expression)
generate an 'and 0xFF' instruction whether it needs it or not.

Just for amusement, I went ahead and looked to see what gcc
(without optimization) would do. For my test case, the two
produced identical code. But they both *also* generated code with
a movl %eax,%eax that served absolutely no purpose at all. :)

But really all that is beside the point -- the difference between
the two is trivial to code and one *is* correct, both
theoretically and practically, but the other only correct in
practice -- today's practice that is. If it involved any
significant effort to do it the completely correct way, I wouldn't
be arguing the point. But since it doesn't....