Skip to content

Commit

Permalink
avoid cgoCheckPointer. ref golang/go#12416
Browse files Browse the repository at this point in the history
  • Loading branch information
mattn committed Dec 30, 2015
1 parent 5651a9d commit e969434
Showing 1 changed file with 16 additions and 2 deletions.
18 changes: 16 additions & 2 deletions sqlite3.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,20 @@ void _sqlite3_result_blob(sqlite3_context* ctx, const void* b, int l) {
sqlite3_result_blob(ctx, b, l, SQLITE_TRANSIENT);
}
int _sqlite3_create_function(
sqlite3 *db,
const char *zFunctionName,
int nArg,
int eTextRep,
uintptr_t pApp,
void (*xFunc)(sqlite3_context*,int,sqlite3_value**),
void (*xStep)(sqlite3_context*,int,sqlite3_value**),
void (*xFinal)(sqlite3_context*)
) {
return sqlite3_create_function(db, zFunctionName, nArg, eTextRep, (void*) pApp, xFunc, xStep, xFinal);
}
void callbackTrampoline(sqlite3_context*, int, sqlite3_value**);
void stepTrampoline(sqlite3_context*, int, sqlite3_value**);
void doneTrampoline(sqlite3_context*);
Expand Down Expand Up @@ -353,7 +367,7 @@ func (c *SQLiteConn) RegisterFunc(name string, impl interface{}, pure bool) erro
if pure {
opts |= C.SQLITE_DETERMINISTIC
}
rv := C.sqlite3_create_function(c.db, cname, C.int(numArgs), C.int(opts), unsafe.Pointer(&fi), (*[0]byte)(unsafe.Pointer(C.callbackTrampoline)), nil, nil)
rv := C._sqlite3_create_function(c.db, cname, C.int(numArgs), C.int(opts), C.uintptr_t(uintptr(unsafe.Pointer(&fi))), (*[0]byte)(unsafe.Pointer(C.callbackTrampoline)), nil, nil)
if rv != C.SQLITE_OK {
return c.lastError()
}
Expand Down Expand Up @@ -478,7 +492,7 @@ func (c *SQLiteConn) RegisterAggregator(name string, impl interface{}, pure bool
if pure {
opts |= C.SQLITE_DETERMINISTIC
}
rv := C.sqlite3_create_function(c.db, cname, C.int(stepNArgs), C.int(opts), unsafe.Pointer(&ai), nil, (*[0]byte)(unsafe.Pointer(C.stepTrampoline)), (*[0]byte)(unsafe.Pointer(C.doneTrampoline)))
rv := C._sqlite3_create_function(c.db, cname, C.int(stepNArgs), C.int(opts), C.uintptr_t(uintptr(unsafe.Pointer(&ai))), nil, (*[0]byte)(unsafe.Pointer(C.stepTrampoline)), (*[0]byte)(unsafe.Pointer(C.doneTrampoline)))
if rv != C.SQLITE_OK {
return c.lastError()
}
Expand Down

3 comments on commit e969434

@ianlancetaylor
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach is unsafe. You are passing a pointer to C as a uintptr, where that pointer contains other Go pointers. That breaks the Go 1.6 pointer passing rules (http://tip.golang.org/cmd/cgo/#hdr-Passing_pointers) and also breaks the new documentation for when it is safe to convert from unsafe.Pointer to uintptr (http://tip.golang.org/pkg/unsafe/#Pointer). This code will very likely break in a future Go release.

The only safe way to implement this kind of thing is to add a handle on the Go side.

@mattn
Copy link
Owner Author

@mattn mattn commented on e969434 Jan 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean that I should pass the argument as dummy of pointer, for example index of array of go pointers? I don't get understanding clearly. If you are okay, could you please give me your patch?

@ianlancetaylor
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I sent a pull request at #268 which shows the approach I mean.

Please sign in to comment.