Skip to content

Fix string handling in pushAny #231

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.

Sign up for GitHub

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

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

Fix string handling in pushAny #231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 13 additions & 20 deletions src/lib.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4589,34 +4589,27 @@ pub const Lua = opaque {
}

/// Pushes any string type
fn pushAnyString(lua: *Lua, value: anytype) error{OutOfMemory}!void {

@robbielyman robbielyman Oct 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is there really no error condition?

@AshAmetrine AshAmetrine Oct 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Not since the unnecessary temporary allocation was removed.

fn pushAnyString(lua: *Lua, value: anytype) void {
const info = @typeInfo(@TypeOf(value)).pointer;
switch (info.size) {
.slice => {
std.debug.assert(info.child == u8);
_ = lua.pushString(value);
},
.one => {
const childinfo = @typeInfo(info.child).array;
std.debug.assert(childinfo.child == u8);
std.debug.assert(childinfo.sentinel() != null);

if (childinfo.sentinel()) |sentinel| {

@robbielyman robbielyman Oct 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why is this code deleted?

@AshAmetrine AshAmetrine Oct 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This assertion rejected pointers to arrays without a sentinel, such as: *const [2]u8.

if (sentinel != 0) {
@compileError("Sentinel of slice must be a null terminator");
}
}
_ = lua.pushStringZ(value);
_ = lua.pushString(value);
},
.c, .many, .slice => {
.many => {
std.debug.assert(info.child == u8);
if (info.sentinel()) |sentinel| {
if (sentinel != 0) {
@compileError("Sentinel of slice must be a null terminator");
}
_ = lua.pushStringZ(value);
} else {
const null_terminated = try lua.allocator().dupeSentinel(u8, value, 0);
defer lua.allocator().free(null_terminated);
_ = lua.pushStringZ(null_terminated);
if (info.sentinel() != 0) {
@compileError("Sentinel of string pointer must be a null terminator");
}
_ = lua.pushStringZ(std.mem.span(value));
},
.c => @compileError("C pointer length cannot be determined; pass a slice instead"),

@robbielyman robbielyman Oct 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why is this here? Lua itself obviously works with C pointers all the time?

@AshAmetrine AshAmetrine Oct 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Quoting the Zig documentation: "The only valid reason for using a C pointer is in auto-generated code from translating C code."
I see no reason why they should be supported. The caller should know the type.

@natecraddock natecraddock Oct 12, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It is possible that the value comes from another C library. I have recently been doing work using Lua and sqlite and I need to work with C strings occasionally. I think C pointers should be supported

@AshAmetrine AshAmetrine Oct 13, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

In that case, you would either know the length or whether it has a null terminator, then you could use either ptr[0..len] or std.mem.span(ptr).

If we supported C pointers with pushAny, it would be making the assumption that it has a null terminator, which results in UB when there isn't one.

Side note: I did not remove support for C pointers in this change. I only changed the error message.

}
}

Expand Down Expand Up @@ -4700,7 +4693,7 @@ pub const Lua = opaque {
},
.pointer => |info| {
if (comptime isTypeString(info)) {
try lua.pushAnyString(value);
lua.pushAnyString(value);
} else switch (info.size) {
.one => {
if (info.attrs.@"const") {
Expand Down Expand Up @@ -4764,7 +4757,7 @@ pub const Lua = opaque {
if (info.tag_type == null) @compileError("Parameter type is not a tagged union");
lua.createTable(0, 0);
errdefer lua.pop(1);
try lua.pushAnyString(@tagName(value));
lua.pushAnyString(@tagName(value));

inline for (info.field_names) |name| {
if (std.mem.eql(u8, name, @tagName(value))) {
Expand Down
11 changes: 7 additions & 4 deletions src/tests.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2684,10 +2684,13 @@ test "pushAny" {
try testing.expect(my_bool);

//string literal
try lua.pushAny("hello world");
const value = try lua.toString(-1);
const end = std.mem.indexOfSentinel(u8, 0, value);
try testing.expect(std.mem.eql(u8, value[0..end], "hello world"));
try lua.pushAny("hello\x00world");
try testing.expectEqualStrings("hello\x00world", try lua.toString(-1));

//null-terminated string pointer
const pointer: [*:0]const u8 = "hello world";
try lua.pushAny(pointer);
try testing.expectEqualStrings("hello world", try lua.toString(-1));

//null
try lua.pushAny(null);
Expand Down