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

libcomposefs: include linux/limits.h to satisfy usage of XATTR_NAME_MAX #366

Merged
merged 1 commit into from
Sep 27, 2024

Conversation

ziyao233
Copy link
Contributor

XATTR_NAME_MAX is defined in linux/limits.h, but leaked by glibc since its sys/param.h includes the former header, which is not true on musl and leads to errors like:

	../libcomposefs/lcfs-writer.c:1688:16: error: use of undeclared identifier 'XATTR_NAME_MAX'
	 1688 |         if (namelen > XATTR_NAME_MAX) {

Include linux/limits.h explicitly to build properly on musl.

XATTR_NAME_MAX is defined in linux/limits.h, but leaked by glibc since
its sys/param.h includes the former header, which is not true on musl
and leads to errors like:

	../libcomposefs/lcfs-writer.c:1688:16: error: use of undeclared identifier 'XATTR_NAME_MAX'
	 1688 |         if (namelen > XATTR_NAME_MAX) {

Include linux/limits.h explicitly to build properly on musl.

Signed-off-by: Yao Zi <[email protected]>
Copy link
Contributor

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Hi, thanks for the patch!

First a procedural note, we have an ongoing attempt at a relicensing: #344

Are you OK with licensing this contribution under those terms instead of the current ones?

Also, it should be trivial to add a CI build on a musl system, would be a nice followup to this.

@ziyao233
Copy link
Contributor Author

Hi, thanks for the patch!

First a procedural note, we have an ongoing attempt at a relicensing: #344

Are you OK with licensing this contribution under those terms instead of the current ones?

Also, it should be trivial to add a CI build on a musl system, would be a nice followup to this.

I'm not familiar with GitHub CI, but if possible I suggest building on Alpine Linux, which is also the most popular musl-based distro.

@cgwalters cgwalters merged commit e2afe90 into containers:main Sep 27, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants