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

Switch to POSIX implementation of basename #1471

Merged
merged 2 commits into from
Jun 5, 2024

Conversation

nmorey
Copy link
Contributor

@nmorey nmorey commented Jun 4, 2024

  • Drop the useless basename calls in rping
  • Include libgen for the POSIX version if kernel-boot and mlx5

Both call basename on non-const string generated by readlink of a sysfs path. The original string never re-used afterward.
So in these cases, both flavours of basename behave exactly the same from the code PoV

This should solve the issue raised in #1467 and #1443

nmorey added 2 commits June 4, 2024 08:49
Drop calls to basename on a constant string without a path in it ("rping")

Signed-off-by: Nicolas Morey <nmorey@suse.com>
Include libgen.h to use the POSIX version and not the GNU version of basename.
Because it is used on readlink from sysfs paths, there is no risk of trailing '/'
so they both behave the same way.

This fixes issues with the latest musl which removed the GNU implementation of basename.

Signed-off-by: Nicolas Morey <nmorey@suse.com>
@nmorey nmorey closed this Jun 5, 2024
@nmorey nmorey deleted the dev/basename branch June 5, 2024 06:34
@nmorey nmorey restored the dev/basename branch June 5, 2024 06:40
@nmorey nmorey reopened this Jun 5, 2024
@nmorey
Copy link
Contributor Author

nmorey commented Jun 5, 2024

I'm an idiot. Thought it got merged and deleted the branch :/

@rleon rleon merged commit 1840c1b into linux-rdma:master Jun 5, 2024
13 checks passed
@nmorey nmorey deleted the dev/basename branch June 5, 2024 16:02
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