fix datepicker so that it works with Darwin, and possibly fix stack overwrite

[zigster64]

Nice work !

Having some small issues running this on Mac. (haven't tested this on Linux or Win)

With the datePicker widget, zig ends up complaining about possible stack smashing, if you pass the address of a stack variable.

In the original code, it creates a stack variable 'time', and passes the address of this to ui.DateTimePickerTime(&time). Sounds reasonable, but the compiler_rt detects possible stack smashing.

If I allocate a new one on the heap, and pass that through, that works fine (but its ugly because it allocates memory)

Alternative - if the widgets can be structs, rather than opaques .... then you can add fields to the struct, and the fields are heap allocated when widget.New() is called. That's what I have done in this PR, and it seems to work well.

What is the requirement to have the widgets declared as opaques ? If we can do structs instead, it still compiles and runs ok ... and the benefit of being able to add additional fields is nice.

Also for reference with the need to allocate a new struct_tm rather than pass a ref to a local variable : https://github.com/andlabs/ui/blob/70a69d6ae31ed9d8bb0619a191179c63d846ab8e/datetimepicker.go#L53

This is a Go wrapper around libui, and you can see in the DateTimePicker implementation, it also has to do the "allocate a new time on the heap, and pass that to the function" ... so it must be a general problem.

[zigster64]

ah - just found out why they are opaques then :)

The "New()" calls return whatever Control the C library returns ... so it's not quite the same as what I thought. Having these widgets declared as opaques then allows zig to enforce some nice type safety over these generic Control values. Fair enough.

No probs - will fix up this PR later on.

[zigster64]

done now - ready for review if you like

[leroycep]

The stack overwrite may be due to something like the glib extension to tm, see "Notes" at the bottom of this page: https://linux.die.net/man/3/localtime

I don't know if libui-ng is being compiled with C99, but perhaps compiling it with C99 as the std would fix it.

[desttinghim]

Thanks for the PR! The changes for unmutated var and the date logic are appreciated. As @leroycep points out, it appears there is an underlying issue at play here that needs to be resolved. libui (on gtk and darwin) does a memcopy using sizeof(struct tm), with the pointer passed in as the memory location to use. I just ran a test printing out the size of struct_tm in ui.zig and the size of struct tm in datetimepicker.c:

info: size of struct_tm: 36
Sizeof struct tm: 56

This confirms @leroycep's suspicion that the glibc struct tm is being used (with GTK anyway), I suspect Apple is doing something similar with their struct tm. Actually, after a bit of poking around with C, I think every target libui supports has a struct tm with a sizeof 56. Re-reading the documentation I was referencing when writing struct_tm, it says the c standard only specifies that the given fields must be present.

Allocating an entire page for each value likely covered up the issue. I think a better solution would be to reintroduce the old logic fn Time() logic and pad out the struct to be 56 bytes. Adding a field like _padding: [20]u8, ought to do the trick.

NOTE: I wrote the next paragraphs before you updated the PR

Nice work ! Thank you! With the datePicker widget, zig ends up complaining about possible stack smashing, if you pass the address of a stack variable.

In the original code, it creates a stack variable 'time', and passes the address of this to ui.DateTimePickerTime(&time). Sounds reasonable, but the compiler_rt detects possible stack smashing. Oh, I hadn't seen this issue pop up in my testing, though I suspected it was an error I might run into.

ah - just found out why they are opaques then :)

The "New()" calls return whatever Control the C library returns ... so it's not quite the same as what I thought. Having these widgets declared as opaques then allows zig to enforce some nice type safety over these generic Control values. Fair enough.

No probs - will fix up this PR later on.

Yes, libui itself defines the types as opaques like this: typedef struct uiWindow uiWindow; which tells C that it is a struct but that the actual layout is not defined. libui then defines each type to start with a uiControl element and uses the space after to store custom data per-control. It's a very useful way to do generics in C :slightly_smiling_face:

[zigster64]

Beautiful ! That was a bit subtle. Seems it really was stack smashing then - well done Zig compiler_rt for yelling about that.

This approach in this latest patch works (on Mac) - pad out our struct_tm to cover the extra fields found in the BSD standard time.h, which also covers "GNU extensions". No allocations this time

GNU extensions https://www.gnu.org/software/libc/manual/html_node/Broken_002ddown-Time.html

I double checked on my FreeBSD box, and struct tm conforms to this standard as well.

Test run, printing out the full struct_tm values, and the computed TZ == lgtm

Can I get a test run on Linux and Windows to confirm that this works there as well ? I guess it would, al long as the length of struct_tm was at least big enough to hold all of whatever "struct tm" holds

That leaves me scratching my head wondering what the Go lib guys were doing when they allocate a new time on the heap for every read of the dateTime control (???). ah well, we don't need to do that.

[zigster64]

The stack overwrite may be due to something like the glib extension to tm, see "Notes" at the bottom of this page: https://linux.die.net/man/3/localtime

I don't know if libui-ng is being compiled with C99, but perhaps compiling it with C99 as the std would fix it.

Thanks, that was well spotted. My brain didn't even register the difference between "struct_tm" vs "struct tm" when reading the code.

[zigster64]

Unrelated question - I want to point my dev code to my local copy of the C lib libui-ng, from my fork of your fork.

Which of the gazillion branches should I be using ? Master seems the most up to date, but its missing the SetValueDouble stuff

[desttinghim]

Unrelated question - I want to point my dev code to my local copy of the C lib libui-ng, from my fork of your fork.

Which of the gazillion branches should I be using ? Master seems the most up to date, but its missing the SetValueDouble stuff

I believe I used wp-internal-3. It is kind of a huge mess though :see_no_evil: I should clean up the spurious ones