[nanomsg] Re: Warning fixes

  • From: Boszormenyi Zoltan <zboszor@xxxxx>
  • To: nanomsg@xxxxxxxxxxxxx, Martin Sustrik <sustrik@xxxxxxxxxx>
  • Date: Thu, 16 Jan 2014 11:29:28 +0100

Hi,

2013-12-19 15:21 keltezéssel, Boszormenyi Zoltan írta:
2013-12-17 12:47 keltezéssel, Boszormenyi Zoltan írta:
Hi,

I get 3 warnings when compiling nanomsg (current GIT) with
GCC 4.8.2 for 64-bit under Fedora 20:

src/core/global.c: In function 'nn_global_submit_counter':
src/core/global.c:904:13: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 5 has type 'uint64_t' [-Wformat=]
             s->socket_name, name, value);
             ^
src/core/global.c:919:17: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 8 has type 'uint64_t' [-Wformat=]
                 timebuf, value);
                 ^
src/core/global.c:923:17: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 8 has type 'uint64_t' [-Wformat=]
                 timebuf, value);
                 ^

The first attached patch fixes it in a "naive" way.
The %llu format needs an (unsigned long long)value argument,
since uint64_t is only (unsigned long) under 64-bit Linux.
It should be portable but actually isn't.

The second, more portable patch uses macros (PRIu64)
from <inttypes.h> so the compiler doesn't make the value
wider than necessary.

This header doesn't exist under Visual Studio but you can
install it from http://code.google.com/p/msinttypes/

I had the same problem in another project regarding portability,
and I used MinGW32 at the time. As it turned out, "I64d", "I64u"
and others are the proper 64-bit format specifiers for Windows.
It seems to depend on msvcrt.dll, not the compiler version.

The patches touch exactly the places that commit 624c483 modified.

I am not familiar with CMakeLists.txt but added the check for the header 
according to
http://www.cmake.org/Wiki/CMake:How_To_Write_Platform_Checks
to CMakeLists.txt and also to configure.ac and src/core/global.c.
It compiled without warnings for me under GCC.

I have tested this attached patch with CMake 2.8.12.1 and Windows 7.

I installed Visual Studio Express 2010 and 2013.

Visual Studio Express 2010 fails in several ways. It crashes all the time
when I start building the nanomsg project on an unpatched source tree.
CMake indicates errors running cl.exe properly when I used the nmake generator.

With the "Visual Studio 12 (2013)" generator, CMake fails with an error
that says "get_filename_component" and wrong number of arguments.

Finally, this patch and the nmake generator (with VS2013 Express) in CMake
found the inttypes.h file just fine. Microsoft got the clue and started 
supporting
C99 and inttypes.h, it is included with Visual Studio 2013.

If someone can make an older Visual Studio work (maybe with an older Windows),
this patch should properly complain when cmake is run if the inttypes.h file is 
missing.

It definitely did complain (and only about the missing header) when I 
temporarily
renamed inttypes.h in \Program Files (x86)\Microsoft Visual Studio 
12.0\VC\include


is there any problem with this patch? Can you review (and apply) it?

Best regards,
Zoltán Böszörményi


Other related posts: