Project

General

Profile

Actions

Bug #19732

closed

Possible missing header (stdint.h) in event.h

Added by itarato (Peter Arato) 11 months ago. Updated 11 months ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:113908]

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) 11 months ago

  • Status changed from Open to Feedback

Updated by itarato (Peter Arato) 11 months 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) 11 months 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) 11 months 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 includes with HAVE_STDINT_H check.

Updated by itarato (Peter Arato) 11 months ago

Thanks for pointing that out. Updated the PR (https://github.com/ruby/ruby/pull/7945) with those adjustments.

Actions #6

Updated by itarato (Peter Arato) 11 months ago

  • Status changed from Open to Closed

Applied in changeset git|b943e9c7b9d9cc8ba4bbf043414ab1ed4e1a8b5f.


Fixes [Bug #19732]. Add missing stdint.h header to event.h.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0