Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support IADsNameTranslate #8

Open
ItsMattL opened this issue Sep 17, 2021 · 4 comments
Open

Support IADsNameTranslate #8

ItsMattL opened this issue Sep 17, 2021 · 4 comments

Comments

@ItsMattL
Copy link
Contributor

Add support for the NameTranslate (aka IADsNameTranslate) interface.

@ItsMattL
Copy link
Contributor Author

I spent some time on this today [1], but something is broken and I'm not sure what. Specifically, Set is failing with an obscure "Unspecified error" message.

The issue seems to be something relating to the comutil package (or at least, my limited understanding of how to use it). I tried to copy the other IDispatch interfaces in the API, but clearly missed something. To rule out problems with my environment, I implemented the same interface using oleutil and it worked like a charm, so the problem's in the implementation, not my machine or environment. Unfortunately the oleutil code doesn't really blend with the way the rest of the api package is written, so I wasn't sure it would be accepted as a pull request.

  1. https://github.com/ItsMattL/adsi/blob/translate/api/iadsnametranslate_windows.go

@JoshuaSjoding
Copy link
Collaborator

It looks like your virtual function address table isn't quite right:

type IADsNameTranslateVtbl struct {
	ole.IDispatchVtbl
	Get    uintptr
	GetEx  uintptr
	Init   uintptr
	InitEx uintptr
	Set    uintptr
	SetEx  uintptr
}

The Microsoft documentation lists these functions in alphabetical order, not in the order they're defined in the COM virtual function table structure. This has been a pet peeve of mine for a very long time. If the official documentation actually states what the order of these functions should be, I've never found it.

Note that getter and setter property methods like this one also take up an entry in the virtual function table.

In the past, I've always had to look at source code from other language bindings (Python, Perl, Rust, etc.) to find out what the correct order actually should be. In this particular case, it looks like this is the order you're looking for (after ole.IDispatchVtbl):

  • Put_ChaseReferral
  • Init
  • InitEx
  • Set
  • Get
  • SetEx
  • GetEx

@JoshuaSjoding
Copy link
Collaborator

Also, I think I would probably move all of the constants from api/iadsnametranslate.go to the api/constants.go file.

@JoshuaSjoding
Copy link
Collaborator

Support has been merged in with #9. Thanks! Below are thoughts on possible future improvements.

I have some lingering curiosity about whether the Go-facing calls in the adsi package (not the api package) could be reworked to be a little more friendly. Having to make three function calls is a bit of a nuisance and I'm not sure what benefit separating out the Set and Get calls actually provides. Perhaps those should be made into one function that calls both? I'm not very familiar with NameTranslate usage though, so maybe there's a good reason the COM api is set up the way it is.

I also wonder if strongly typed enumerations should be used in the adsi package for initType, formatType and setType instead of uint32.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants