Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 21 Jun 2023 10:11:52 +0100
From: Jonathan Wakely <jwakely.gcc@...il.com>
To: libc-coord@...ts.openwall.com
Cc: Paul Eggert <eggert@...ucla.edu>
Subject: Re: thread-safe localtime() for an arbitrary timezone

On Tue, 20 Jun 2023 at 21:56, enh <enh@...gle.com> wrote:
>
>
>
> On Mon, Jun 19, 2023 at 2:40 AM Jonathan Wakely <jwakely.gcc@...il.com> wrote:
>>
>> On Thu, 15 Jun 2023 at 20:57, enh <enh@...gle.com> wrote:
>> >
>> >
>> >
>> > On Thu, Jun 15, 2023 at 10:03 AM enh <enh@...gle.com> wrote:
>> >>
>> >>
>> >>
>> >> On Wed, Jun 14, 2023 at 11:19 AM Paul Eggert <eggert@...ucla.edu> wrote:
>> >>>
>> >>> On 6/14/23 11:06, enh wrote:
>> >>> > is anyone aware of any standardization work in this area? i know netbsd
>> >>> > has tzalloc()/localtime_rz()/tzfree() (and tzcode has implementations), but
>> >>> > they're the only ones who've shipped anything, right? and there's no
>> >>> > in-progress work on any alternative?
>> >>>
>> >>> On my long list of things to do has been to implement the tzcode API in
>> >>> glibc. Nothing publishable, alas.
>> >>>
>> >>> The tzcode API differs slightly from NetBSD's with, as I recall, the
>> >>> blessing of the NetBSD implementer in question, so I hope it would be a
>> >>> better point of departure. (Also on my list of things to do is to merge
>> >>> tzcode's back into NetBSD....)
>> >>
>> >>
>> >> any thoughts on either renaming `struct state` (i went with `struct __timezone_t` to match the typedef to timezone_t for the pointer) or having a knob for that (i added a `#define state __timezone_t` in the `#if NETBSD_INSPIRED` in private.h)? why do i care? because tzcode pulls in the system <time.h>, which is actually fine for the tzcode code, but breaks the OpenBSD wcsftime().
>> >>
>> >> (i hope one day to find the time to write a wcsftime()-in-terms-of-strftime() wrapper so i can just delete that, but working on wide character functions no-one uses or should be using is quite hard to motivate. oh, wait... FreeBSD and NetBSD both already went that route. i'll try that, in which case only tzcode itself will need to know that the "real" name of the public `struct __timezone_t` is actually `struct state`...)
>> >
>> >
>> > switching to the FreeBSD "just call the non-wide function" implementation of wcsftime() went well.
>> >
>> > writing some basic unit tests for these functions went fine, though it was sad that i couldn't use `std::unique_ptr<timezone_t, decltype(&tzfree)> seoul{tzalloc("Asia/Seoul"), tzfree}` because the size of timezone_t isn't known.
>>
>> Surely the size of timezone_t is known. It must be for it to be
>> returned by value from tzalloc and passed by value into tzfree. The
>> reason you can't use unique_ptr like that is that you've got a type
>> error. timezone_t is the pointer type, not the "pointee" type, and so
>> what you have declared there is a unique_ptr that owns a timezone_t*
>> and tries to pass that to tzfree, which won't compile.
>>
>> Something like this would work:
>>
>> static_assert(std::is_pointer_v<timezone_t>);
>> struct tz_deleter {
>>   using pointer = timezone_t;
>>   void operator()(timezone_t t) const { tzfree(t); }
>> };
>> std::unique_ptr<std::remove_pointer_t<timezone_t>, tz_deleter>
>> seoul(tzalloc("Asia/Seoul"));
>>
>
> yes, but my anecdata in the Android codebase has been that almost no-one will write a custom deleter. whereas seeding the codebase with a few good things to copy & paste worked wonders for closedir() and fclose() especially.

So seed the codebase with the deleter and a typedef. All you need to provide is:

struct tz_deleter {
   using pointer = timezone_t;
   void operator()(timezone_t t) const { tzfree(t); }
};
using tz_ptr = std::unique_ptr<std::remove_pointer_t<timezone_t>, tz_deleter>;

And then people can just use tz_ptr and it does the right thing.

> because of that, hiding the pointers behind a typedef is counterproductive for safe c++ use.

No, you're just holding it wrong. You can do it without a custom deleter type:

std::unique_ptr<std::remove_pointer_t<timezone_t>, decltype(&tzfree)>
seoul{tzalloc("Asia/Seoul"), &tzfree};

This works. The reason to use a custom deleter is that (1) you don't
need to pass &tzfree to the constructor, (2) the unique_ptr is only as
large as sizeof(timezone_t) because it doesn't need to also store
&tzfree, (3) it can optimize better because it isn't dereferencing a
function pointer at runtime.


Even better IMHO would be to avoid this API entirely in C++ and use
C++20's std::chrono::time_zone but that's not evenly distributed yet.
I only added it to GCC a few months ago and you need GCC 13.1 to use
it. It's not in LLVM's libc++ yet.

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.