public inbox for newlib@sourceware.org
 help / color / mirror / Atom feed
From: Brian Inglis <Brian.Inglis@SystematicSw.ab.ca>
To: newlib@sourceware.org
Subject: Re: [PATCH] Change time_t to 64-bit by default
Date: Fri, 08 Sep 2017 05:40:00 -0000	[thread overview]
Message-ID: <83abd70b-58c7-3b51-bf6e-e8ec3b810d25@SystematicSw.ab.ca> (raw)
In-Reply-To: <1504820198.2562.5.camel@op.pl>

On 2017-09-07 15:36, Freddie Chopin wrote:
> On Thu, 2017-09-07 at 14:59 -0600, Brian Inglis wrote:
>> On 2017-09-07 12:47, Brian Inglis wrote:
>>> On 2017-09-07 10:03, Sebastian Huber wrote:
>>>>
>>>> ----- Freddie Chopin <freddie_chopin@op.pl> schrieb:
>>>>> On Thu, 2017-09-07 at 15:00 +0200, Sebastian Huber wrote:
>>>>>> The casts in Newlib gmtime_r() look pretty useless. I will
>>>>>> send a
>>>>>> patch 
>>>>>> with:
>>>>>>
>>>>>> diff --git a/newlib/libc/time/gmtime_r.c
>>>>>> b/newlib/libc/time/gmtime_r.c
>>>>>> index 81c7c94b1..6475df3ba 100644
>>>>>> --- a/newlib/libc/time/gmtime_r.c
>>>>>> +++ b/newlib/libc/time/gmtime_r.c
>>>>>> @@ -56,8 +56,8 @@ _DEFUN (gmtime_r, (tim_p, res),
>>>>>>     unsigned erayear, yearday, month, day;
>>>>>>     unsigned long eraday;
>>>>>>
>>>>>> -  days = ((long)lcltime) / SECSPERDAY +
>>>>>> EPOCH_ADJUSTMENT_DAYS;
>>>>>> -  rem = ((long)lcltime) % SECSPERDAY;
>>>>>> +  days = lcltime / SECSPERDAY + EPOCH_ADJUSTMENT_DAYS;
>>>>>> +  rem = lcltime % SECSPERDAY;
>>>>>>     if (rem < 0)
>>>>>>       {
>>>>>>         rem += SECSPERDAY;
>>>>>
>>>>> You should also replace "long days, rem;" with "time_t days,
>>>>> rem;".
>>>>
>>>> 2147483647 days are roughly 5879489 years. This should be enough
>>>> for most applications.
>>>
>>> The library supports tm_year from -2^31 -2147483648 to 2^31-1
>>> 2147483647 and
>>> year from -2^31+1900 -2147481748 to 2^31-1+1900 2147485547 allowing
>>> time_t from
>>> about -67768100536348801 to 67768036191676799.
>>
>> Should int ... year be changed to long year or time_t year to be able
>> to handle
>> tm_year + 1900 mentioned above?
> 
> Probably, but there's a high probability that there are more things
> that would need changing - for example without additional casts this:
> 
> year = ADJUSTED_EPOCH_YEAR + erayear + era * YEARS_PER_ERA + (month <= 1);
> 
> would probably be calculated as "int" anyway.

Those are the problems I'm looking to patch.
The question is, going forward, should we use long for bigger than int values as
usual, or use time_t instead, to consistently avoid long, in these time functions.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

  reply	other threads:[~2017-09-07 21:58 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-07 12:34 Sebastian Huber
2017-09-07 13:00 ` Corinna Vinschen
2017-09-07 13:10   ` Sebastian Huber
2017-09-07 14:17     ` Corinna Vinschen
2017-09-07 16:03     ` Freddie Chopin
2017-09-07 16:32       ` Sebastian Huber
2017-09-07 18:47         ` Freddie Chopin
2017-09-07 20:59         ` Brian Inglis
2017-09-07 21:37           ` Brian Inglis
2017-09-07 21:58             ` Freddie Chopin
2017-09-08  5:40               ` Brian Inglis [this message]
2017-09-08  8:19                 ` Freddie Chopin
2017-09-08 14:46                   ` Corinna Vinschen
2017-09-08 15:00                     ` Brian Inglis
2017-09-08 16:34                       ` Corinna Vinschen
2017-09-10  4:04                         ` Brian Inglis
2017-09-11 15:17                           ` Time Fixes Diff (was: Change time_t to 64-bit by default) Brian Inglis
2017-09-11 18:53                             ` Time Fixes Diff Craig Howland
2017-09-12  5:34                               ` Brian Inglis
2017-09-12 20:19                                 ` Sebastian Huber

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83abd70b-58c7-3b51-bf6e-e8ec3b810d25@SystematicSw.ab.ca \
    --to=brian.inglis@systematicsw.ab.ca \
    --cc=newlib@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).