* [PATCH v2] fix integer overflow of tm_year in __secs_to_tm
@ 2016-11-03 2:29 Daniel Sabogal
2016-11-07 17:09 ` Rich Felker
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Sabogal @ 2016-11-03 2:29 UTC (permalink / raw)
To: musl
From: Daniel Sabogal <dsabogal@ufl.edu>
the overflow check for years+100 did not account for the extra
year computed from the remaining months. instead, perform this
check after obtaining the final number of years.
---
v2: Subtract 12 from months, not 10.
#include <time.h>
#include <stdio.h>
extern int __secs_to_tm(long long t, struct tm *tm);
int main(void)
{
struct tm tm = {0};
if (__secs_to_tm(67768036191676800LL, &tm) < 0) return 1;
printf("%d\n", tm.tm_year);
}
src/time/__secs_to_tm.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/time/__secs_to_tm.c b/src/time/__secs_to_tm.c
index 3a3123a..c1cc28c 100644
--- a/src/time/__secs_to_tm.c
+++ b/src/time/__secs_to_tm.c
@@ -60,15 +60,16 @@ int __secs_to_tm(long long t, struct tm *tm)
for (months=0; days_in_month[months] <= remdays; months++)
remdays -= days_in_month[months];
+ if (months >= 10) {
+ months -= 12;
+ years++;
+ }
+
if (years+100 > INT_MAX || years+100 < INT_MIN)
return -1;
tm->tm_year = years + 100;
tm->tm_mon = months + 2;
- if (tm->tm_mon >= 12) {
- tm->tm_mon -=12;
- tm->tm_year++;
- }
tm->tm_mday = remdays + 1;
tm->tm_wday = wday;
tm->tm_yday = yday;
--
2.10.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] fix integer overflow of tm_year in __secs_to_tm
2016-11-03 2:29 [PATCH v2] fix integer overflow of tm_year in __secs_to_tm Daniel Sabogal
@ 2016-11-07 17:09 ` Rich Felker
2016-11-08 3:40 ` Daniel Sabogal
0 siblings, 1 reply; 4+ messages in thread
From: Rich Felker @ 2016-11-07 17:09 UTC (permalink / raw)
To: musl
On Wed, Nov 02, 2016 at 10:29:36PM -0400, Daniel Sabogal wrote:
> From: Daniel Sabogal <dsabogal@ufl.edu>
>
> the overflow check for years+100 did not account for the extra
> year computed from the remaining months. instead, perform this
> check after obtaining the final number of years.
> ---
> v2: Subtract 12 from months, not 10.
Thanks. I almost accepted the old patch with the error. Maybe in the
future consider including a test case with the patch.
I don't want to make testcases a prerequisite for bug fixes because
that leads to bugs going unfixed for a long time, but perhaps for
obscure issues like this unlikely to be hit in real-world use, it
would be good to strongly encourage submission of test cases with
patches.
Rich
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] fix integer overflow of tm_year in __secs_to_tm
2016-11-07 17:09 ` Rich Felker
@ 2016-11-08 3:40 ` Daniel Sabogal
2016-11-08 3:57 ` Rich Felker
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Sabogal @ 2016-11-08 3:40 UTC (permalink / raw)
To: musl
On Mon, Nov 7, 2016 at 12:09 PM, Rich Felker <dalias@libc.org> wrote:
> On Wed, Nov 02, 2016 at 10:29:36PM -0400, Daniel Sabogal wrote:
>> From: Daniel Sabogal <dsabogal@ufl.edu>
>>
>> the overflow check for years+100 did not account for the extra
>> year computed from the remaining months. instead, perform this
>> check after obtaining the final number of years.
>> ---
>> v2: Subtract 12 from months, not 10.
>
> Thanks. I almost accepted the old patch with the error. Maybe in the
> future consider including a test case with the patch.
I provided a sample program within the patch.
Did you have something else in mind for test cases?
> I don't want to make testcases a prerequisite for bug fixes because
> that leads to bugs going unfixed for a long time, but perhaps for
> obscure issues like this unlikely to be hit in real-world use, it
> would be good to strongly encourage submission of test cases with
> patches.
I agree.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] fix integer overflow of tm_year in __secs_to_tm
2016-11-08 3:40 ` Daniel Sabogal
@ 2016-11-08 3:57 ` Rich Felker
0 siblings, 0 replies; 4+ messages in thread
From: Rich Felker @ 2016-11-08 3:57 UTC (permalink / raw)
To: musl
On Mon, Nov 07, 2016 at 10:40:52PM -0500, Daniel Sabogal wrote:
> On Mon, Nov 7, 2016 at 12:09 PM, Rich Felker <dalias@libc.org> wrote:
> > On Wed, Nov 02, 2016 at 10:29:36PM -0400, Daniel Sabogal wrote:
> >> From: Daniel Sabogal <dsabogal@ufl.edu>
> >>
> >> the overflow check for years+100 did not account for the extra
> >> year computed from the remaining months. instead, perform this
> >> check after obtaining the final number of years.
> >> ---
> >> v2: Subtract 12 from months, not 10.
> >
> > Thanks. I almost accepted the old patch with the error. Maybe in the
> > future consider including a test case with the patch.
>
> I provided a sample program within the patch.
> Did you have something else in mind for test cases?
Admittedly I missed it somehow, but I guess to call it a test case I'd
want to see expected results and a justification for them. In this
case diff of old vs new output for various inputs would have caught
the bug in v1.
It might be nice to have a test in libc-test that just runs a bunch of
time_t-tm-time_t and tm-time_t-tm round trips for random inputs and
checks that they round-trip successfully...
> > I don't want to make testcases a prerequisite for bug fixes because
> > that leads to bugs going unfixed for a long time, but perhaps for
> > obscure issues like this unlikely to be hit in real-world use, it
> > would be good to strongly encourage submission of test cases with
> > patches.
>
> I agree.
...but the above ideas are getting well beyond what I'd want to impose
on bug reporters/minor patch authors. So it's more just brainstorming
about the tests that would be helpful for someone with the time to
help with testing to implement.
Rich
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-11-08 3:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-03 2:29 [PATCH v2] fix integer overflow of tm_year in __secs_to_tm Daniel Sabogal
2016-11-07 17:09 ` Rich Felker
2016-11-08 3:40 ` Daniel Sabogal
2016-11-08 3:57 ` Rich Felker
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
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 NNTP newsgroup(s).