Skip to content

Conversation

@andrew-d
Copy link
Member

@andrew-d andrew-d commented Jan 7, 2026

When enabled, this ensures that sql.RawBytes values do not actually contain pointers to memory owned by SQLite.

Updates tailscale/corp#35671

@andrew-d andrew-d requested review from bradfitz and raggi January 7, 2026 17:08
@andrew-d andrew-d force-pushed the andrew/raw-bytes-copy branch from 7034e6c to 58e2090 Compare January 7, 2026 17:52
}
n := int(C.sqlite3_column_bytes(stmt.stmt, C.int(col)))
return unsafe.Slice((*byte)(unsafe.Pointer(res)), n)
slice := unsafe.Slice((*byte)(unsafe.Pointer(res)), n)
Copy link
Member

Choose a reason for hiding this comment

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

I'm trying to remember/confirm what unsafe.Slice's behavior is when n == 0.

Maybe we don't want that pointer in the slice header even a 0 len and 0 cap?

Worth a if n == 0 { return nil } above this?

Oh, no, that'd change it to map to a NULL column, huh?

So I guess this is correct.

(sorry, thinking aloud)

@bradfitz
Copy link
Member

bradfitz commented Jan 7, 2026

Want to add some tests that verify basic stuff still works with this enabled?

When enabled, this ensures that sql.RawBytes values do not actually
contain pointers to memory owned by SQLite.

Also add some tests that verify the behaviour of ColumnBlob, both with
and without this flag set.

Updates tailscale/corp#35671

Signed-off-by: Andrew Dunham <andrew@tailscale.com>
@andrew-d andrew-d force-pushed the andrew/raw-bytes-copy branch from 58e2090 to ca57704 Compare January 7, 2026 20:09
@andrew-d
Copy link
Member Author

andrew-d commented Jan 7, 2026

Want to add some tests that verify basic stuff still works with this enabled?

Just added a basic test case; PTAL?

Copy link
Member

@bradfitz bradfitz left a comment

Choose a reason for hiding this comment

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

Thanks!

@andrew-d andrew-d merged commit e28eaf5 into main Jan 7, 2026
2 checks passed
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.

2 participants