caget shows incorrect output for binary integers

[mdavidsaver]

Richard Fearn reports

> `caget -lb` outputs an empty string if the value of the PV is zero.
>
> The algorithm in sprint_long never modifies the ret array if val is zero, since no set bit is ever encountered.

[mdavidsaver (mdavidsaver)]

With a test .db file

> record(longin, "x") {}
> record(ai,"y") {}

With VAL=0 I see

> $ caget -lb x
> x 0
> $ caget -lb y
> y
> $ caget -0b x
> x
> $ caget -0b y
> y 0

With VAL=4 I see

> $ caget -lb x
> x 4
> $ caget -lb y
> y 100
> $ caget -0b x
> x 100
> $ caget -0b y
> y 4

Does anyone else find this behavior confusing?

[Richard Fearn (richardfearn)]

> Does anyone else find this behavior confusing?

Yes, but I interpret the options as saying:
* "-l" only takes effect if the returned value is floating point
* "-0" only takes effect if the returned value is an integer

x returns a LONG, whereas y returns a DOUBLE.

So "-lb" has no effect for x; "-0b" has no effect for y.

[Richard Fearn (richardfearn)]

The bug has a slightly worse effect when using camonitor, since the array containing the binary representation of the last value doesn't change when zero is encountered:

[...]
ess-devHost:aiExample 2015-02-05 13:57:43.106351 110 HIGH MINOR
ess-devHost:aiExample 2015-02-05 13:57:44.106790 111 HIGH MINOR
ess-devHost:aiExample 2015-02-05 13:57:45.107820 1000 HIHI MAJOR
ess-devHost:aiExample 2015-02-05 13:57:46.108123 1001 HIHI MAJOR
ess-devHost:aiExample 2015-02-05 13:57:47.108504 1001 LOLO MAJOR <-- should show "0"
ess-devHost:aiExample 2015-02-05 13:57:48.109039 1 LOLO MAJOR
ess-devHost:aiExample 2015-02-05 13:57:49.109933 10 LOLO MAJOR
ess-devHost:aiExample 2015-02-05 13:57:50.111493 11 LOW MINOR
[...]

[mdavidsaver (mdavidsaver)]

With the proposed fix I see

> $ ./bin/linux-x86/caget -0b x y
> x 0
> y 0
> $ ./bin/linux-x86/caget -lb x y
> x 0
> y 0

and

> $ ./bin/linux-x86/caget -lb x y
> x 4
> y 100
> $ ./bin/linux-x86/caget -0b x y
> x 100
> y 4

which is an improvement.

[mdavidsaver (mdavidsaver)]

> The bug has a slightly worse effect when using camonitor

That's definately not acceptable.

[mdavidsaver (mdavidsaver)]

In tool_lib.c

> char *val2str (const void *v, unsigned type, int index)
> {
> #define STR 500
> static char str[STR];

I think it would be a good idea to re-init this buffer at each invocation with a non-zero value to make these kind of bugs more obvious. Something like:

> memset(str, '!', sizeof(str));

I'll add this if there are no objections.

[mdavidsaver (mdavidsaver)]

Ha, with a shared buffer other fun collisions are possible!

> $ camonitor -0b x string
> x 2015-02-05 10:06:00.104804
> string 2015-02-05 10:05:58.179452 hello
> x 2015-02-05 10:06:22.245442 100
> string 2015-02-05 10:06:26.818234 world
> x 2015-02-05 10:06:28.393319 world

[Richard Fearn (richardfearn)]

> Ha, with a shared buffer other fun collisions are possible!

Yeah - I think lines 2 and 6 in your output would also be fixed by the change to sprint_long.

[Richard Fearn (richardfearn)]

> I think it would be a good idea to re-init this buffer at each invocation with a non-zero value to make these kind of bugs more obvious.

Agreed. That buffer isn't initialised, and if the PV is zero, nothing is written to the buffer before it's printed out. Is that undefined behaviour? (I don't know.) I guess it ought to be initialised to *something*, even if it's just an empty string...

[mdavidsaver (mdavidsaver)]

Your fix is now merged, and I've added initialiation of the string buffer, which should hopefully make any similar problems stand out.