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

Add missing includes #29

Merged
merged 1 commit into from
Aug 8, 2024
Merged

Add missing includes #29

merged 1 commit into from
Aug 8, 2024

Conversation

sertonix
Copy link
Contributor

@sertonix sertonix commented Aug 8, 2024

Missing includes are considered an error since GCC 14

Missing includes are considered an error since GCC 14
Copy link
Contributor

@pvuorela pvuorela left a comment

Choose a reason for hiding this comment

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

Looking good.

@pvuorela pvuorela merged commit 2013b0f into sailfishos:master Aug 8, 2024
@spiiroin
Copy link
Contributor

spiiroin commented Aug 8, 2024

@sertonix Out of curiosity: how did need for libgen.h come up? Did you get compilation time warnings?

... the idea has sort of been to explicitly avoid libgen.h by using GNU basename() via

#define _GNU_SOURCE
#include <string.h>

@sertonix sertonix deleted the include branch August 8, 2024 11:33
@sertonix
Copy link
Contributor Author

sertonix commented Aug 8, 2024

These patches came from alpine linux which uses musl libc. And musl libc follows the posix spec which only requires basename(3) in libgen.h.

@spiiroin
Copy link
Contributor

spiiroin commented Aug 9, 2024

... which uses musl libc. And musl libc follows the posix spec ...

NB This would have been better context info for the PR than gnu version something.

Anyway, posix basename() behavior differs from gnu basename(). MCE codebase expects gnu basename(). Which makes blindly taking posix basename in use = Incorrect fix for the musl problem.

Please check / review / test: #31

@sertonix
Copy link
Contributor Author

sertonix commented Aug 9, 2024

Yes, that seems to work too. It looks like musl only does modifications to path that didn't change the behavior.

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.

3 participants