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

[libunwind] Move errno.h and signal.h includes under the block where they're needed #78054

Merged
merged 1 commit into from
Jan 16, 2024

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Jan 13, 2024

Commit fc1c478 added includes of <signal.h> and <errno.h> to UnwindCursor.hpp. The library previously built on platforms where these headers are not provided. These headers should be included only in the case where they are actually needed, i.e. on Linux.

…they're needed

Commit fc1c478 added includes of <signal.h> and <errno.h> to
UnwindCursor.hpp. The library previously built on platforms where these
headers are not provided. These headers should be included only in the
case where they are actually needed, i.e. on Linux.
@ldionne ldionne requested a review from a team as a code owner January 13, 2024 17:42
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 13, 2024

@llvm/pr-subscribers-libunwind

Author: Louis Dionne (ldionne)

Changes

Commit fc1c478 added includes of <signal.h> and <errno.h> to UnwindCursor.hpp. The library previously built on platforms where these headers are not provided. These headers should be included only in the case where they are actually needed, i.e. on Linux.


Full diff: https://github.com/llvm/llvm-project/pull/78054.diff

1 Files Affected:

  • (modified) libunwind/src/UnwindCursor.hpp (+2-2)
diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index 8517d328bd058b..7753936a5894a3 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -12,8 +12,6 @@
 #define __UNWINDCURSOR_HPP__
 
 #include "cet_unwind.h"
-#include <errno.h>
-#include <signal.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -35,6 +33,8 @@
 #if defined(_LIBUNWIND_TARGET_LINUX) &&                                        \
     (defined(_LIBUNWIND_TARGET_AARCH64) || defined(_LIBUNWIND_TARGET_RISCV) || \
      defined(_LIBUNWIND_TARGET_S390X))
+#include <errno.h>
+#include <signal.h>
 #include <sys/syscall.h>
 #include <sys/uio.h>
 #include <unistd.h>

@ldionne
Copy link
Member Author

ldionne commented Jan 13, 2024

CC @ajordanr-google

By the way, I noticed that you checked the "hide my email address" Github feature, which leads to your commits being authored as Jordan R AW <103465530+ajordanr-google@users.noreply.github.com>. Please uncheck that, we strive to make it easy to track who is the author of a commit.

@ajordanr-google
Copy link
Contributor

Done! Thanks for the heads up on the email. I'm not too familiar with the GitHub workflow still. Also thanks for this fix PR.

@ldionne ldionne merged commit c1a4424 into llvm:main Jan 16, 2024
43 checks passed
@ldionne ldionne deleted the review/signal-libunwind branch January 16, 2024 15:31
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
…they're needed (llvm#78054)

Commit fc1c478 added includes of <signal.h> and <errno.h> to
UnwindCursor.hpp. The library previously built on platforms where these
headers are not provided. These headers should be included only in the
case where they are actually needed, i.e. on Linux.
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

4 participants