Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xkbvalidate: Use $CC instead of hardcoded gcc #66648

Merged
merged 3 commits into from Aug 14, 2019

Conversation

aszlig
Copy link
Member

@aszlig aszlig commented Aug 14, 2019

I initially didn't use $CC because I thought this would be GCC specific, but it turns out that Clang actually accepts -std=gnu11.

So using $CC here might not work on compilers other than Clang or GCC, but at the moment those are the compilers we typically use in nixpkgs, so even if we'd use some other compiler it might even work there.

I've tested this by compiling against clangStdenv with both $CC and clang hardcoded and it works.

This was reported by @dkudriavtsev on IRC.

I initially didn't use $CC because I thought this would be GCC specific,
but it turns out that Clang actually accepts -std=gnu11.

So using $CC here might not work on compilers other than Clang or GCC,
but at the moment those are the compilers we typically use in nixpkgs,
so even if we'd use some other compiler it *might* even work there.

I've tested this by compiling against clangStdenv with both $CC and
clang hardcoded and it works.

This was reported by @dkudriavtsev on IRC.

Signed-off-by: aszlig <aszlig@nix.build>
The only reason why I was using _GNU_SOURCE was because of vasprintf(),
so getting rid of that extension should make the source way more
portable.

When using vsnprintf() with a null pointer for the output buffer and a
size of 0, I wasn't quite sure whether this would be undefined
behaviour, so I looked it up in the C11 standard.

In section 7.21.6.5, it explicitly mentions this case, so we're lucky:

  If n is zero, nothing is written, and s may be a null pointer.

Additionally, section 7.21.6.12 writes the following about vsnprintf():

  The vsnprintf function does not invoke the va_end macro.

So to be sure to avoid undefined behaviour I subsequently added the
corresponding va_end() calls.

With this, the platforms attribute is now "unix", because the program
should now even run on OS X, even though it usually wouldn't be needed.

Signed-off-by: aszlig <aszlig@nix.build>
So far, the output binary has been just "validate", which is quite a
very generic name and doesn't match the package name.

Even though I highly doubt that this program will ever be used outside
of NixOS modules, it's nevertheless less confusing to have a consistent
naming.

Signed-off-by: aszlig <aszlig@nix.build>
@aszlig aszlig merged commit 16ecd0d into NixOS:master Aug 14, 2019
aszlig added a commit that referenced this pull request Aug 14, 2019
This allows xkbvalidate to be compiled via Clang and also has a few
other portability improvements, eg. it now can even be compiled on OS X,
even though it's probably not needed there.

In addition, I changed the binary name so that it matches the package
name.

I'm merging this in right now, because there is only the xserver NixOS
module where this is used, so the risk of a catastrophic breakage is
very low.

Checks and build done by ofborg also ran successfully and I also did a
few local tests (eg. running via valgrind to avoid leaks) to make sure
it's still working properly.
@aszlig aszlig deleted the xkbvalidate-clang branch August 14, 2019 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant