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

[libc][errno] Remove previously added errno numbers #92163

Merged
merged 1 commit into from
May 15, 2024

Conversation

robincaloudis
Copy link
Contributor

@robincaloudis robincaloudis commented May 14, 2024

Introduced in #91150. Not needed anymore as #92041 fixed the root cause. ENAMETOOLONG and EOVERFLOW are well defined in <linux/errno.h>.

Post mortem: Due to the previously missing inclusion of <linux/errno.h> (fixed with #92041), I misinterpreted an undefined macro issue during the development of #91150 as being caused by a missing definition rather than by the missing inclusion of the linux header. I realized too late that ENAMETOOLONG and EOVERFLOW were correctly defined in <linux/errno.h> and that it was my missing inclusion that caused the problem.

@robincaloudis robincaloudis marked this pull request as ready for review May 14, 2024 20:22
@llvmbot llvmbot added the libc label May 14, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented May 14, 2024

@llvm/pr-subscribers-libc

Author: Robin Caloudis (robincaloudis)

Changes

Introduced in #91150. Not needed anymore as #92041 fixed the root cause. ENAMETOOLONG and EOVERFLOW are well defined in &lt;linux/errno.h&gt;.

Post mortem: Due to the previously missing inclusion of &lt;linux/errno.h&gt; (fixed with #92041), I misinterpreted an undefined macro issue during the development of #91150 as being caused by a missing definition rather than by the missing inclusion of the linux header. I realized too late that ENAMETOOLONG and EOVERFLOW were correctly defined in &lt;linux/errno.h&gt; and that it was my missing inclusion that caused the problem.


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

1 Files Affected:

  • (modified) libc/include/llvm-libc-macros/generic-error-number-macros.h (-2)
diff --git a/libc/include/llvm-libc-macros/generic-error-number-macros.h b/libc/include/llvm-libc-macros/generic-error-number-macros.h
index b5b1b676dacc3..7ee0352669b8a 100644
--- a/libc/include/llvm-libc-macros/generic-error-number-macros.h
+++ b/libc/include/llvm-libc-macros/generic-error-number-macros.h
@@ -44,7 +44,5 @@
 #define EDOM 33
 #define ERANGE 34
 #define EILSEQ 35
-#define ENAMETOOLONG 36
-#define EOVERFLOW 75
 
 #endif // LLVM_LIBC_MACROS_GENERIC_ERROR_NUMBER_MACROS_H

@robincaloudis robincaloudis changed the title [libc][errno] Remove errno numbers [libc][errno] Remove previously added errno numbers May 14, 2024
Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, and thanks for explaining why this was an issue

@robincaloudis
Copy link
Contributor Author

You are welcome. Do you mind merging this PR? I still lack write access.

Copy link
Contributor

@lntue lntue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Do you mind resolving the merge conflict?

@lntue lntue merged commit 0980f71 into llvm:main May 15, 2024
4 checks passed
mub-at-arm pushed a commit to mub-at-arm/llvm-project that referenced this pull request May 16, 2024
Introduced in llvm#91150. Not
needed anymore as llvm#92041 fixed
the root cause. `ENAMETOOLONG` and `EOVERFLOW` are well defined in
`<linux/errno.h>`.

Post mortem: Due to the previously missing inclusion of
`<linux/errno.h>` (fixed with
llvm#92041), I misinterpreted an
undefined macro issue during the development of
llvm#91150 as being caused by a
missing definition rather than by the missing inclusion of the linux
header. I realized too late that `ENAMETOOLONG` and `EOVERFLOW` were
correctly defined in `<linux/errno.h>` and that it was my missing
inclusion that caused the problem.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants