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

Mapping Option to nullable types #816

Closed
ghost opened this issue May 22, 2021 · 8 comments · Fixed by #1939
Closed

Mapping Option to nullable types #816

ghost opened this issue May 22, 2021 · 8 comments · Fixed by #1939
Labels
enhancement New feature or request

Comments

@ghost
Copy link

ghost commented May 22, 2021

As far as I'm aware, it isn't possible to reflect the nullable strings that are used in certain APIs with Option. For example, CreateProcessA takes many parameters that can be null to represent different configurations for creating the target process.

unsafe fn create_process(app: Option<&str>, cmd: Option<&str>, curdir: Option<&str>, ...) -> ... {
    let _ = CreateProcessA(
        app,
        cmd,
        ...,
        curdir,
        ...,
    );

    // ...
}

As you would expect, the None type would be implicitly converted to PSTR::NULL which I think might be useful when trying to write idiomatic code, especially libraries.

I think something like this should be fairly easy to implement as some string types seem to have hand-written implementation details.

Ideally, this is probably something that could be handled for every nullable type and not just PSTR and friends, right?

@ghost ghost closed this as completed May 22, 2021
@ghost ghost reopened this May 22, 2021
@ghost ghost changed the title Mapping `Option' to nullable string-types Mapping Option to nullable string-types May 22, 2021
@ghost
Copy link
Author

ghost commented May 22, 2021

Sorry for closing and then immediately reopening... had some struggles with my ability to use a computer.

@ghost
Copy link
Author

ghost commented May 22, 2021

I've just noticed #292, almost definitely related?

@ghost ghost changed the title Mapping Option to nullable string-types Mapping Option to nullable types May 22, 2021
@kennykerr
Copy link
Collaborator

Hi Daniel, #292 is specifically about WinRT reference parameters. For CreateProcessA there is some support for Option but more work is needed. Here's a complete example:

use bindings::Windows::Win32::System::Threading::*;

fn main() {
    unsafe {
        CreateProcessA(
            "c:\\windows\\notepad.exe",
            None,
            std::ptr::null_mut(),
            std::ptr::null_mut(),
            false,
            Default::default(),
            std::ptr::null_mut(),
            None,
            &mut STARTUPINFOA {
                cb: std::mem::size_of::<STARTUPINFOA>() as _,
                ..Default::default()
            },
            &mut PROCESS_INFORMATION::default(),
        );
    }
}

As you can see, the optional string parameters can be replaced with None. I plan to also support this for the remaining optional pointer parameters when I get a moment. There's also microsoft/win32metadata#433 to simplify the STARTUPINFOA initialization.

@kennykerr
Copy link
Collaborator

I'll keep this issue open to track the remaining usability issues. Ideally, we can write it more simply as follows:

CreateProcessA(
    "c:\\windows\\notepad.exe",
    None,
    None,
    None,
    false,
    Default::default(),
    None,
    None,
    &mut Default::default(),
    &mut Default::default(),
);

@kennykerr kennykerr added the enhancement New feature or request label May 24, 2021
@ghost
Copy link
Author

ghost commented May 24, 2021

Thanks for the information! I was too stuck on trying to find a way to map my Option<&str> to an Option<PSTR> which I then thought I could unwrap to a PSTR::NULL on empty strings but I wasn't getting anywhere. The fleshed out example looks really nice and I'm excited to see progress in the future. Thanks for all of the work you put into the project, it's appreciated very much!

@mtth-bfft
Copy link

Hi @danielframpton , @kennykerr ,

Manually unwrapping each Option<&str> works, but when writing safe wrapper around functions, every optional argument doubles the number of match arms or nested ifs, so it quickly gets ugly, e.g.

if let Some(base) = base {
    if let Some(filter) = filter {
        unsafe { ldap_searchW(..., base, filter, ....) }
    } else {
        unsafe { ldap_searchW(..., base, None, ....) }
    }
}
else {
    if let Some(filter) = filter {
        unsafe { ldap_searchW(..., None, filter, ....) }
    } else {
        unsafe { ldap_searchW(..., None, None, ....) }
    }
}

Did you manage to get a more generic solution working? Or did I miss a simpler solution? I'm still new to windows-rs, but do you need some help, maybe with something non-generic just to start with Option<PWSTR>?

@kennykerr kennykerr mentioned this issue Jun 10, 2022
@kennykerr
Copy link
Collaborator

The recent improvements to string and parameter bindings in 0.39.0 made these kinds of issues a lot less confusing. I'll close this issue for now, but feel free to open new issues for any remaining examples you come across.

@kennykerr
Copy link
Collaborator

On second thought, I was inspired to deal with a lot of the remaining pointer parameters and turn them into references (#1939) - this gets us much closer to ideal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants