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

ffi/util: Deal with stupid GNU basename quirks #1825

Merged
merged 2 commits into from
Jun 17, 2024
Merged

Conversation

NiLuJe
Copy link
Member

@NiLuJe NiLuJe commented Jun 16, 2024

basename("/foo/") yielding "" is very much not what we signed up for...


This change is Reviewable

basename("/foo/") yielding "" is very much not what we signed up for...
@NiLuJe
Copy link
Member Author

NiLuJe commented Jun 16, 2024

There is thankfully no such madness happening with dirname, so we once again have a guarantee that joining the output of dirname with the output of basename yields something sensible.

@NiLuJe
Copy link
Member Author

NiLuJe commented Jun 16, 2024

See koreader/koreader#12038 (comment) for more details, because I had completely forgotten we'd switched to (attempt to) prefer the GNU imp, because (ironically) Android...

@mergen3107
Copy link

Ah, OK xD You beat me to it, lol

@NiLuJe
Copy link
Member Author

NiLuJe commented Jun 16, 2024

because (ironically) Android...

Specifically, the only guarantee we have on bionic is that the public symbol will be named basename. This happens to match the name of the GNU implementation on glibc, where the POSIX one is instead exported as __xpg_basename....

Whether the Android implementation leans more towards a POSIX or GNU behavior is irrelevant (for the record, it leans towards a sane, POSIX behavior, while also not modifying its input ;p).

We basically don't know what we're getting, so, we need to deal with all
the potential quirks.
Copy link
Member

@Frenzie Frenzie left a comment

Choose a reason for hiding this comment

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

Ummmm, well, okay.

@NiLuJe NiLuJe merged commit 9b707fa into koreader:master Jun 17, 2024
3 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.

None yet

3 participants