C API: Safer function pointer casting#10486
Merged
edolstra merged 3 commits intoNixOS:masterfrom Apr 16, 2024
Merged
Conversation
See NixOS#8699 (comment) Casting a function pointer to `void*` is undefined behavior in the C spec, since there are platforms with different sizes for these two kinds of pointers. A safe alternative might be `void (*callback)()`
12 tasks
Ericson2314
reviewed
Apr 13, 2024
src/libexpr-c/nix_api_value.cc
Outdated
Comment on lines
18
to
20
| # include "gc/gc.h" | ||
| # define GC_INCLUDE_NEW 1 | ||
| # include "gc_cpp.h" | ||
| #include "gc/gc.h" | ||
| #define GC_INCLUDE_NEW 1 | ||
| #include "gc_cpp.h" |
Member
There was a problem hiding this comment.
Please don't do this, the indentation between ifdef and endif helps legibility.
Member
There was a problem hiding this comment.
Did clang-format with our settings do this by any chance?
Member
Author
There was a problem hiding this comment.
yep, that was caused by cland-format settings (my editor auto-formats it on save)
I'm going to revert it
Member
|
|
Member
Author
right, and we already had it but I totally missed it 🤦 nix/src/libutil-c/nix_api_util.h Line 129 in 03eb4f7 Updated to use the typedef |
Ericson2314
approved these changes
Apr 15, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
See #8699 (comment)
Casting a function pointer to
void*is undefined behavior in the C spec, since there are platforms with different sizes for these two kinds of pointers. A safe alternative might bevoid (*callback)()Context
Stabilize the C interface introduced in #8699
Milestone: https://github.com/NixOS/nix/milestone/52
Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.