Some questions regarding the implementation

[Saancreed]

Hello!

I've been looking for some NVAPI implementations for Wine recently and yours seems to be most feature–complete (or at least is the most actively maintained). So I had a look at the source code which raised a few questions regarding usage of nvidia-settings external binary for querying some attributes which are not supported by NVML yet:

1. I've noticed that about a year ago you removed the XNVCtrl–based queries in favor of mixed NVML/nvidia-settings solution. My question is: why is that? While I agree that NVML is the preferable choice whenever possible, using Xlib calls directly in nvapi should be both more elegant and have better performance than spawning external process which has to parse arguments (twice, because both sh and nvidia-settings need to do this separately), open its own connection to X display, execute the Xlib request, close the connection, format and print the result, and then read (and parse if not a string value) stdout of this process and only after all this return the result to caller. Perhaps even keeping the connection alive ourselves would improve performance some more than opening and closing it on every request (although I'm not sure how that with all the locking under the hood would compare to connecting when needed). Are there any issues I'm not aware of that made calling nvidia-settings -t -q Irq preferable solution to stuff like XNVCTRLQueryTargetAttribute(display, NV_CTRL_TARGET_TYPE_GPU, gpu_index, 0, NV_CTRL_IRQ, &irq)?
2. Have you tried to upstream at least some of your changes to wine-staging? If not, have you considered doing that in the future or does the licensing of NVML headers cause issues on this front?

In addition to that, I've noticed two things that seem incorrect (but I might be wrong here):

1. In DllMain, when attaching to the process and nvapi_init_nvml() call fails, shouldn't you return FALSE afterwards?
2. In nvapi_shutdown_nvml, you are calling nvmlShutdown() twice, which will probably confuse NVML's internal refcounter (and checks only the result of second call while reporting the result of the first one in error message).