Bug 554 - print stats error
print stats error
Status: RESOLVED FIXED
Product: unbound
Classification: Unclassified
Component: server
unspecified
Other Linux
: P5 minor
Assigned To: unbound team
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-02-12 11:26 CET by chenfzh
Modified: 2014-04-10 13:21 CEST (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description chenfzh 2014-02-12 11:26:13 CET
version: 1.4.21


During the time I testing unbound, and using unbound-control stats to get the queries number, when the queries number are more than four billion(I didn't see the exactly number, but it should be 4294967295), the value of *.num.queries is wrong. I think it's caused by the code below.

in daemon/remote.c, print_stats
print variables of type size_t like this
    // size_t val
    printf("%u", (unsigned)val)

In x86_64 GNU/Linux, type size_t have 8 bits, unsigned(unsigned int) have 4bits.
Comment 1 Wouter Wijngaards 2014-02-13 15:50:27 CET
Hi Chenfzh,

On a 64bit computer it should printout bigger numbers.  But on a 32bit machine we do not take the effort of printing even bigger numbers.  With size_t 64bits that would actually be possible, but we did not want to complicate the statistics machinery.  Impressive to breach that 2**32 barrier :-)

The size_t is indeed counted, but it is unfortunately very complicated to print sizet at 64bits when it is 64bit (and not 32bit), hence the print as an integer (which is often correct, but not in your case with 32bit integers and 64bit size_t).  The complication is in portability for the code.

gcc -m64 should make the integers 64bit?  Could that help you get the correct statistics?

Best regards,
   Wouter
Comment 2 chenfzh 2014-02-14 00:51:44 CET
I use this code for testing in 64bits computer.
the first printf of this code is similar to the code unbound using.

        size_t a = 7119719664;
        printf("%u\n", (unsigned)a);
        printf("%lu\n", a);
        printf("%zu\n", a);

the output are the same both in gcc -m64 and gcc with no specific options(I think -m64 is default in a 64bits computer)

output:
       2824752368
       7119719664
       7119719664

Maybe %zu can make it better("it's a C99 addition"), this problem is mentioned here https://stackoverflow.com/questions/940087/whats-the-correct-way-to-use-printf-to-print-a-size-t and 
https://stackoverflow.com/questions/2524611/how-to-print-size-t-variable-portably
Comment 3 chenfzh 2014-02-14 01:00:41 CET
I use this code for testing in 64bits computer.
the first printf of this code is similar to the code unbound using.

        size_t a = 7119719664;
        printf("%u\n", (unsigned)a);
        printf("%lu\n", a);
        printf("%zu\n", a);

the output are the same both in gcc -m64 and gcc with no specific options(I think -m64 is default in a 64bits computer)

output:
       2824752368
       7119719664
       7119719664

Maybe %zu can make it better("it's a C99 addition"), this problem is mentioned here https://stackoverflow.com/questions/940087/whats-the-correct-way-to-use-printf-to-print-a-size-t and 
https://stackoverflow.com/questions/2524611/how-to-print-size-t-variable-portably

But it's not supported by MS, MS uses 'I' according to http://msdn.microsoft.com/en-us/library/vstudio/tcxf1dw6.aspx ......
Comment 4 Wouter Wijngaards 2014-02-14 09:27:59 CET
Hi Chenfzh,

Right now unbound is avoiding the printf-64bit issues by printing 'int'.  But nasty that int is still 32bit on 64bit systems.

We could go to "%lu", (long)val  it prints correctly in your test.  It is also very portable.  Prints 32bit-values on 32bit systems, and 64bit on 64bit compiles, which is fine I think?

Best regards,
   Wouter
Comment 5 chenfzh 2014-02-17 03:31:05 CET
I modified the code, using "%lu, (unsigned long)"after I found unbound-control stats error, it works well.

I tested it in a 32 bits system, it's ok, exceed, just like expected.
Comment 6 Wouter Wijngaards 2014-02-17 09:09:46 CET
Hi Chenfzh,

That is good!  I'll put it on TODO to implement this on svn trunk for a future unbound release.

Best regards,
   Wouter
Comment 7 Wouter Wijngaards 2014-04-10 13:21:34 CEST
Hi Chenfzh,

The change is made in svn.  Thanks for the reports, testing and proposed patch :-)

Best regards, Wouter