Wireshark · Ethereal-dev: Re: [Ethereal-dev] Re: BACnet packet-bacapp corrections

[Jaap Keuter <jaap.keuter@xxxxxxxxx>]

Hi,

How about lvt == 0? Is that an allowed value?

-       switch (lvt) {
   ......
-               default:
-                       valid = FALSE;
-                       break;
+       if (lvt <= 8) {
+               valid = TRUE;

The previous code says no, your patch says yes.

Thanx,
Jaap

(sorry for being picy ;)

On Sat, 15 Apr 2006, Steve Karg wrote:

> Hi Jaap,
>
> > Doesn't this snippet
> >
> > -       switch (lvt) {
> >  .....
> > -               case 8:
> > -                       *val = tvb_get_ntoh64(tvb, offset);
> > -                       break;
> > +       if (lvt < 8) {
> >
> > show that you no longer handle the lvt == 8 case?
>
> Yes - you are right, the Unsigned decode function should read:
>  > +       if (lvt <= 8) {
>
> The same can be said for the Signed portion which should read:
>  > +       if (lvt <= 7) {
>
> Thanks for catching that!  Another patch is attached to replace the
> first one.
>
> Patch summary:
>
> 1) BACnet signed values were being decoded incorrectly for negative
> values since BACnet tries to be clever and minimizes the number of bytes
> sent on the wire and drops the leading FF on negative values.  For
> example, -200 is passed as FF 38 on the wire, but would display as 65336.
>
> 2) Since the BACnet unsigned values were decoded using a 64-bit entity,
> I changed the decoding such that allows all 8 bytes to be decoded.  The
> function can now decode 5, 6, and 7 byte values.
>
> 3) Corrected warning about signed/unsigned in a pointer parameter.
>
> Best Regards,
>
> Steve
>