Bug #19732
closedPossible missing header (stdint.h) in event.h
Description
Ruby's event.h (https://github.com/ruby/ruby/blob/813a5f4fc46a24ca1695d23c159250b9e1080ac7/include/ruby/internal/event.h#L103) is using type aliases from stdint.h, however it's not directly included.
An example where this causes issues is when using rb_debug_inspector_current_depth()
https://github.com/ruby/ruby/blob/813a5f4fc46a24ca1695d23c159250b9e1080ac7/include/ruby/debug.h#L279-L284C46. In a gem using a C-extension that already includes debug.h
, when adding the call rb_debug_inspector_current_depth()
, the compilation fails with:
make
compiling debug_inspector.c
In file included from /home/itarato/.rubies/ruby-master/include/ruby-3.3.0+0/ruby/debug.h:16,
from debug_inspector.c:12:
/home/itarato/.rubies/ruby-master/include/ruby-3.3.0+0/ruby/internal/event.h:105:9: error: unknown type name ‘uint32_t’
105 | typedef uint32_t rb_event_flag_t;
| ^~~~~~~~
This is on ruby/ruby
latest commit (813a5f4fc46a24ca1695d23c159250b9e1080ac7), but also tried on tag 3.2 (same error).
I've also proposed a fix: https://github.com/ruby/ruby/pull/7945
Updated by nobu (Nobuyoshi Nakada) over 1 year ago
- Status changed from Open to Feedback
Could you share the code to reproduce?
Is it https://github.com/banister/debug_inspector/tree/master/ext/debug_inspector/debug_inspector.c?
Updated by itarato (Peter Arato) over 1 year ago
While preparing that diff I realized what happened. I'm using clang-format
which sorts includes and ordered https://github.com/banister/debug_inspector/blob/5424c4094df30adfecd961a4e77d95f1fea9bc48/ext/debug_inspector/debug_inspector.c#L12-L13 to:
#include "ruby/debug.h"
#include "ruby/ruby.h"
in which case ruby.h
do not have the chance to trigger the include of stdint.h
in time. Would this count as an issue? My guess is that clang-format is pretty popular (eg default formatter in vscode) so changes are more folks would bump to this when writing new C extensions.
Updated by nobu (Nobuyoshi Nakada) over 1 year ago
- Status changed from Feedback to Open
Although I don't think it is a big issue as we don't assume or guarantee all our headers can be usable individually, welcome the improvement.
It seems ruby/internal/event.h is which really needs uint32_t
.
$ clang -I/opt/local/include/ruby-3.3.0+0/{x86_64-darwin22/,} -include ruby/internal/event.h -c -o /dev/null -xc - < /dev/null
In file included from <built-in>:1:
/opt/local/include/ruby-3.3.0+0/ruby/internal/event.h:103:9: error: unknown type name 'uint32_t'
typedef uint32_t rb_event_flag_t;
^
1 error generated.
Also HAVE_STDINT_H
is usable there.
$ clang -I/opt/local/include/ruby-3.3.0+0/{x86_64-darwin22/,} -include ruby/internal/event.h -E -dM -xc - < /dev/null | grep HAVE_STDINT_H
#define HAVE_STDINT_H 1
So it would be nice to add the include
with checking HAVE_STDINT_H
in event.h file.
diff --git a/include/ruby/internal/event.h b/include/ruby/internal/event.h
index 04b137a1939..bbf0aa45019 100644
--- a/include/ruby/internal/event.h
+++ b/include/ruby/internal/event.h
@@ -23,6 +23,10 @@
#include "ruby/internal/dllexport.h"
#include "ruby/internal/value.h"
+#ifdef HAVE_STDINT_H
+# include <stdint.h>
+#endif
+
/* These macros are not enums because they are wider than int.*/
/**
Updated by nobu (Nobuyoshi Nakada) over 1 year ago
Sorry, I got wrongly if it were about to add it to debug.h.
The file is correct, but just the include
should be after the other include
s with HAVE_STDINT_H
check.
Updated by itarato (Peter Arato) over 1 year ago
Thanks for pointing that out. Updated the PR (https://github.com/ruby/ruby/pull/7945) with those adjustments.
Updated by itarato (Peter Arato) over 1 year ago
- Status changed from Open to Closed
Applied in changeset git|b943e9c7b9d9cc8ba4bbf043414ab1ed4e1a8b5f.
Fixes [Bug #19732]. Add missing stdint.h header to event.h.