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

Common structs lack Default trait #286

Closed
raphaelcohn opened this issue May 12, 2016 · 3 comments
Closed

Common structs lack Default trait #286

raphaelcohn opened this issue May 12, 2016 · 3 comments

Comments

@raphaelcohn
Copy link
Contributor

Whilst implementing wrappers around sysctl, I had to create a mutable value on the stack which would be overwritten by sysctl. To make the wrapper useful, I wanted to make it generic in <T: Default>. For c_int, etc, this is fine, but structs like timeval this isn't. Rust's rules mean I can't impl the Default trait to structs defined in libc.

My workaround is to have two wrapper functions, one which takes <T> and has an extra user-supplied default, and one that takes <T: Default>.

A simple solution to this would be to have the s! macro generate a default trait for all structs like this. In the near term, however, would you be open to this sort of change? Or is it something you don't feel belongs in libc?

@raphaelcohn
Copy link
Contributor Author

To put some meat on the bones, so to speak, here's the impact:-

extern crate libc;
use self::libc::c_int;
use self::libc::c_void;
use self::libc::c_uint;
use self::libc::timeval;
use std::io::Error;
use std::io::Result;
use std::ptr;
use std::mem::size_of_val;

#[cfg(not(any(target_os = "linux", target_os = "android", target_os = "windows", target_os = "solaris")))]
pub fn maximum_number_of_processes() -> Result<c_int>
{
    sysctl_wrapper::<c_int>(self::libc::CTL_KERN, self::libc::KERN_MAXPROC)
}

#[cfg(not(any(target_os = "linux", target_os = "android", target_os = "windows", target_os = "solaris")))]
pub fn boot_time() -> Result<timeval>
{
    sysctl_wrapper_default::<timeval>(self::libc::CTL_KERN, self::libc::KERN_BOOTTIME, timeval {tv_sec: 0, tv_usec: 0})
}

#[cfg(not(any(target_os = "linux", target_os = "android", target_os = "windows", target_os = "solaris")))]
fn sysctl_wrapper<T: Default>(ctl: c_int, ctl_category: c_int) -> Result<T>
{
    let mut value: T = Default::default();
    sysctl_wrapper_default(ctl, ctl_category, value)
}

#[cfg(not(any(target_os = "linux", target_os = "android", target_os = "windows", target_os = "solaris")))]
fn sysctl_wrapper_default<T>(ctl: c_int, ctl_category: c_int, mut value: T) -> Result<T>
{
    // Optimisation 2: cache mem::size_of::<c_int>();
    let mut mib: [i32; 2] = [ctl, ctl_category];
    let mut size = size_of_val(&value);
    let pointer: *mut c_void = &mut value as *mut _ as *mut c_void;

    unsafe
    {
        match self::libc::sysctl(mib.as_mut_ptr() as *mut c_int, mib.len() as c_uint, pointer, &mut size as *mut usize, ptr::null_mut(), 0)
        {
            0 => Ok(value),
            -1 => Err(Error::last_os_error()),
            unexpected @ _ => panic!("Did not expect result code {}", unexpected),
        }
    }
}

@alexcrichton
Copy link
Member

This is indeed correct! The design of libc currently doesn't specify that the Default trait (or others like PartialEq and such) will be defined for all types. This helps cut down on compile time and binary size.

Could another wrapper be used or something like mem::zeroed?

@raphaelcohn
Copy link
Contributor Author

Right-ho; I understand the design rationale. Thanks for pointing me in the direction of mem::zeroed, from which I found mem::unitialized. This is exactly what's needed inside the sysctl_wrapper_default, and means I can eliminate a lot of noise.

danielverkamp pushed a commit to danielverkamp/libc that referenced this issue Apr 28, 2020
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

2 participants