Fix warnings with lib/libpmc

[ngie]

Summary

• Use MIN instead of similar hand rolled macro.
• Sort headers.
• Use errno.h instead of sys/errno.h.
• Wrap sizeof argument in parentheses.
• Remove __BSD_VISIBLE and _XOPEN_SOURCE #defines to mute warnings about incompatible snprintf definitions.

This fixes a number of warnings I've been seeing lately in my builds.

Sort makefile variables per style.Makefile(9) (CFLAGS/CWARNFLAG.gcc) and bump WARNS to 3.

MFC after: 2 weeks

Test Plan

make tinderbox

[arichardson]

LGTM.

lib/libpmc/pmu-events/jevents.h

14

This min() macro is a lot better than the one from sys/param.h since it only evaluates arguments once and checks that they are of the same type. Shouldn't matter here since I'm sure clang is smart enough to not evaluate (unsigned)rlim.rlim_max / 2 twice.

I would really like a sensible min()+max() macro but I'm not sure we can change MIN/MAX since there might be some (probably broken) uses that rely on arguments being evaluated twice.

[ngie]

lib/libpmc/pmu-events/jevents.h

14

The problem is that it was triggering -Wconversion issues because one of the values was interpreted as unsigned pointer and the other as signed pointer (!?).

Furthermore, typeof is a non-standard GNU extension: https://stackoverflow.com/a/12081525, https://en.wikipedia.org/wiki/Typeof . I don't think we should be relying on GNU extensions like this.

[ngie]

Add forgotten Makefile.inc

[arichardson]

lib/libpmc/pmu-events/jevents.h

14

typeof (or __typeof/ __typeof__) is already used in quite a few places in the source tree (e.g. in __containerof) so I'm pretty sure it isn't possible to build with a compiler that doesn't support typeof.

To fix the -Wconversion, I think changing the 512 to 512u should also work.

I think removing yet another custom min/max macro is good, I'm just wondering if we can improve the default one from param.h.

[ngie]

lib/libpmc/pmu-events/jevents.h

14

I'd be game to improve the existing macro... it's just that we'd need to be careful/judicious about it, since it would affect a number of downstream consumers of the macro (both internal and external to the base system). At the end of the day, would it really be worth the effort for something that's been in place for many moons?

About the proposed -Wconversion fix, that would work, unless the values compared are variable depending on the architecture. I recently embarked on this quest for the Google Test project, and it proved annoying to say the least: https://github.com/google/googletest/pull/2170 .

[ngie]

lib/libpmc/pmu-events/jevents.c

867

Casting rlim.rlim_max to unsigned is bogus :/.

[ngie]

Remove bogus (unsigned) cast with call to MIN().

[emaste]

lib/libpmc/Makefile

109

I thought it was typical to have WARNS near the top, but looking now I see a variety of placements.

lib/libpmc/libpmc_json.cc

44–46

Why don't these sort with the rest?

lib/libpmc/pmu-events/jevents.c

4–5

Is there some sort of upstream for this?

[ngie]

lib/libpmc/Makefile

109

From style.Makefile:

•   Special variables (i.e., LIB, SRCS, MLINKS, etc.) are listed in order
    of “product”, then building and installing a binary.  Special
    variables may also be listed in “build” order: i.e., ones for the
    primary program (or library) first.  The general “product” order is:
    PROG/[SH]LIB/SCRIPTS FILES LINKS [NO_]MAN MLINKS INCS SRCS WARNS
    CFLAGS DPADD LDADD.  The general “build” order is:
    PROG/[SH]LIB/SCRIPTS SRCS WARNS CFLAGS DPADD LDADD INCS FILES LINKS
    [NO_]MAN MLINKS.

So I should move it up.

lib/libpmc/libpmc_json.cc

44–46

These aren't system headers and rely on other headers being included and other definitions be defined before them.

lib/libpmc/pmu-events/jevents.c

4–5

Hmm... @mmacy added this copyright attribute in rS334128 last year, so I think this is copy-pasted from another place.

*does a google search* Wait.. it _was_ copied from one of the Intel folks' (andikleen@) repositories who works on Linux: https://github.com/andikleen/pmu-tools/blob/master/jevents/jevents.c . Why wasn't this noted in the original code import :(?

[ngie]

lib/libpmc/libpmc_json.cc

44–46

Ah, part of the problem is that some of the changes needed to make the libraries compile with C++ haven't been imported. In particular, we need to sprinkle __BEGIN_DECLS/__END_DECLS around the headers.

[ngie]

Sort the Makefile variables per style.Makefile(9)

[ngie]

Let's try triggering the issue in bug 233177 again

[ngie]

Try again with a patched libcurl

[jtl]

LGTM