NVIDIA / go-nvml

Go Bindings for the NVIDIA Management Library (NVML)
Apache License 2.0
290 stars 62 forks source link

Add fallback ErrorString function #99

Closed elezar closed 7 months ago

elezar commented 7 months ago

This change adds a fallback ErrorString function that is used when the nvmlErrorString function cannot be resolved. For example, when the library is not loaded.

Closes #82

klueska commented 7 months ago

Does this mimic what we did in go-nvlib?

elezar commented 7 months ago

Does this mimic what we did in go-nvlib?

The fallbackErrorString function is the same. The mechanics is different since I don't set a global function var but do a symbol lookup with every call. I thought that this was simpler since it didn't require state (the global) function pointer to be updated.

Note that when we originally implemented the logic in go-nvlib, we didn't have the Lookup function exposed at a package level here.

klueska commented 7 months ago

Seems reasonable. Given that we only end up calling this on error conditions, the extra bit of time to look up the symbol should be negligible.

yxxhero commented 6 months ago

@elezar @klueska

// nvml.ErrorString()
func ErrorString(r Return) string {
    if err := GetLibrary().Lookup("nvmlErrorString"); err == nil {
        return nvmlErrorString(r)
    }
    return fallbackErrorStringFunc(r)
}

I think that it should use err == nil.

elezar commented 6 months ago

Yes, you're right. Thanks for your PR. I think it would be better to invert the return statements.