General guidelines for integer types (`int` / `uint` / `uint32` / `uint64`) in manual wrappers

[XuehaiPan]

Go is a strongly, statically typed language. It will not auto-cast values between integer types (int / uint / uint32 / uint64) on return.

The bindings auto-generated by c-for-go always have a suffix 32 or 64 of uint types. But some manual wrappers cast the type to int or uint before returning while some others do not.

1. The binding receives uint32, the wrapper returns int:

   https://github.com/NVIDIA/go-nvml/blob/053ac6826f4cbf14b5197cc36a472278662a20b3/gen/nvml/device.go#L24-L29

2. The wrapper receives int, the binding receives uint32:

   https://github.com/NVIDIA/go-nvml/blob/053ac6826f4cbf14b5197cc36a472278662a20b3/gen/nvml/device.go#L31-L36

3. The binding and the wrapper both use uint32:

   https://github.com/NVIDIA/go-nvml/blob/053ac6826f4cbf14b5197cc36a472278662a20b3/gen/nvml/device.go#L383-L388

4. Bug: the binding requires []uint32 but []uint is given:

   https://github.com/NVIDIA/go-nvml/blob/053ac6826f4cbf14b5197cc36a472278662a20b3/gen/nvml/device.go#L103-L109

[klueska]

The goal was to have it be int when the exact type doesn't matter (to make it a bit more usable within existing Go programs that use int everyhwere). And to be specific to uint32 and uint64 when it does matter. We should never be using uint.

[klueska]

Your (4.) is clearly a bug and uint32[] should be passed, but int[] should be returned.

I take it back, the reason it is being passed as a uint[] is because the underlying type is an unsigned long* which maps to uint[] in Go (which you can also see in the generated definition for this function):

func nvmlDeviceGetCpuAffinityWithinScope(Device Device, CpuSetSize uint32, CpuSet *uint, Scope AffinityScope) Return {
    cDevice, _ := *(*C.nvmlDevice_t)(unsafe.Pointer(&Device)), cgoAllocsUnknown
    cCpuSetSize, _ := (C.uint)(CpuSetSize), cgoAllocsUnknown
    cCpuSet, _ := (*C.ulong)(unsafe.Pointer(CpuSet)), cgoAllocsUnknown
    cScope, _ := (C.nvmlAffinityScope_t)(Scope), cgoAllocsUnknown
    __ret := C.nvmlDeviceGetCpuAffinityWithinScope(cDevice, cCpuSetSize, cCpuSet, cScope)
    __v := (Return)(__ret)
    return __v
}

[XuehaiPan]

the reason it is being passed as a uint[] is because the underlying type is an unsigned long* which maps to uint[] in Go (which you can also see in the generated definition for this function)

I regenerated the bindings with the latest version of c-for-go, It gives me 3 bugs like this and the tests will not pass. (unsigned int and unsigned long are the same type in x64 C)

Results with the latest c-for-go:

// nvmlDeviceGetCpuAffinityWithinScope function as declared in nvml/nvml.h
func nvmlDeviceGetCpuAffinityWithinScope(Device Device, CpuSetSize uint32, CpuSet *uint32, Scope AffinityScope) Return {
    cDevice, cDeviceAllocMap := *(*C.nvmlDevice_t)(unsafe.Pointer(&Device)), cgoAllocsUnknown
    cCpuSetSize, cCpuSetSizeAllocMap := (C.uint)(CpuSetSize), cgoAllocsUnknown
    cCpuSet, cCpuSetAllocMap := (*C.ulong)(unsafe.Pointer(CpuSet)), cgoAllocsUnknown
    cScope, cScopeAllocMap := (C.nvmlAffinityScope_t)(Scope), cgoAllocsUnknown
    __ret := C.nvmlDeviceGetCpuAffinityWithinScope(cDevice, cCpuSetSize, cCpuSet, cScope)
    runtime.KeepAlive(cScopeAllocMap)
    runtime.KeepAlive(cCpuSetAllocMap)
    runtime.KeepAlive(cCpuSetSizeAllocMap)
    runtime.KeepAlive(cDeviceAllocMap)
    __v := (Return)(__ret)
    return __v
}

Difference:

 // nvmlDeviceGetCpuAffinityWithinScope function as declared in nvml/nvml.h
-func nvmlDeviceGetCpuAffinityWithinScope(Device Device, CpuSetSize uint32, CpuSet *uint, Scope AffinityScope) Return {
-   cDevice, _ := *(*C.nvmlDevice_t)(unsafe.Pointer(&Device)), cgoAllocsUnknown
-   cCpuSetSize, _ := (C.uint)(CpuSetSize), cgoAllocsUnknown
-   cCpuSet, _ := (*C.ulong)(unsafe.Pointer(CpuSet)), cgoAllocsUnknown
-   cScope, _ := (C.nvmlAffinityScope_t)(Scope), cgoAllocsUnknown
+func nvmlDeviceGetCpuAffinityWithinScope(Device Device, CpuSetSize uint32, CpuSet *uint32, Scope AffinityScope) Return {
+   cDevice, cDeviceAllocMap := *(*C.nvmlDevice_t)(unsafe.Pointer(&Device)), cgoAllocsUnknown
+   cCpuSetSize, cCpuSetSizeAllocMap := (C.uint)(CpuSetSize), cgoAllocsUnknown
+   cCpuSet, cCpuSetAllocMap := (*C.ulong)(unsafe.Pointer(CpuSet)), cgoAllocsUnknown
+   cScope, cScopeAllocMap := (C.nvmlAffinityScope_t)(Scope), cgoAllocsUnknown
    __ret := C.nvmlDeviceGetCpuAffinityWithinScope(cDevice, cCpuSetSize, cCpuSet, cScope)
+   runtime.KeepAlive(cScopeAllocMap)
+   runtime.KeepAlive(cCpuSetAllocMap)
+   runtime.KeepAlive(cCpuSetSizeAllocMap)
+   runtime.KeepAlive(cDeviceAllocMap)
    __v := (Return)(__ret)
    return __v
 }

[elezar]

Yes, we noted that an update to c-for-go would generate different types. We pin to the version specified as we didn't have capacity to properly investigate these at the time.

[klueska]

The "newer" version generated by c-for-go actually looks incorrect because the underlying C API specifies the CpuSet as an unsigned long *. On a 32-bit system an unsigned long is 32 bits and on a 64-bit system an unsigned long is 64 bits. Likewise, in Go, on a 32-bit system a uint is 32 bits and on a 64-bit system a uint is 64 bits. Pinning this to always be 32 bits via a uint32 * seems incorrect to me.

[XuehaiPan]

The "newer" version generated by c-for-go actually looks incorrect because the underlying C API specifies the CpuSet as an unsigned long *.

@klueska You are correct, the latest c-for-go produces "wrong" types. unsigned long should be translated to uint or uint64 on x86_64 machines. But it works fine since C will auto-cast types when the C function receives arguments (where Golang will not).

Output on x86_64 machine:

$ cat test.c
#include <stdio.h>

int main()
{
    printf("sizeof(unsigned)           = %ld (%ld bits)\n", sizeof(unsigned), 8 * sizeof(unsigned));
    printf("sizeof(unsigned int)       = %ld (%ld bits)\n", sizeof(unsigned int), 8 * sizeof(unsigned int));
    printf("sizeof(unsigned long)      = %ld (%ld bits)\n", sizeof(unsigned long), 8 * sizeof(unsigned long));
    printf("sizeof(unsigned long int)  = %ld (%ld bits)\n", sizeof(unsigned long int), 8 * sizeof(unsigned long int));
    printf("sizeof(unsigned long long) = %ld (%ld bits)\n", sizeof(unsigned long long), 8 * sizeof(unsigned long long));
    return 0;
}

$ gcc test.c && ./a.out
sizeof(unsigned)           = 4 (32 bits)
sizeof(unsigned int)       = 4 (32 bits)
sizeof(unsigned long)      = 8 (64 bits)
sizeof(unsigned long int)  = 8 (64 bits)
sizeof(unsigned long long) = 8 (64 bits)

[XuehaiPan]

Related commit: https://github.com/xlab/c-for-go/commit/ddcfe2673053652b6566fa8fabe6d0966a6b4ed9.

[klueska]

@XuehaiPan can we close this issue since the overall concern/confusion has been addressed.