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

Fix bug in dataVisitorDispatch #46

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chzhongsending
Copy link

@chzhongsending chzhongsending commented Aug 17, 2023

The obx_data_visitor in ObjectBox Core (C library) is:

/// The callback for reading data one-by-one
/// @param data is the read data buffer
/// @param size specifies the length of the read data
/// @param user_data is a pass-through argument passed to the called API
/// @return true to keep going, false to cancel.
typedef bool obx_data_visitor(const void* data, size_t size, void* user_data);

// ...

/// Walk over matching objects using the given data visitor
OBX_C_API obx_err obx_query_visit(OBX_query* query, obx_data_visitor* visitor, void* user_data);

The first two parameter should be data and size according to the C header file, the third parameter is the user data.
But objectbox-go defined dataVisitorDispatch function in Go has a wrong parameter order:

// datavisitor.go
var dataVisitor = (*C.obx_data_visitor)(unsafe.Pointer(C.dataVisitorDispatch))

// datavisitorc.go
//export dataVisitorDispatch
// This function finds the data visitor (based on the pointer to the visitorId) and calls it with the given data
// NOTE: don't change ptr contents, it's `const void*` in C but go doesn't support const pointers
func dataVisitorDispatch(visitorIdPtr unsafe.Pointer, data unsafe.Pointer, size C.size_t) C.bool {
// ...
}

visitorIdPtr was passed as user data here:

// Find returns all objects matching the query
func (query *Query) Find() (objects interface{}, err error) {
	defer runtime.KeepAlive(query)

	if err := query.check(); err != nil {
		return nil, err
	}

	const existingOnly = true
	if supportsResultArray {
		var cFn = func() *C.OBX_bytes_array {
			return C.obx_query_find(query.cQuery)
		}
		return query.box.readManyObjects(existingOnly, cFn)
	}

	var cFn = func(visitorArg unsafe.Pointer) C.obx_err {
                // visitorArg is passed as user data here
		return C.obx_query_visit(query.cQuery, dataVisitor, visitorArg)
	}
	return query.box.readUsingVisitor(existingOnly, cFn)
}

This might lead to a crash when you use a ObjectBox Core C version without OBXFeature_ResultArray (e.g. Mobile version like iOS).
Simply re-order the parameter will fix this issue:

func dataVisitorDispatch(data unsafe.Pointer, size C.size_t, visitorIdPtr unsafe.Pointer) C.bool

The obx_data_visitor in core is:

```c
typedef bool obx_data_visitor(const void* data, size_t size, void* user_data);
```

So the `dataVisitorDispatch` function in Go have wrong parameter order.
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

Successfully merging this pull request may close these issues.

1 participant