Re: [PATCH v2] ctime.3: Document how to check errors from mktime(3)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[TO += mtk]

Hi Paul,

On Fri, Aug 23, 2024 at 10:44:56AM GMT, Paul Eggert wrote:
> On 2024-08-23 07:37, Alejandro Colomar wrote:
> >      ++    if (tm.tm_wday == \-1)
> 
> 'if (tm.tm_wday < 0)' is a bit faster on typical machines. (There are
> multiple instances of this issue.)

I had the costume of checking errors with <0, and Michael Kerrisk
convinced me of using ==-1 because it's more explicit for the reader.
I now prefer ==-1.  I find that when I read <0, it's sometimes unclear
whether the author really intended to reject all negatives values or
not, and it also requires that I check the specific API to know if
other negative values are possible.  On the contrary, -1 is usual for
meaning an error, and rarely is the time when I need to check that it
actually means an error.

(And tangentially, I don't think anyone who calls mktime(3) will be
 worried about a single instruction.  But I see that this discussion
 applies in other places where that might be more meaningful.)

> > +To determine if
> 
> "if" -> "whether"

Thanks.

> 
> > +    printf("%jd\[rs]n", (intmax_t) t);
> 
> This is not portable in general, as time_t might be unsigned. You could use
> strftime instead of printf. But see below for a better suggestion.

Out of curiosity, in systems where time_t is unsigned, does a mktime(3)
call with a time representing a time before Epoch result in an error, or
is it stored as a huge time_t value?

Does any existing system (or historic one) use an unsigned time_t, or is
that only hypothetical?

> > +    tm.tm_year  = atoi(*p++) \- 1900;
> 
> This doesn't work for the year 2147485547 (2**31 + 1899), which mktime can
> handle on typical machines with 32-bit int and 64-bit time_t. Also, all the
> atoi calls silently mess up if the argument overflows or is syntactically
> invalid.

We extensively use atoi(3) in the EXAMPLES sections where we intend to
communicate to the user that they should add error handling and call
strtol(3) (or something better, cough) but that for brevity we don't.

The program serves to illustrate how mktime(3) works, and how its errors
are handled.

> 
> To simplify the example, I suggest not doing I/O or parsing. Just have a
> function that accepts a struct tm *, and returns true or false and updates
> the struct tm when it returns true. That would avoid the issues with printf
> and atoi.

But then it wouldn't allow the reader to interact with the function as
easily.  They'd need to recompile the program every time.  We try that
example programs are interactive when it makes sense, and I think this
one makes sense.  The small oddities in the parsing are obviously wrong
so that they are not copied.

Thanks for the review!


Have a lovely day!
Alex


-- 
<https://www.alejandro-colomar.es/>

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Kernel Documentation]     [Netdev]     [Linux Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux