Zstd: Compiler macro warnings with -pedantic

[traversc]

When compiling with -Wall -pedantic

warning: ISO C99 requires at least one argument for the "..." in a variadic macro
RETURN_ERROR_IF(!cctxParams, GENERIC);

warning: ISO C99 requires at least one argument for the "..." in a variadic macro
RETURN_ERROR_IF(!cctxParams, GENERIC);

etc. There are quite a few. Would it be possible to fix?

[terrelln]

I can fix this specific case, but I have no way to guarantee that it won't pop up in the future because the flag to detect it is gcc-specific.

I will leave this issue open, because we may be able to compile zstd with pedantic, but it will require quite some work. We may be able to compile with pedantic, but we would have to disable certain checks. The flags to disable those checks, like -Wno-long-long and -Wno-variadic-macro would have to be available on all compilers.

We're always happy to add more warning flags, but it is a process that takes some time, which is a scarce resource. If anyone is willing to spend some time enabling pedantic, I'm more than happy to code review! It probably makes sense to add in flags that -pedantic enables one-by-one to break up the work into smaller pull requests.

[baziotis]

Would that kind of thinking be an acceptable solution?

#include <stdio.h>

#define RETURN_ERROR_IF2(cond, err) printf("exactly 2\n");
#define RETURN_ERROR_IF_MORE(cond, err, ...) printf("more than 2\n");
#define _RM(c, e, ...) RETURN_ERROR_IF_MORE(c, e, __VA_ARGS__)

#define REDUCE_MACRO(\
    _1, _2, _3, _4, \
    _5, _6, _7, _8, \
    _9, _10, _11, _12, \
    _13, _14, _15, _16, ...) _16

#define RETURN_ERROR_IF(...) \
    REDUCE_MACRO(__VA_ARGS__, \
        _RM, _RM, _RM, _RM, \
        _RM, _RM, _RM, _RM, \
        _RM, _RM, _RM, _RM, \
        _RM, RETURN_ERROR_IF2,)(__VA_ARGS__)

int main(void) {
    RETURN_ERROR_IF(1,2);
    RETURN_ERROR_IF(1,2,3);
    RETURN_ERROR_IF(1,2,3,4);

    return 0;
}

or maybe we should create 2 separate macros (one for exactly 2 and one for more) ?

[terrelln]

That may work, but zstd doesn't currently compile under -pedantic. If we can't compile with -pedantic, we can't detect regressions on this warning in our CI, so the complexity of the fix isn't worth it.

Fixing this warning doesn't particularly matter to us, since enabling it won't help catch potential bugs. We would fix this if we fix all of the other places that -pedantic flags, so we could enable it on our CI.

There is a bit of work to be done to enable -pedantic, like removing a few C-style comments. I will happily accept a PR that allows us to compile with -pedantic on all of our CI tests. Otherwise, I will revive PR #1540 when I have some time to fix the rest of the -pedantic warnings, but it isn't a high priority.

[baziotis]

Ok, got it. I'm new to the project (and open-source) so I figured it's a good first issue. I will try to make it able to compile with -pedantic and then fix something of higher priority.

[bimbashrestha]

Its been a while since this thread saw activity. @baziotis, are you still interested in trying to make zstd compile with -pedantic?

[baziotis]

Oh, I had forgotten that. For some reason I had found difficulties last year and did not finish it.
Now I don't have much time but if I find any, I'll try to do a PR to help with that.

[baziotis]

https://github.com/facebook/zstd/pull/1989

[bimbashrestha]

The changes required seem to complicated for the time being. Closing this issue.

[MarkCallow]

The flag to detect this warning is not gcc specific. Clang, and therefore Emscripten, also raises this warning when using -Wpedantic. There are a very large number of them (110 to be precise) when compiling the single file decoder. So please figure out a fix.

Could a fix be to change the macro to

#define RETURN_ERROR_IF_MORE(cond, ...)

with the current err parameter being the first variadic parameter?

For the time being I shall have to turn off this warning.