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

Add std.mem.zeroes to the standard library #4092

Merged
merged 1 commit into from
Jan 7, 2020

Conversation

FireFox317
Copy link
Contributor

I'm not sure if we should only allow extern structs or specific types, but this is a start.

See: #2257 (comment)

This zero initializes the type passed in. Can be used to zero
initialize c structs.
@JesseRMeyer
Copy link

Is returning stack memory of Item undefined behavior in Zig once zero's scope returns?

@andrewrk
Copy link
Member

andrewrk commented Jan 6, 2020

Is returning stack memory of Item undefined behavior in Zig once zero's scope returns?

It's a by-value return, so it's fine. The memory is provided by the caller and the data is copied into it. What you have to look out for is taking the address of stack memory, e.g. with &.

@JesseRMeyer
Copy link

Is returning stack memory of Item undefined behavior in Zig once zero's scope returns?

It's a by-value return, so it's fine. The memory is provided by the caller and the data is copied into it. What you have to look out for is taking the address of stack memory, e.g. with &.

Yeah, I just noticed that. Thought I first saw an address return but my eyes must be fuzzy today.

@LemonBoy
Copy link
Contributor

LemonBoy commented Jan 6, 2020

Why is it limited to extern struct?

@FireFox317
Copy link
Contributor Author

@LemonBoy because I'm not sure if normal/packed structs can also be zeroed like this. Because you can get null pointers and stuff, correct me if I'm wrong

@LemonBoy
Copy link
Contributor

LemonBoy commented Jan 6, 2020

Indeed, I had something less safe in mind such as a member + return like the secureZero function declared below.

@andrewrk
Copy link
Member

andrewrk commented Jan 6, 2020

I do think this function should be discouraged for non-extern, non-packed structs (even if a reflection-based version exists that writes 0 memberwise).

I intentionally removed the zeroes keyword from the language because it allows us to put extra fields into structs for various reasons. Safety, hot code swapping, incremental compilation, and profiling, to name a few.

Writing zeroes to a non-packed, non-extern struct would clobber the safety field that allows safety of @ptrCast to work, for example. And it would clobber the detection of whether the struct was undefined.

}

var item: T = undefined;
@memset(@ptrCast([*]u8, &item), 0, @sizeOf(T));
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this defined? e.g. if T was a u24 then the size is 3 but @sizeOf returns 4.
See e.g. #4093

Copy link
Member

Choose a reason for hiding this comment

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

The code is correct. @sizeOf gives the number of bytes the type takes up in memory. This function is valid for types which have well-defined memory layout, which includes extern structs (the main use case).

pub fn zeroes(comptime T: type) T {
if (@sizeOf(T) == 0) return T{};

if (comptime meta.containerLayout(T) != .Extern) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should allow packed as well?

Copy link
Member

Choose a reason for hiding this comment

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

not necessary for merge, made complicated by #3133

@daurnimator daurnimator added the standard library This issue involves writing Zig code for the standard library. label Jan 6, 2020
@FireFox317
Copy link
Contributor Author

Feel free to ask me to add anything. I can also allow packed structs and add a note regarding #3133 if that would be desired.

@andrewrk
Copy link
Member

andrewrk commented Jan 7, 2020

Feel free to ask me to add anything. I can also allow packed structs and add a note regarding #3133 if that would be desired.

This is a useful first implementation. Further PRs are welcome to improve it as well :)

@andrewrk andrewrk merged commit 0deab8f into ziglang:master Jan 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants