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

Merged
natecraddock merged 1 commit into from
Oct 24, 2026
Merged

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 @@ -4591,34 +4591,27 @@ pub const Lua = opaque {
}

/// Pushes any string type
fn pushAnyString(lua: *Lua, value: anytype) error{OutOfMemory}!void {
Comment thread
natecraddock marked this conversation as resolved.
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| {
Comment thread
natecraddock marked this conversation as resolved.
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
Contributor 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
Contributor 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.

@natecraddock natecraddock Oct 24, 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.

I just tested with this PR. This fails

const c_str: [*c]const u8 = "hello zig";
try lua.pushAny(c_str);
try testing.expectEqualStrings("hello zig", try lua.toString(-1));

and both of these work

const c_str: [*c]const u8 = "hello zig";
try lua.pushAny(std.mem.span(c_str));
try testing.expectEqualStrings("hello zig", try lua.toString(-1));

const c_str: [*c]const u8 = "hello zig";
const ptr: [*:0]const u8 = c_str;
try lua.pushAny(ptr);
try testing.expectEqualStrings("hello zig", try lua.toString(-1));

I gave it more thought and I think I'm actually okay with this change. If someone has a [*c] pointer, they can use span or coerce to a null-terminated many-item pointer.

}
}

Expand Down Expand Up @@ -4702,7 +4695,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 @@ -4766,7 +4759,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
Loading