From: Rich Felker <dalias@libc.org>
To: musl@lists.openwall.com
Subject: Re: [PATCH v2] fix integer overflow of tm_year in __secs_to_tm
Date: Mon, 7 Nov 2016 22:57:14 -0500 [thread overview]
Message-ID: <20161108035714.GH1555@brightrain.aerifal.cx> (raw)
In-Reply-To: <CAFhhQJRkE+NnVdmrOUVqm2XLKksP59DW4HM2=nAbR8PLb-gi_A@mail.gmail.com>
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
prev parent reply other threads:[~2016-11-08 3:57 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-03 2:29 Daniel Sabogal
2016-11-07 17:09 ` Rich Felker
2016-11-08 3:40 ` Daniel Sabogal
2016-11-08 3:57 ` Rich Felker [this message]
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=20161108035714.GH1555@brightrain.aerifal.cx \
--to=dalias@libc.org \
--cc=musl@lists.openwall.com \
/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.
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).