Hi Paul,
On 3/12/23 02:44, Paul Eggert wrote:
> From f3514f26297e884a00d4fb29191bd9978eb03e7b Mon Sep 17 00:00:00 2001
> From: Paul Eggert <egg...@cs.ucla.edu>
> Date: Sat, 11 Mar 2023 00:42:29 -0800
> Subject: [PATCH 6/8] Fix crash with large timestamps
>
> * libmisc/date_to_str.c (date_to_str): Do not crash if gmtime
> returns NULL because the timestamp is far in the future.
> Instead, use a dummy struct tm * to pacify any pedantic runtime.
> Simplify by always calling strftime, instead of sometimes strftime
> and sometimes strlcpy.
>
> Signed-off-by: Paul Eggert <egg...@cs.ucla.edu>
> ---
> libmisc/date_to_str.c | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/libmisc/date_to_str.c b/libmisc/date_to_str.c
> index f3b9dc76..0840c38c 100644
> --- a/libmisc/date_to_str.c
> +++ b/libmisc/date_to_str.c
> @@ -35,13 +35,16 @@
>
> void date_to_str (size_t size, char buf[size], long date)
> {
> - time_t t;
> + time_t t = date;
> + struct tm *tm = gmtime (&t);
>
> - t = date;
> - if (date < 0) {
> - (void) strlcpy (buf, "never", size);
> - } else {
> - (void) strftime (buf, size, "%Y-%m-%d", gmtime (&t));
> - buf[size - 1] = '\0';
> - }Please split the patch into two: one that fixes the UB, and another that removes the strlcpy(3) calls. They can be done independently, and I'm not convinced by your measurement of simplicity :) On 3/12/23 02:38, Paul Eggert wrote: > It depends on how one measures simplicity. The reader will need to know > strftime's API regardless; requiring the reader to also know strlcpy's > API makes the reader's job harder. I expect readers to easily learn that from the relevant man page :) Once you know what strlcpy(3) is, that is, a function that copies a string with truncation, it's straightforward to read. Knowing strftime(3)'s details is more obscure for those who are not time nerds :-) > > Also, it's less machine code to call just the one function (if one cares > about simplicity of debugging 😄. Heh, I could count with my fingers the number of days I've used gdb(1) in my entire life :D. Hail fprintf(3) and stderr! Which BTW benefit from expanded conditionals, rather than ternary ops. > > If you still prefer calling two different functions instead of just one, > feel free to modify it to use plain strcpy. strlcpy isn't needed here as > the destination buffers are all big enough. Do we know that? The API receives a size as a parameter, which could perfectly be 1 (well, I know we're not going to do that, but since it's a generic API, I wouldn't rely on current sizes). Cheers, Alex > To be honest though I like > it the way it is (though it could use a comment; I'll add that). > + /* A dummy whose address can be passed to strftime to avoid > + undefined behavior. It's OK for it to be uninitialized > + since no conversion specs are used. */ > + struct tm dummy; > + > + (void) strftime (buf, size, > + date < 0 ? "never" : tm ? "%Y-%m-%d" : "future", > + tm ? tm : &dummy); > + buf[size - 1] = '\0'; > } > -- > 2.37.2 > -- <http://www.alejandro-colomar.es/> GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
OpenPGP_signature
Description: OpenPGP digital signature
