[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJgzZop0-m40xgipg2fi86FQmmt_FEaMfEJm2m-WD97qaA+foA@mail.gmail.com>
Date: Wed, 21 Jun 2023 13:09:36 -0700
From: enh <enh@...gle.com>
To: libc-coord@...ts.openwall.com
Cc: Paul Eggert <eggert@...ucla.edu>
Subject: Re: thread-safe localtime() for an arbitrary timezone
On Wed, Jun 21, 2023 at 2:12 AM Jonathan Wakely <jwakely.gcc@...il.com>
wrote:
> 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.
>
i think as a standard c++ library maintainer yourself you forget that
that's not trivial to most :-)
i find that most people can come up with the custom deleter on their own,
but there are only a handful of uses of `std::remove_pointer_t` in the
codebase (and they're from the handful of c++ experts i'd have expected). i
expect _manually_ writing out the underlying `struct __timezone_t` is far
more likely in practice.
but, yeah, there's no reason i can't mention this in the doc comment at
least ... i'll do that.
> 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.
>
yeah, making it easier to implement that on Android _without_ duplicating
all the tzdata code (and possibly data) is another thing that's in the back
of my mind. although the `get_info()` stuff will be a PITA if anyone does
implement this (rather than just give up and move to rust, as appears to be
the current trend).
Content of type "text/html" skipped
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.
