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.

Sign up for GitHub

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

Jump to bottom

Issue 11650: Process::new and dynamic_library::open_external should take &[u8]s, not Paths or ~strs #13954

Merged
merged 3 commits into from
May 15, 2014
Merged

Issue 11650: Process::new and dynamic_library::open_external should take &[u8]s, not Paths or ~strs #13954

merged 3 commits into from
May 15, 2014

Commits on May 15, 2014

  1. Add ToCStr impl for &Path and StrBuf 

    This is a stopgap until DST (rust-lang#12938) lands.

Until DST lands, we cannot decompose &str into & and str, so we cannot
usefully take ToCStr arguments by reference (without forcing an
additional & around &str). So we are instead temporarily adding an
instance for &Path and StrBuf, so that we can take ToCStr as owned. When
DST lands, the &Path instance should be removed, the string instances
should be revisted, and arguments bound by ToCStr should be passed by
reference.

FIXMEs have been added accordingly.
    @aturon
    aturon committed May 15, 2014
    Configuration menu
    Copy the full SHA
    8f9cbe0 View commit details
    Browse the repository at this point in the history

  2. Process::new etc should support non-utf8 commands/args 

    The existing APIs for spawning processes took strings for the command
and arguments, but the underlying system may not impose utf8 encoding,
so this is overly limiting.

The assumption we actually want to make is just that the command and
arguments are viewable as [u8] slices with no interior NULLs, i.e., as
CStrings. The ToCStr trait is a handy bound for types that meet this
requirement (such as &str and Path).

However, since the commands and arguments are often a mixture of
strings and paths, it would be inconvenient to take a slice with a
single T: ToCStr bound. So this patch revamps the process creation API
to instead use a builder-style interface, called `Command`, allowing
arguments to be added one at a time with differing ToCStr
implementations for each.

The initial cut of the builder API has some drawbacks that can be
addressed once issue rust-lang#13851 (libstd as a facade) is closed. These are
detailed as FIXMEs.

Closes rust-lang#11650.

[breaking-change]
    @aturon
    aturon committed May 15, 2014
    Configuration menu
    Copy the full SHA
    046062d View commit details
    Browse the repository at this point in the history

  3. Change dynamic_library::open_external to take ToCStr 

    `std::unstable::dynamic_library::open_external` currently takes a
`Path`, but because `Paths` produce normalized strings, this can
change the semantics of lookups in a given environment. This patch
generalizes the function to take a `ToCStr`-bounded type, which
includes both `Path`s and `str`s.

Closes rust-lang#11650.
    @aturon
    aturon committed May 15, 2014
    5 Configuration menu
    Copy the full SHA
    e71202a View commit details
    Browse the repository at this point in the history