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

Handle zero-length Span inputs for sqlite3_bind_text* #558

Closed
wants to merge 1 commit into from
Closed

Handle zero-length Span inputs for sqlite3_bind_text* #558

wants to merge 1 commit into from

Conversation

nil4
Copy link

@nil4 nil4 commented Oct 9, 2023

Same as sqlite3_bind_blob, the SQLite C APIs sqlite3_bind_text* require the data pointer argument to be non-null to actually accept zero-length input. Otherwise, when a null data pointer is passed, the explicit length is ignored and a SQLITE_NULL value is bound instead.

This PR updates sqlite3_bind_text and sqlite3_bind_text16 to handle zero-length ReadOnlySpans, by substituting a valid pointer for the zero-length case.

The generated sqlite3_bind_blob code already handled this scenario by allocating a new byte[] for each call e.g. at:

// passing a zero-length blob to sqlite3_bind_blob() requires
// a non-null pointer, even though conceptually, that pointer
// point to zero things, ie nothing.
var ba_fake = new byte[] { 42 };
ReadOnlySpan<byte> span_fake = ba_fake;
fixed (byte* p_fake = span_fake)
{
return NativeMethods.sqlite3_bind_blob(stm, paramIndex, p_fake, 0, new IntPtr(-1));
}

This PR proposes a potentially lower-overhead solution (passing the address of a local) for both sqlite3_bind_text* and sqlite3_bind_blob, as suggested by Span<T> usage Rule #9:

In the previous example, pbData can be null if, for example, the input span is empty. If the exported method absolutely requires that pbData be non-null, even if cbData is 0, the method can be implemented as follows

These changes address the sqlite3_bind_text* functions behavior when called with empty ReadOnlySpan.

Fixes #557

Same as `sqlite3_bind_blob`, the SQLite C APIs `sqlite3_bind_text**` require the data pointer argument to be non-null to really accept zero-length input.

If a null data pointer is passed, the explicit length is ignored and a `SQLITE_NULL` value is used instead.

This commit updates `sqlite3_bind_text` and `sqlite3_bind_text16` to handle zero-length `ReadOnlySpan`s by substituting a valid pointer in the zero-length case.

`sqlite3_bind_blob` already  handled this case by allocating a new `byte[]` for each call. Per [`Span<T>` usage Rule #9](https://learn.microsoft.com/en-us/dotnet/standard/memory-and-spans/memory-t-usage-guidelines),  the address of a local can be used instead, as a potentially more efficient approach:

> *In the previous example, `pbData` can be null if, for example, the input span is empty. If the exported method absolutely requires that `pbData` be non-null, even if `cbData` is 0, the method can be implemented as follows*
ericsink added a commit that referenced this pull request Nov 7, 2023
… but leaving the explicit checks for Length == 0
@ericsink
Copy link
Owner

ericsink commented Nov 7, 2023

I made a code change similar to this PR, but keeping the explicit checks for Length == 0. Thanks!

@ericsink ericsink closed this Nov 7, 2023
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.

sqlite3_bind_text calls with empty Span<byte> result in null values
2 participants