Converting a [string] to a **c_char is subtlely difficult

[huonw]

It's currently non-trivial to do a &[&str] -> **c_char conversion correctly; I think there should be a function in std::c_str that handles this (as well as optionally null terminating the **c_char).

v is a &[&str], f is a function taking a **c_char. I've put the types of intermediates for clarity.

Incorrect

a

v.as_imm_buf(|buf: *&str, _| f(buf as **c_char))

The *&str → **c_char cast is wrong.

b

let ptrs: ~[*c_char] = v.map(|s: & &str| s.with_c_str(|cstr: *c_char| cstr));
ptrs.as_imm_buf(|buf: **c_char, _| f(buf))

The *c_char from with_c_str shouldn't leave the closure, since it with_c_str allocates an internal buffer. (This is a general problem with methods that pass a raw pointer to a closure: it is extremely easy and tempting to just return that pointer immediately and thus (possibly) create a dangling reference.)

Correct

(at least, I think it is correct)

let c_strs: ~[CString] = v.map(|s: & &str| s.to_c_str());
let mut ptrs: ~[*c_char] = c_strs.map(|c: &CString| c.with_ref(|ptr| ptr));
if null_terminate {
    ptrs.push(std::ptr::null());
}
ptrs.as_imm_buf(|buf: **c_char, _| f(buf))

The c_strs vector must live at least as at least as long as the **c_char is used. In this (unusual) case, returning the *c_char from with_ref is ok (if c_strs is kept alive long enough).

[erickt]

The simplest thing to do would be to add a function to std::c_str to implement this pattern. It wouldn't protect against someone from returning an interior pointer (#7694), but it would be simple enough that most people should reach for it first. Here's an untested example implementation:

trait WithCStrs {
    fn with_c_strs<T>(&self, null_terminated: bool, f: &fn(**libc::c_char) -> T) -> T;
}

impl<'self, T: Str> WithCStrs for &'self [T] {
    fn with_c_strs<T: Str, U>(&self, null_terminate: bool, f: &fn(**libc::c_char) -> U) -> U {
        let c_strs: ~[CString] = self.map(|s: & &str| s.to_c_str());
        let mut ptrs: ~[*c_char] = c_strs.map(|c: &CString| c.with_ref(|ptr| ptr));
        if null_terminate {
            ptrs.push(std::ptr::null());
        }
        ptrs.as_imm_buf(|buf: **c_char, _| f(buf))
    }
}

[lilyball]

@erickt I agree. I'm not worried about someone returning an interior pointer though, because they can't dereference it without unsafe code.

[huonw]

Updated (although missing some of the bells and whistles):

fn with_c_strings<T: ToCStr, U>(x: &[T], f: |*const *const libc::c_char| -> U) -> U {
    let c_strings: Vec<CString> = x.iter().map(|s| s.to_c_str()).collect();
    let mut ptrs: Vec<*const libc::c_char> = c_strings.iter().map(|cs| cs.as_ptr()).collect();

    ptrs.push(ptr::null());

    f(ptrs.as_ptr())
}

[lilyball]

@huonw In the interests of avoiding a reallocation on the ptrs vector when pushing the null pointer, I would suggest the following:

pub fn with_c_strings<T: ToCStr, U>(x: &[T], f: |*const *const libc::c_char| -> U) -> U {
    let c_strings: Vec<CString> = x.iter().map(|s| s.to_c_str()).collect();
    let ptrs = c_strings.iter().map(|cs| cs.as_ptr())
                        .chain(Some(ptr::null::<libc::c_char>()).move_iter())
                        .collect::<Vec<*const libc::c_char>>();

    f(ptrs.as_ptr())
}

[tamird]

This has some complications in today's rust - it would look like this:

#![feature(libc)]

extern crate libc;

use std::ptr;
use std::ffi::CString;

pub fn with_c_strings<T: Copy+Into<Vec<u8>>, U, F>(ts: &[T], f: F) -> U where
    F: FnOnce(*const *const libc::c_char) -> U {
    let c_strs: Vec<_> = ts.iter().map(|&t| {
        CString::new(t).unwrap()
    }).collect();
    let ptrs: Vec<_> = c_strs.iter().map(|c_str| c_str.as_ptr())
                     .chain(Some(ptr::null()))
                     .collect();

    f(ptrs.as_ptr())
}

Unfortunately CString::new() has two unaccounted-for-in-this-discussion features:

• it returns a result (failure if the argument passed in contains a null byte)
• it mutates the argument by appending a null byte (meaning the argument must be copied)

I'm not sure there's a sane way to address this issue without violating the static guarantee of null-termination.

[steveklabnik]

Triage: this is still a papercut, but not enough of one for someone to have commented in almost two years. As such, I'm going to give this a close. If someone wants to write a library function to make this easier, please do so!