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 dirname and basename #2974

Merged
merged 1 commit into from Oct 25, 2022
Merged

add dirname and basename #2974

merged 1 commit into from Oct 25, 2022

Conversation

SteveLauC
Copy link
Contributor

This PR adds dirname(3) and basename(3) on the following platforms:

  • Linux with glibc
  • Linux with musl
  • Android
  • FreeBSD
  • DragonFlyBSD
  • NetBSD
  • OpenBSD
  • Apple platforms

I tested this PR on my host machine (Linux with glibc), and got the following error:

RUNNING ALL TESTS
bad basename function pointer: rust: 140093945892128 (0x7f6a29e14d20) != c 140093945544944 (0x7f6a29dc00f0)
thread 'main' panicked at 'some tests failed', /home/steve/Documents/workspace/libc/target/debug/build/libc-test-592f01d15ee93e7a/out/main.rs:12:21
stack backtrace:
   0: std::panicking::begin_panic
             at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:616:12
   1: main::main
             at /home/steve/Documents/workspace/libc/target/debug/build/libc-test-592f01d15ee93e7a/out/main.rs:12:21
   2: core::ops::function::FnOnce::call_once
             at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/ops/function.rs:248:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
error: test failed, to rerun pass '--test main'

The reason for this error probably is that there are two basename(3) on Linux with glibc, the POSIX version and the GNU version, and they clash with each other. In C, if one #include <libgen.h>, then the POSIX version will be available; If one #define _GNU_SOURCE and #include <string.h>, then the GNU one will be used.

Can we distinguish them in libc?

@rust-highfive
Copy link

Some changes occurred in OpenBSD module

cc @semarie

@rust-highfive
Copy link

r? @JohnTitor

(rust-highfive has picked a reviewer for you, use r? to override)

@semarie
Copy link
Contributor

semarie commented Oct 23, 2022

LGTM for OpenBSD part.


Regarding Linux, at library level, you can't have both functions with the same symbol.

When you include libgen.h, the preprocessor will replace basename with __xpg_basename. But rustc doesn't use the preprocessor so you need to "replace" it yourself: you should specify the symbol you intent the function to link to:

#[link_name = "__xpg_basename"]
pub fn basename(path: *mut ::c_char) -> *mut ::c_char;

Please note that a proper documentation might be required to point that the function is the posix one (and not the gnu one).

I dunno if libc should expose posix or gnu one or both (but with different names).

@SteveLauC
Copy link
Contributor Author

SteveLauC commented Oct 23, 2022

Just tested, we can have both of them in Rust since we don't have a preprocessor:

// libc: linux/gnu/mod.rs

#[link_name = "__xpg_basename"]
pub fn posix_basename(path: *mut ::c_char) -> *mut ::c_char;

#[link_name = "basename"]
pub fn gnu_basename(path: *const ::c_char) -> *mut ::c_char;
Expected Behavior
path argument POSIX basename returns GNU basename returns
"/" "/" "" (an empty string)
Test its functionality
// main.rs: test code

use libc::{c_char, gnu_basename, posix_basename, printf};

fn main() {
    let path1 = String::from("/\0");
    let path2 = String::from("/\0");

    let res_format =
        String::from("posix_basename: %s \ngnu_basename: %s\n\0");

    unsafe {
        let res1 = posix_basename(path1.as_ptr() as *mut c_char);
        let res2 = gnu_basename(path2.as_ptr() as *const c_char);

        printf(res_format.as_ptr() as *mut c_char, res1, res2);
    }
}
$ cargo r -q
posix_basename: /
gnu_basename:
libc-test

libc-test will complain that both symbols are not found:D

$ cd libc-test
$ cargo t 
  cargo:warning=/home/steve/Documents/workspace/libc/target/debug/build/libc-test-592f01d15ee93e7a/out/main.c:45272:24: error: ‘posix_basename’ undeclared (first use in this
function)
  cargo:warning=45272 |                 return posix_basename;
  cargo:warning=      |                        ^~~~~~~~~~~~~~
  cargo:warning=/home/steve/Documents/workspace/libc/target/debug/build/libc-test-592f01d15ee93e7a/out/main.c:45272:24: note: each undeclared identifier is reported only once
 for each function it appears in
  cargo:warning=/home/steve/Documents/workspace/libc/target/debug/build/libc-test-592f01d15ee93e7a/out/main.c: In function ‘__test_fn_gnu_basename’:
  cargo:warning=/home/steve/Documents/workspace/libc/target/debug/build/libc-test-592f01d15ee93e7a/out/main.c:45277:24: error: ‘gnu_basename’ undeclared (first use in this fu
nction); did you mean ‘ns_samename’?
  cargo:warning=45277 |                 return gnu_basename;
  cargo:warning=      |                        ^~~~~~~~~~~~
  cargo:warning=      |                        ns_samename

@JohnTitor
Copy link
Member

JohnTitor commented Oct 24, 2022

If these are completely usable, I'm fine to accept your suggestion with skipping tests :)

@JohnTitor
Copy link
Member

Thanks! @bors r+

@bors
Copy link
Contributor

bors commented Oct 25, 2022

📌 Commit 5ffdbc6 has been approved by JohnTitor

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Oct 25, 2022

⌛ Testing commit 5ffdbc6 with merge a8b7b9c...

@bors
Copy link
Contributor

bors commented Oct 25, 2022

☀️ Test successful - checks-actions, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13, checks-cirrus-freebsd-14
Approved by: JohnTitor
Pushing a8b7b9c to master...

@bors bors merged commit a8b7b9c into rust-lang:master Oct 25, 2022
@bors bors mentioned this pull request Oct 25, 2022
@SteveLauC SteveLauC deleted the dirname-basename branch October 25, 2022 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants