Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2002)

Issue 5272042: code review 5272042: syscall: dll function load and calling changes (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 5 months ago by brainman
Modified:
13 years, 5 months ago
Reviewers:
CC:
golang-dev, jp, rsc
Visibility:
Public.

Description

syscall: dll function load and calling changes New DLL and Proc types to manage and call dll functions. These were used to simplify syscall tests in runtime package. They were also used to implement LazyDLL and LazyProc. LazyProc, like Proc, now have Call function, that just a wrapper for SyscallN. It is not as efficient as Syscall, but easier to use. NewLazyDLL now supports non-ascii filenames. LazyDLL and LazyProc now have Load and Find methods. These can be used during runtime to discover if some dll functions are not present. All dll functions now return errors that fit os.Error interface. They also contain Windows error number. Some of these changes are suggested by jp.

Patch Set 1 #

Patch Set 2 : diff -r 21dd3105073c https://go.googlecode.com/hg/ #

Patch Set 3 : diff -r 21dd3105073c https://go.googlecode.com/hg/ #

Total comments: 14

Patch Set 4 : diff -r dd81822c18a9 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+305 lines, -137 lines) Patch
M src/pkg/runtime/syscall_windows_test.go View 1 3 chunks +36 lines, -57 lines 0 comments Download
M src/pkg/runtime/windows/os.h View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/runtime/windows/syscall.goc View 1 2 3 2 chunks +13 lines, -6 lines 0 comments Download
M src/pkg/runtime/windows/thread.c View 1 2 chunks +2 lines, -2 lines 0 comments Download
M src/pkg/syscall/Makefile View 1 1 chunk +1 line, -0 lines 0 comments Download
A src/pkg/syscall/dll_windows.go View 1 2 3 1 chunk +252 lines, -0 lines 0 comments Download
M src/pkg/syscall/syscall_windows.go View 1 2 chunks +0 lines, -71 lines 0 comments Download

Messages

Total messages: 4
brainman
Hello golang-dev@googlegroups.com (cc: jp@webmaster.ms), I'd like you to review this change to https://go.googlecode.com/hg/
13 years, 5 months ago (2011-10-13 12:25:16 UTC) #1
jp
http://codereview.appspot.com/5272042/diff/5001/src/pkg/runtime/windows/syscall.goc File src/pkg/runtime/windows/syscall.goc (right): http://codereview.appspot.com/5272042/diff/5001/src/pkg/runtime/windows/syscall.goc#newcode10 src/pkg/runtime/windows/syscall.goc:10: func loadlibrary(filename uintptr) (handle uintptr, err uintptr) { Maybe ...
13 years, 5 months ago (2011-10-13 13:08:31 UTC) #2
rsc
LGTM The sentences that say "Returns error..." should be "It returns an error...".
13 years, 5 months ago (2011-10-13 16:52:18 UTC) #3
brainman
13 years, 5 months ago (2011-10-15 06:30:05 UTC) #4
*** Submitted as http://code.google.com/p/go/source/detail?r=9e73a59f386c ***

syscall: dll function load and calling changes

New DLL and Proc types to manage and call dll functions. These were
used to simplify syscall tests in runtime package. They were also
used to implement LazyDLL and LazyProc.

LazyProc, like Proc, now have Call function, that just a wrapper for
SyscallN. It is not as efficient as Syscall, but easier to use.

NewLazyDLL now supports non-ascii filenames.

LazyDLL and LazyProc now have Load and Find methods. These can be used
during runtime to discover if some dll functions are not present.

All dll functions now return errors that fit os.Error interface. They
also contain Windows error number.

Some of these changes are suggested by jp.

R=golang-dev, jp, rsc
CC=golang-dev
http://codereview.appspot.com/5272042

http://codereview.appspot.com/5272042/diff/5001/src/pkg/runtime/windows/sysca...
File src/pkg/runtime/windows/syscall.goc (right):

http://codereview.appspot.com/5272042/diff/5001/src/pkg/runtime/windows/sysca...
src/pkg/runtime/windows/syscall.goc:10: func loadlibrary(filename uintptr)
(handle uintptr, err uintptr) {
On 2011/10/13 13:08:31, jp wrote:
> Maybe func loadlibrary(filename *uint16) ?
> It will remove 2 extra castings at the call point.

Done.

> 
> And returns (handle syscall.Handle, err syscall.Errno)
> Not sure if it possible in cgo to specify such a type.

I don't think it is possible too.

http://codereview.appspot.com/5272042/diff/5001/src/pkg/runtime/windows/sysca...
src/pkg/runtime/windows/syscall.goc:24: func getprocaddress(handle uintptr,
procname uintptr) (proc uintptr, err uintptr) {
On 2011/10/13 13:08:31, jp wrote:
> same: procname *uint8

Done.

http://codereview.appspot.com/5272042/diff/5001/src/pkg/syscall/dll_windows.go
File src/pkg/syscall/dll_windows.go (right):

http://codereview.appspot.com/5272042/diff/5001/src/pkg/syscall/dll_windows.g...
src/pkg/syscall/dll_windows.go:18: type Errno uint64
On 2011/10/13 13:08:31, jp wrote:
> uint32.
> Even on Win64.

I copied this from os package. I expect this to be temporary. We will change it
once os.Errno will move to syscall package. Then we could decide what the value
of it should be.

http://codereview.appspot.com/5272042/diff/5001/src/pkg/syscall/dll_windows.g...
src/pkg/syscall/dll_windows.go:48: if e != 0 {
On 2011/10/13 13:08:31, jp wrote:
> h, e := loadlibrary(StringToUTF16Ptr(name)) /* if loadlibrary would accept
> *uint16 */

Done.

http://codereview.appspot.com/5272042/diff/5001/src/pkg/syscall/dll_windows.g...
src/pkg/syscall/dll_windows.go:52: Msg:     "Failed to load " + name + ": " +
Errstr(int(e)),
On 2011/10/13 13:08:31, jp wrote:
> /* if loadlibrary would return syscall.Errno */

It would not.

http://codereview.appspot.com/5272042/diff/5001/src/pkg/syscall/dll_windows.g...
src/pkg/syscall/dll_windows.go:74: a, e := getprocaddress(uintptr(d.Handle),
uintptr(unsafe.Pointer(StringBytePtr(name))))
On 2011/10/13 13:08:31, jp wrote:
> a, e := getprocaddress(uintptr(d.Handle), StringBytePtr(name)) /* if
> getprocaddress would accept *uint8 */

Done.

http://codereview.appspot.com/5272042/diff/5001/src/pkg/syscall/dll_windows.g...
src/pkg/syscall/dll_windows.go:79: Msg:     "Failed to find " + name + "
procedure in " + d.Name + ": " + Errstr(int(e)),
On 2011/10/13 13:08:31, jp wrote:
> /* if getprocaddrress would return syscall.Errno */

It would not.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b