Skip to content

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

Merged
NiLuJe merged 2 commits into
koreader:masterfrom
NiLuJe:master
Jun 17, 2024
Merged

ffi/util: Deal with stupid GNU basename quirks#1825
NiLuJe merged 2 commits into
koreader:masterfrom
NiLuJe:master

Conversation

@NiLuJe

@NiLuJe NiLuJe commented Jun 16, 2024 •
edited by Frenzie
Loading

Copy link
Copy Markdown
Member

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

This change is Reviewable

@NiLuJe
ffi/util: Deal with stupid GNU basename quirks
dc3b26f 
basename("/foo/") yielding "" is very much not what we signed up for...
@NiLuJe

NiLuJe commented Jun 16, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

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

NiLuJe commented Jun 16, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

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

mergen3107 commented Jun 16, 2024

Copy link
Copy Markdown

Ah, OK xD You beat me to it, lol

@mergen3107 mergen3107 mentioned this pull request Jun 16, 2024
Closed
@NiLuJe

NiLuJe commented Jun 16, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

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).

@NiLuJe
Update the comment to make it slightly more accurate.
976d0d8 
We basically don't know what we're getting, so, we need to deal with all
the potential quirks.
Frenzie
Frenzie approved these changes Jun 16, 2024
View reviewed changes

@Frenzie Frenzie left a comment

Copy link
Copy Markdown
Member

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
@mergen3107 mergen3107 mentioned this pull request Jun 18, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Frenzie Frenzie Frenzie approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@NiLuJe @mergen3107 @Frenzie