search

ludocode / mpack

MPack - A C encoder/decoder for the MessagePack serialization format / msgpack.org[C]
MIT License
533 stars 82 forks source link

ISO C99 requires at least one argument for the "..." in a variadic macro #52

Closed rhizoome closed 5 years ago

rhizoome commented 7 years ago

In case mpack is meant to be ISO C99/C11 compatible there are errors:

mpack.c:1280:36: error: ISO C99 requires at least one argument for the "..." in a variadic macro [-Werror]
     mpack_assert(count > UINT16_MAX);
ludocode commented 7 years ago

Yeah, this is one of those issues where I'm not sure if it's worth fixing. If you really want pedantic C99 compatibility, you could define mpack_assert(...) to nothing. This issue is actually fixable though by just adding a message to all asserts.

In any case I do want to adhere to C99 as closely as possible for all the non-debug and non-unittest code at least. To actually achieve it, it mostly depends on what other errors there are when building with -pedantic. If this is the only error then it's probably worth fixing, but I imagine there are others. It'll take a bit more investigation to figure out, so I'll leave this open for now.

rhizoome commented 7 years ago

I learned how to define different CFLAGS for mpack in make

mpack.o: CFLAGS=$(MPACK_CFLAGS)

In my project I have a similar assert macro and I defined a message for all asserts. :smile: But I agree that its probably not worth effort.

ludocode commented 5 years ago

I decided to fix this after all. I got bit by a different bug that would have been caught by GCC's pedantic warnings (3c501f434660ebc3e904029faa1b77cf2d5fa2b7) so -Wpedantic is now permanently in MPack's compiler options.

I didn't actually bother adding a format string to all assert calls (especially since the unit test macros have the same issue.) Instead I did some preprocessor hacks in 90d68658448a6a407d2e19d95502019aa46eef52 and a2049b9c910ef83bd0f2ea7a181af431e0d3f79d to allow no format string. Turns out there were other uses of __VA_ARGS__ that were dangerous on MSVC due to its strange argument expansion rules so it was a good idea to fix this. Unfortunately the definition of mpack_assert() is super ugly now but it works.