code review 218042: syscall: minimal mingw version of syscall to call windo...

syscall: minimal mingw version of syscall to call windows dlls

lots of missing parts, but builds and can call dlls, see a sample code in syscall_mingw.go

[brainman, 2010-02-22 00:20:57]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[rsc, 2010-02-22 23:17:26]

looks like a reasonable start, thanks.

http://codereview.appspot.com/218042/diff/1001/1003
File src/pkg/runtime/mingw/syscall.go (right):

http://codereview.appspot.com/218042/diff/1001/1003#newcode7
src/pkg/runtime/mingw/syscall.go:7: // dll helpers
this comment will show up in the documentation
for LoadLibrary.  

LoadLibrary and GetProcAddress are public.
Can they be made private?

http://codereview.appspot.com/218042/diff/1001/1004
File src/pkg/runtime/mingw/thread.c (right):

http://codereview.appspot.com/218042/diff/1001/1004#newcode83
src/pkg/runtime/mingw/thread.c:83: ·LoadLibrary(void *filename, uint32 h)
These are for syscall, right?
You can instead add them to a separate .cgo file
that says it is in package syscall, and then they
can be lowercase.
Using cgo would help avoid the *&h stuff too.

http://codereview.appspot.com/218042/diff/1001/1004#newcode86
src/pkg/runtime/mingw/thread.c:86: *&h = (uint32)stdcall_raw(LoadLibraryEx,
filename, 0, 0);
Why enter + raw + exit instead of the standard stdcall?

http://codereview.appspot.com/218042/diff/1001/1004#newcode95
src/pkg/runtime/mingw/thread.c:95: *&fp = (void *)stdcall_raw(GetProcAddress, m,
name);
Same question

http://codereview.appspot.com/218042/diff/1001/1007
File src/pkg/syscall/syscall_mingw.go (right):

http://codereview.appspot.com/218042/diff/1001/1007#newcode56
src/pkg/syscall/syscall_mingw.go:56: //sys	LoadLibraryA(libname string) (handle
HMODULE, errno int)
add blank line so that this doesn't show up as the LoadLibrary doc comment.

http://codereview.appspot.com/218042/diff/1001/1012
File src/pkg/syscall/ztypes_mingw_386.go (right):

http://codereview.appspot.com/218042/diff/1001/1012#newcode38
src/pkg/syscall/ztypes_mingw_386.go:38: type DWORD uint32
These names are pretty ugly.
Are they necessary?

[brainman, 2010-02-23 05:23:04]

http://codereview.appspot.com/218042/diff/1001/1003
File src/pkg/runtime/mingw/syscall.go (right):

http://codereview.appspot.com/218042/diff/1001/1003#newcode7
src/pkg/runtime/mingw/syscall.go:7: // dll helpers
Will get rid of that file altogether.

http://codereview.appspot.com/218042/diff/1001/1004
File src/pkg/runtime/mingw/thread.c (right):

http://codereview.appspot.com/218042/diff/1001/1004#newcode83
src/pkg/runtime/mingw/thread.c:83: ·LoadLibrary(void *filename, uint32 h)
> These are for syscall, right?

Yes.

> You can instead add them to a separate .cgo file
> that says it is in package syscall, and then they
> can be lowercase.
> Using cgo would help avoid the *&h stuff too.
> 

You should have told me about it earlier <g>. Will change.

http://codereview.appspot.com/218042/diff/1001/1004#newcode86
src/pkg/runtime/mingw/thread.c:86: *&h = (uint32)stdcall_raw(LoadLibraryEx,
filename, 0, 0);
On 2010/02/22 23:17:26, rsc wrote:
> Why enter + raw + exit instead of the standard stdcall?
> 

Will use stdcall instead, but inside new cgo file.

http://codereview.appspot.com/218042/diff/1001/1004#newcode95
src/pkg/runtime/mingw/thread.c:95: *&fp = (void *)stdcall_raw(GetProcAddress, m,
name);
On 2010/02/22 23:17:26, rsc wrote:
> Same question
> 

Same answer.

http://codereview.appspot.com/218042/diff/1001/1007
File src/pkg/syscall/syscall_mingw.go (right):

http://codereview.appspot.com/218042/diff/1001/1007#newcode56
src/pkg/syscall/syscall_mingw.go:56: //sys	LoadLibraryA(libname string) (handle
HMODULE, errno int)
On 2010/02/22 23:17:26, rsc wrote:
> add blank line so that this doesn't show up as the LoadLibrary doc comment.
> 

Done.

http://codereview.appspot.com/218042/diff/1001/1012
File src/pkg/syscall/ztypes_mingw_386.go (right):

http://codereview.appspot.com/218042/diff/1001/1012#newcode38
src/pkg/syscall/ztypes_mingw_386.go:38: type DWORD uint32
On 2010/02/22 23:17:26, rsc wrote:
> These names are pretty ugly.

That is what Microsoft call them.

> Are they necessary?

I don't know yet. Some syscall function prototypes will use them, some
structures too. We could use uint32 and like instead. But it might simplify some
future windows ports, like 64 bit windows (that I know nothing of).

What do you think?

[rsc, 2010-02-23 06:32:36]

> I don't know yet. Some syscall function prototypes will use them, some
> structures too. We could use uint32 and like instead. But it might
> simplify some future windows ports, like 64 bit windows (that I know
> nothing of).
>
> What do you think?

I don't mind HMODULE as much,
but DWORD should just be uint32.
And BOOL would be nice as bool,
but if it's really a uint32 then that's
not okay.

Maybe

type Bool uint32
type Module uint32

and leave the others out?

Russ

[brainman, 2010-02-23 06:36:44]

> I don't mind HMODULE as much,
> but DWORD should just be uint32.
> And BOOL would be nice as bool,
> but if it's really a uint32 then that's
> not okay.
> 
> Maybe
> 
> type Bool uint32
> type Module uint32
> 
> and leave the others out?
> 

I don't think it is that important at the moment.

I will make the changes and send you a new review.

Alex

[brainman, 2010-02-24 00:21:54]

Hello rsc (cc: golang-dev@googlegroups.com),

Please take another look.

[brainman, 2010-03-03 05:52:36]

On 2010/02/23 06:32:36, rsc wrote:
> 
> and leave the others out?
> 

Did you have a chance to look at the latest code?

Alex

[rsc, 2010-03-03 23:28:20]

> Did you have a chance to look at the latest code?

Not yet.  Still digging out of a code review pile...

[rsc, 2010-03-09 02:19:02]

LGTM

[rsc, 2010-03-09 02:21:00]

On 2010/03/09 02:19:02, rsc wrote:
> LGTM

This doesn't apply cleanly because some of the
files you started with have changed formatting
slightly.  Could you please run "hg sync" and
then "hg mail 218042" to upload new diffs against
the current head of the tree?

Thanks.

[brainman, 2010-03-09 02:24:31]

Hello rsc (cc: golang-dev@googlegroups.com),

Please take another look.

[rsc, 2010-03-09 23:09:12]

*** Submitted as http://code.google.com/p/go/source/detail?r=686f60fda473 ***

syscall: minimal mingw version of syscall to call windows dlls

lots of missing parts, but builds and can call dlls, see a sample code in
syscall_mingw.go

R=rsc
CC=golang-dev
http://codereview.appspot.com/218042

Committer: Russ Cox <rsc@golang.org>