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

chore(ffi): Improve error buffer writing #276

Draft
wants to merge 23 commits into
base: main
Choose a base branch
from
Draft

Conversation

Oyami-Srk
Copy link
Member

This PR is meant to be merged after #263.

Oyami-Srk and others added 21 commits October 17, 2024 10:36
…ong with fields

The FFI interface introducing by this commit has corresponding Go wrapping:
```
func ValidateExpression(atc string, s *Schema) (bool, []string, int64) {
	atcC := unsafe.Pointer(C.CString(atc))
	defer C.free(atcC)

	errLen := C.ulong(1024)
	errBuf := [1024]C.uchar{}

	expr := C.expression_validate((*C.uchar)(atcC), s.s, &errBuf[0], &errLen)
	defer C.expression_validate_free_result(expr)

	if expr == nil {
		fmt.Println("Error: ", string(errBuf[:errLen]))
		return false, nil, 0
	}

	validate := bool(expr.validate)
	operators := int64(expr.operators)
	flds := make([]string, expr.fields_total)
	flds_slice := unsafe.Slice(expr.fields, expr.fields_total)

	for i := range flds {
		flds[i] = C.GoString((*C.char)(unsafe.Pointer(flds_slice[i])))
	}

	return validate, flds, operators
}
```
…Rust

An example go binding for the FFI interface introduced by this commit:
```Go
func ValidateExpression(atc string, s *Schema) (bool, []string, uint64, error) {
	atcC := unsafe.Pointer(C.CString(atc))
	defer C.free(atcC)

	errLen := C.ulong(1024)
	errBuf := [1024]C.uchar{}

	fieldsLen := C.ulong(1024)
	fieldsBuf := [1024]C.uchar{}
	fieldsTotal := C.ulong(0)
	operatorsC := C.uint64_t(0)

	result := C.expression_validate((*C.uchar)(atcC), s.s, &fieldsBuf[0], &fieldsLen, &fieldsTotal, &operatorsC, &errBuf[0], &errLen)

	if bool(result) == false {
		return false, nil, 0, fmt.Errorf(string(errBuf[:errLen]))
	}

	operators := uint64(operatorsC)

	flds := make([]string, uintptr(fieldsTotal))
	p := 0
	for i := range flds {
		flds[i] = C.GoString((*C.char)(unsafe.Pointer(&fieldsBuf[p])))
		p += len(flds[i]) + 1
	}

	return true, flds, operators, nil
}
```
…stead.

This commit contains two parts:
1. Moving everything guarded by feature flag `expr_validation` to `ffi.rs`.
2. Remove feature flag `expr_validation`.
3. Remove public interfaces `get_field` and `get_operator` for `Predicate`.

No internal logic has been changed.
remove the redundant error message when buffer is too small.
remove the ability to fetch fields count and total length without a valid `field_buf`. Must pass a valid `field_buf` if calling for getting fields.
Co-authored-by: Datong Sun <datong.sun@konghq.com>
@Oyami-Srk Oyami-Srk marked this pull request as draft November 6, 2024 07:10
Base automatically changed from KAG-5013 to main November 20, 2024 05:28
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