John Barrett wrote:
> Here is the patch and source files for the preliminary wire protocol
> implementation -- comments and suggestions welcome

This sounds fun, so I grabbed it and had a peek.  One bug report in
messagebuf.cxx, which has some code that I can't figure out:

> void FGMPSMessageBuf::put(unsigned char byte, bool raw)
> {
>         if (!raw) {
>                 if ((byte & 0xf0) == 0xf0) {
>                         buf += 0xff;
>                         byte = byte ^ 0xff;
>                 }
>         }
>         buf += byte;
> }

If I read this correctly, if "raw" is false (which is the default
argument), then values in the range 0-239 are passed normally, while
values in the range 240-255 cause an extra byte of 255 to be inserted
in the stream, followed by the bitwise negation of the original byte.
I don't see anywhere for the extra 255 to be interpreted on read,
though.

I'll admit that I have absolutely no idea what this code is supposed
to do. :) Are you maybe trying to handle 2's complement signed values
via an escaping mechanism?  If so, you need to change the bit test to
0x80, and can elide the complement operation entirely.  It's best to
leave signedness determination in the hands of the user code, though.

One other nit is a collection of portability bugs in your type
declarations.  A "long" value isn't guaranteed to be 32 bits, on an
LP64 platform (64 bit unices, but not Win64) it's a 64 bit quantity.
I also see some locations where you appear to be assuming that an
"int" is 16 bits, which was true on the PDP-11 and 8086, but nowhere
else; it's not clear if this extra precision will result in real bugs
or not.

Andy

-- 
Andrew J. Ross        Beyond the Ordinary        Plausibility Productions
Sole Proprietor       Beneath the Infinite       Hillsboro, OR
                  Experience... the Plausible?



_______________________________________________
Flightgear-devel mailing list
[EMAIL PROTECTED]
http://mail.flightgear.org/mailman/listinfo/flightgear-devel

Reply via email to