Null-terminate go strings

[bkshrader]

If possible, I would like if the package would automatically append a null terminator where appropriate converting from go strings to c strings.

An example of the current behavior when creating a vulkan instance demonstrates my problem:

instanceExtensions := []string{
    vk.KhrSurfaceExtensionName, // Note - using constant defined by the package
}
instanceCreateInfo := &vk.InstanceCreateInfo{
    SType: vk.StructureTypeInstanceCreateInfo,
    PApplicationInfo: appInfo,
    EnabledExtensionCount: uint32(len(instanceExtensions)),
    PpEnabledExtensionNames: instanceExtensions,
}
err := vk.Error(vk.CreateInstance(instanceCreateInfo, nil, &v.instance)) // vulkan error: extension not present

I would like if using the constant vk.KhrSurfaceExtensionName or replacing it with its literal value "VK_KHR_surface" would both succeed without the need to append a null character, e.g. "VK_KHR_surface\x00", though I feel at a minimum the package constants should succeed without having to append the null-terminator at runtime.

This would also assist with GLFW integration, since GetRequiredInstanceExtensions() in the go-gl/glfw package does not return null-terminated values either, and would mitigate a current unexpected behavior if attempting to use vk.ToString() to test if an extension is supported, demonstrated below:

extensionCount := uint32(0)
vk.EnumerateInstanceExtensionProperties("", &extensionCount, nil)
supportedExtensions := make([]vk.ExtensionProperties, extensionCount)
vk.EnumerateInstanceExtensionProperties("", &extensionCount, supportedExtensions)

debugFound := false
for _, extProps := range supportedExtensions {
    if vk.ToString(extProps.ExtensionName == "VK_EXT_debug_utils" {
        // False positive: can cause runtime exception if used to test if extension is supported before calling CreateInstance
        debugFound = true
        break
    }

    if vk.ToString(extProps.ExtensionName) == "VK_EXT_debug_utils\x00" {
        // Unreachable since ToString() trims the null terminator
        debugFound = true
        break
    }
}

You can see that the above code will give the wrong result regardless of if the test string is null-terminated or not, causing the need to manually append a null-terminator to the test string after running this test or to write a custom ToString method.

[xlab]

@bkshrader I think Vulkan bindings used unsafe approach to avoid extra allocations (implicit). So you can have safe string only where you need that. Essentially, SafeStrings: false for this project.

This is something to consider when I have time to revisit this. Right now you'd better make a safe string helper. Sorry.

[bkshrader]

No worries. I know it's not critical, just something that would be nice to see. In the meantime, would you consider adding a safe string helper to the library, or at least adding a mention in the documentation that it needs to be done?

The null terminators are an extra step compared to writing vulkan code in c++, and the error message when leaving them out is misleading.

[neurlang]

I think that the check to see if the last character is 0 is pretty cheap. That way, adding null termination on this package side could be done any day without breaking existing users.

[xlab]

I think that the check to see if the last character is 0 is pretty cheap. That way, adding null termination on this package side could be done any day without breaking existing users.

Check is cheap, but actually appending 0 is an allocation. However I think we'd prefer UX there more that true performance.