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 a trait for non-path C string? #2597

Open
SteveLauC opened this issue Feb 2, 2025 · 1 comment
Open

Add a trait for non-path C string? #2597

SteveLauC opened this issue Feb 2, 2025 · 1 comment

Comments

@SteveLauC
Copy link
Member

I am thinking about adding another trait to represent string, the definition will be same as our NixPath trait, and these 2 traits only differ in semantics, NixPath is for path, this new trait is for string.

By adding this trait, we split "C string" (C does not have a string type, it is only a sequence of chars followed by a NUL) into 2 categories by semantics:

  1. path
  2. string

Currently, it will be used in 2 interfaces if added:

  1. The ident argument of openlog() on non-Linux systems

    nix/src/syslog.rs

    Lines 45 to 47 in 0c40b8d

    #[cfg(not(target_os = "linux"))]
    pub fn openlog<S: AsRef<OsStr> + ?Sized>(
    ident: Option<&S>,

    From man page:

    The string pointed to by ident is prepended to every message, and is typically set to the program name.

  2. The path argument of posix_spawnp()

    nix/src/spawn.rs

    Lines 396 to 397 in 0c40b8d

    pub fn posix_spawnp<SA: AsRef<CStr>, SE: AsRef<CStr>>(
    path: &CStr,

    It is the name of the program that the spawned child process will execute

    This argument should not be called path, I will rename it to file


Alternative approaches

  1. Just use the NixPath trait

    It will definitely work, though it will feel weird semantically due to the Path in its name. Or maybe we can rename the trait to something like NixCString?

  2. Use S: AsRef<OsStr>

    openlog() currently uses this approach, after converting it to &OsStr we can use NixPath under the hood, so it is convenient:

    <OsStr as NixPath>::with_nix_path(os_str.as_ref(), |c_str| {
        // do something with c_str
    });

    The only flaw of this method IMHO is that users cannot use &CStr because CStr is not AsRef<OsStr>, but &CStr should be allowed.

@SteveLauC
Copy link
Member Author

cc @asomers, thoughts on this?

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

No branches or pull requests

1 participant