fix(ffitemplate): clone input slices where necessary#171
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Go FFI driver template to avoid aliasing caller-owned C memory by cloning input slices before passing them into driver APIs, improving safety when the C side may reuse or free buffers after calls return.
Changes:
- Added
slicesusage and cloned byte slice inputs in several exported entrypoints (options, partition reads, Substrait plan, GetInfo codes). - Added/expanded “SAFETY” notes around
fromCArrcall sites to clarify when slices are used for writing vs. reading.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cdb := getFromHandle[cDatabase](db.private_data) | ||
| k := C.GoString(key) | ||
| v := fromCArr[byte](value, int(length)) | ||
| var safeLen int |
There was a problem hiding this comment.
Why not just use c.GoBytes(value, length)?
That would do the checks and create the copy, outputting a []byte for you and simplify this a bit.
| if safeLen, code = checkLengthToInt(length, err); code != C.ADBC_STATUS_OK { | ||
| return code | ||
| } | ||
| e := conn.cnxn.SetOptionBytes(conn.newContext(), C.GoString(key), slices.Clone(fromCArr[byte](value, safeLen))) |
There was a problem hiding this comment.
Same comment as above, just use c.GoBytes instead of slices.Clone(fromCArr....)
| if safeLen, code = checkLengthToInt(length, err); code != C.ADBC_STATUS_OK { | ||
| return code | ||
| } | ||
| e := st.stmt.SetSubstraitPlan(st.newContext(), slices.Clone(fromCArr[byte](plan, safeLen))) |
There was a problem hiding this comment.
And so on.... Not gonna comment at every call site
No description provided.