I'm unsure why this refactoring is necessary. It looks like var_os(key) wraps around env_imp::getenv(key) and does what you're doing (or what you want to do) with match cases:

/// Doc comments omitted
#[stable(feature = "env", since = "1.0.0")]
pub fn var<K: AsRef<OsStr>>(key: K) -> Result<String, VarError> {
    _var(key.as_ref())
}

fn _var(key: &OsStr) -> Result<String, VarError> {
    match var_os(key) {
        Some(s) => s.into_string().map_err(VarError::NotUnicode),
        None => Err(VarError::NotPresent),
    }
}

/// Doc comments omitted
#[must_use]
#[stable(feature = "env", since = "1.0.0")]
pub fn var_os<K: AsRef<OsStr>>(key: K) -> Option<OsString> {
    _var_os(key.as_ref())
}

fn _var_os(key: &OsStr) -> Option<OsString> {
    env_imp::getenv(key)
}

Although, I'll admit, I'm curious on about the separate _var_os() and _var() functions here; I'm not noticing anywhere else _var_os() and _var() is used, so it makes me wonder why not just put what's inside _var_os()/_var() inside var_os()/var()?

The split exists to minimize the size of the generic versions.

Oh got it, that makes sense to me!

I'm unsure why this refactoring is necessary.

Primarily to avoid: var -> _var -> -> var_os -> _var_os -> env_imp::getenv

When we can do: var -> _var -> env_imp::getenv

Replacing match with combinators because I thought it read a bit nicer

Yea I can definitely see the layers of function indirection here; I'm wondering if the compiler would optimize this away though and inline it?

Still, I'm with @hkBst on this part:

not a fan of using env_imp::getenv directly here, when this is clearly function that does a small modification to the output of var_os

I'm not a fan of using env_imp::getenv directly here, when this is clearly function that does a small modification to the output of var_os. I suppose _var_os might be okay, but only if there was a benefit.

Not sure why env_imp::getenv is an issue, it’s one less indirection, avoids unnecessary generic var_os calls, and _var_os seems like it should be an inner function to var_os.

Not sure why env_imp::getenv is an issue, it’s one less indirection,

env_imp::getenv seems like an implementation detail, that's wrapped by (_)var_os, so we should probably prefer to use the wrapper when possible.

avoids unnecessary generic var_os calls,

I don't think there is any cost to this, since the compiler can easily inline these simple calls.

and _var_os seems like it should be an inner function to var_os.

Yes, that may well make sense, but is not what you implemented.

OK I've reimplemented var to use an inner function (to minimize the size of the generic versions) and removed the pointless _var_os function.

You've not addressed: "env_imp::getenv seems like an implementation detail, that's wrapped by (_)var_os, so we should probably prefer to use the wrapper when possible."

Not sure why env_imp::getenv is an issue, it’s one less indirection,

env_imp::getenv seems like an implementation detail, that's wrapped by (_)var_os, so we should probably prefer to use the wrapper when possible.

avoids unnecessary generic var_os calls,

I don't think there is any cost to this, since the compiler can easily inline these simple calls.

and _var_os seems like it should be an inner function to var_os.

Yes, that may well make sense, but is not what you implemented.