marlersoft/zigwin32

Null pointer usage?

Closed this issue · 3 comments

I used WriteConsoleA but I'm sure many win32 functions have this problem.
I'm still learning ZIG. Correct me where I'm wrong.

My code example.

const console = @import("win32.zig").system.console;

pub fn main() anyerror!void {
    _ = console.WriteConsoleA(
        console.GetStdHandle(console.STD_OUTPUT_HANDLE),
        "All your codebase are belong to us.",
        "All your codebase are belong to us.".len,
        null,
        null);    // Should it be used like this?
}

WriteConsoleA function declaration.

pub extern "KERNEL32" fn WriteConsoleA(
    hConsoleOutput: HANDLE,
    lpBuffer: [*]const u8,
    nNumberOfCharsToWrite: u32,
    lpNumberOfCharsWritten: ?*u32,
    lpReserved: *c_void,
) callconv(@import("std").os.windows.WINAPI) BOOL;

I think lpReserved: *c_void, shold be ?*c_void to allow null values.
lpReserved parameter should always be null according to the official documentation.

I've been using the "Optional" SAL attribute to determine whether or not pointers can be NULL. However, the metadata seems to be missing this attribute in many places for pointers that can be NULL, including struct fields (see microsoft/win32metadata#523).

For now I've been adding "patches" to the metadata that add optional when it's missing (i.e. https://github.com/marlersoft/win32jsongen/blob/ca90c05775ae1c2eb009013683799278bd5ad62a/Generator/Patch.cs#L27)

I'm not sure how many of these optionals are actually missing though. In the meantime I can add a patch for this and I've opened an issue in the metadata for this particular case here: microsoft/win32metadata#567

By the way, based on the metadata repo's response that they won't be supporting data that indicates whether or not pointers can be null, I've reworked the Zig projection. The new approach is to make all pointers nullable by default. To declare a pointer as being "required" or "notnull", I've add a file named "notnull.json" where we can add function parameters (and probably struct fields later) that cannot be null. This approach means that the API will just work for projects, and if they want more type correctness we can add it later on. This is in contrast to the previous approach where certain functions/types were unusable for projects because they were unable to pass NULL to pointers that were supposed to accept it.

The commit to implement this change is here:
marlersoft/zigwin32gen@b63c76e

The corresponding difference in the bindings is here:
955e915

It only affected 117,526 lines...lol :)

With this I"ll consider the matter solved, let me know if you still have issues.