x/sys/windows: mkwinsyscall: Add support for DLLs with extensions other than .dll

[ranganath42]

What version of Go are you using (go version)?

$ go version
go version go1.20 windows/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
    :
GOARCH=amd64
GOOS=windows
    :

What did you do?

Some of the Windows system dynamic link library (DLL) files have extensions other than .dll.
For example,

• ActiveX Controls files with .ocx extension
• Control Panel applets with .cpl extension
• Device drivers with .drv extension

mkwinsyscall does not support generating the syscall wrappers for these DLLs with different extensions.
The extension is always hard-coded to .dll in the generated code.

//sys ClosePrinter(h syscall.Handle) (err error) = winspool.ClosePrinter

What did you expect to see?

var (
        modwinspool = windows.NewLazySystemDLL("winspool.drv")
        procClosePrinter = modwinspool.NewProc("ClosePrinter")
)

What did you see instead?

var (
        modwinspool = windows.NewLazySystemDLL("winspool.dll")
        procClosePrinter = modwinspool.NewProc("ClosePrinter")
)

Proposed solution

If the user specified the extension of the DLL in the //sys comment, use that extension instead of adding the .dll extension.
Going back to the previous example, the user can specify the extension as .drv in the //sys comment.

//sys ClosePrinter(h syscall.Handle) (err error) = winspool.drv.ClosePrinter

Then, the generated code will be,

var (
        modwinspool = windows.NewLazySystemDLL("winspool.drv")
        procClosePrinter = modwinspool.NewProc("ClosePrinter")
)

The code changes are fairly straightforward. I have working code that I can submit as a pull request if the proposal makes sense.

[ranganath42]

The downside of the proposed solution is that if the DLL name itself has a dot (for example, Windows.Networking.dll), then the user will need to specify the full DLL name including the .dll. extension. Here's the corresponding issue.

[bcmills]

(CC @golang/windows)

[ranganath42]

Anything else that I can provide here, team?

[qmuntal]

The proposed solution enters in conflict with what has been implemented in #57913 to support DLL names with . and -. It is difficult, if not impossible, to differentiate between a . that is part of the name or that is separating the name from the extension in the general case.

For example, if I had to use the method ClosePrinter from an (imaginary but legit) DLL whose file name is printer.drv.dll, I would use the following comment:

//sys ClosePrinter(h syscall.Handle) (err error) = printer.drv.ClosePrinter

Which would generate this:

  modprinter_drv = windows.NewLazySystemDLL("printer.drv.dll")
  procClosePrinter = modprinter_drv.NewProc("ClosePrinter")

With your solution, it could incorrectly generate the following:

  modprinter = windows.NewLazySystemDLL("printer.drv")
  procClosePrinter = modprinter.NewProc("ClosePrinter")

In summary, I would avoid overloading the DLL name in the //sys comment. An alternative approach would be to accept an additional optional parameter, similar to what we already do for custom error codes with [failretval==X]. It could look like this (using your example):

//sys ClosePrinter(h syscall.Handle) (err error) = winspool.ClosePrinter [ext=drv]

@dagood @alexbrainman

[dagood]

Yeah, removing ambiguity makes the most sense to me, and I feel like I should have explored that more rather than escaping . and -. 😄 I had an idea in #57913 that I like the look of (parallels with Go syntax!) but it seems to me would mean changing how the file is parsed/stored a fair amount:

//sysimport windowsnetworking "windows.networking.dll"
//sys	SetSocketMediaStreamingMode(...) (...) = windowsnetworking.SetSocketMediaStreamingMode
//sysimport winspool "winspool.drv"
//sys	ClosePrinter(h syscall.Handle) (err error) = winspool.ClosePrinter

Adding [ext=drv] seems to me like a reasonable minimal way to remove the ambiguity and fix this.

[ranganath42]

Agreed, a new optional parameter like [ext=drv] sounds like a good way to handle exceptions like these. I'll create a pull request.
Thanks for looking into this.

[alexbrainman]

Adding [ext=drv] seems to me like a reasonable minimal way to remove the ambiguity and fix this.

Agreed. It is unfortunate to complicate things, but I don't see any other way to solve this problem. Hopefully there are not many dll files with not dll extension, so this scenario will be pretty rare.

Alex

[ranganath42]

This pull request addresses the above issue.

[alexbrainman]

This pull request addresses the above issue.

Replied at

https://go-review.googlesource.com/c/sys/+/506697

Alex

[ranganath42]

@alexbrainman, thanks for the code review comments. It's been a while, and I didn't follow the contributor guidelines properly earlier (for example, I didn't use the git codereview tool).
I will decline my previous pull request and create a new one with the previous changes, plus address your suggestions.