[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z_QE74Sei_2quSJS@cloudsdale.the-delta.net.eu.org>
Date: Mon, 7 Apr 2025 19:01:35 +0200
From: "Haelwenn (lanodan) Monnier" <contact@...ktivis.me>
To: musl@...ts.openwall.com
Cc: Rich Felker <dalias@...c.org>, ipedrosa@...hat.com,
~hallyn/shadow@...ts.sr.ht, libc-alpha@...rceware.org
Subject: Re: strptime("UTC", "%Z", &tm) in musl
[2025-04-07 01:54:14+0200] Alejandro Colomar:
>Hi Haelwenn,
>
>On Sun, Apr 06, 2025 at 10:55:46PM +0200, Haelwenn (lanodan) Monnier wrote:
>> [2025-04-06 21:57:35+0200] Alejandro Colomar:
>> > I was trying to develop a function that parses a UTC date in the shadow
>> > project, and I came up with this:
>> > <https://github.com/shadow-maint/shadow/pull/1217/>
>> >
>> > $ grepc -htfd get_date .
>> > static long
>> > get_date(const char *s)
>> > {
>> > struct tm tm;
>> > const char *p;
>> >
>> > bzero(&tm, sizeof(tm));
>> >
>> > p = strptime("UTC", "%Z", &tm);
>> > if (p == NULL || !streq(p, ""))
>> > return -1;
>>
>> That bzero seems like a bit of a bad idea, `struct tm` can have some extra
>> fields where 0 isn't the correct value.
>> Quite like how `tm_isdst` uses -1 for unknown state, 0 for no-DST, 1 for DST.
>>
>> Could probably use `if(!gmtime_r(0, &tm)) { /* handle err */ }` instead
>> which also has the nice effect of initializing tm for UTC,
>> allowing to skip the problematic strptime %Z.
>
>Thanks! This is quite clever. I've changed this to my patch:
>
> diff --git i/lib/strtoday.c w/lib/strtoday.c
> index 2d88e8b6..ab03b921 100644
> --- i/lib/strtoday.c
> +++ w/lib/strtoday.c
> @@ -47,13 +47,13 @@ strtoday(const char *str)
> static long
> get_date(const char *s)
> {
> + time_t t;
> struct tm tm;
> const char *p;
>
> - bzero(&tm, sizeof(tm));
> -
> - p = strptime("UTC", "%Z", &tm);
> - if (p == NULL || !streq(p, ""))
> + // Set timezone to UTC
> + t = 0;
> + if (gmtime_r(&t, &tm) == NULL)
> return -1;
>
> p = strptime(s, "%F", &tm);
>
>If you don't mind, I'd like to add you as a co-author of the patch, and
>assign you copyright in that file. Please sign the following commit, if
>you approve it.
Yeah, looks good to me. And I guess Signed-off-by is the correct field for this.
Signed-off-by: "Haelwenn (lanodan) Monnier" <contact@...ktivis.me>
Best regards
>Have a lovely night!
>Alex
>
>
>---
>
><https://github.com/shadow-maint/shadow/pull/1217>
>
>Author: Alejandro Colomar <alx@...nel.org>
>Date: Tue Feb 18 23:25:29 2025 +0100
>
> lib/getdate.c: Use strptime(3) to simplify
>
> The following trick:
>
> t = 0;
> gmtime_r(&t, &tm);
>
> is a clever way to clear the tm(3type) structure, and set it to use UTC.
>
> We need to set it to set UTC with this trick, because strptime(3)
> doesn't set the timezone. I (Alex) tried previously using
>
> bzero(&tm, sizeof(tm));
> strptime("UTC", "%Z", &tm);
>
> but glibc ignores the timezone, and musl (at least I tried in an Alpine
> container) seems to report an error.
>
> The idea to use gmtime_r(3) was from lanodan.
>
> Link: <https://inbox.sourceware.org/libc-alpha/Z_LqUgildoq33vI-@cloudsdale.the-delta.net.eu.org/T/#u>
> Cc: Iker Pedrosa <ipedrosa@...hat.com>
> Cc: Serge Hallyn <serge@...lyn.com>
> Cc: Rich Felker <dalias@...c.org>
> Co-authored-by: "Haelwenn (lanodan) Monnier" <contact@...ktivis.me>
> Signed-off-by: Alejandro Colomar <alx@...nel.org>
>
>diff --git a/lib/getdate.c b/lib/getdate.c
>index 31a5c25b..fa2df83e 100644
>--- a/lib/getdate.c
>+++ b/lib/getdate.c
>@@ -1,81 +1,31 @@
> // SPDX-FileCopyrightText: 2025, Alejandro Colomar <alx@...nel.org>
>+// SPDX-FileCopyrightText: 2025, "Haelwenn (lanodan) Monnier" <contact@...ktivis.me>
> // SPDX-License-Identifier: BSD-3-Clause
>
>
> #include <config.h>
>
>-#include <errno.h>
>-#include <limits.h>
> #include <stddef.h>
>-#include <string.h>
> #include <time.h>
>
>-#include "atoi/a2i/a2s.h"
> #include "getdate.h"
>-#include "string/strchr/strchrcnt.h"
> #include "string/strcmp/streq.h"
>-#include "string/strcmp/strprefix.h"
>-#include "string/strcmp/strsuffix.h"
>-#include "string/strspn/stpspn.h"
>-
>-
>-#define TM_YEAR_ORIGIN 1900
>-
>-
>-static long yyDay;
>-static long yyMonth;
>-static long yyYear;
>-
>-
>-static int parse_date(const char *s);
>
>
> time_t
>-get_date(const char *s);
>+get_date(const char *s)
> {
>- struct tm tm;
>+ time_t t;
>+ struct tm tm;
>+ const char *p;
>
>- if (parse_date(p) == -1)
>+ t = 0;
>+ if (gmtime_r(&t, &tm) == NULL)
> return -1;
>
>- tm.tm_year = yyYear - TM_YEAR_ORIGIN;
>- tm.tm_mon = yyMonth - 1;
>- tm.tm_mday = yyDay;
>- tm.tm_hour = tm.tm_min = tm.tm_sec = 0;
>- tm.tm_isdst = 0;
>+ p = strptime(s, "%F", &tm);
>+ if (p == NULL || !streq(p, ""))
>+ return -1;
>
> return timegm(&tm);
> }
>-
>-
>-static int
>-parse_date(const char *s)
>-{
>- long n;
>-
>- if (!streq(stpspn(s, "0123456789-"), "")
>- || strchrcnt(s, '-') != 2
>- || strprefix(s, "-")
>- || strsuffix(s, "-")
>- || strstr(s, "--"))
>- {
>- return -1;
>- }
>-
>- if (a2sl(&yyYear, s, &s, 10, LONG_MIN, LONG_MAX) == -1 && errno != ENOTSUP)
>- return -1;
>-
>- if (!strprefix(s++, "-"))
>- return -1;
>-
>- if (a2sl(&yyMonth, s, &s, 10, LONG_MIN, LONG_MAX) == -1 && errno != ENOTSUP)
>- return -1;
>-
>- if (!strprefix(s++, "-"))
>- return -1;
>-
>- if (a2sl(&yyDay, s, NULL, 10, LONG_MIN, LONG_MAX) == -1)
>- return -1;
>-
>- return 0;
>-}
>
>--
><https://www.alejandro-colomar.es/>
><https://www.alejandro-colomar.es:8443/>
><http://www.alejandro-colomar.es:8080/>
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.
