478161 - 5th arg of mktime should begin at zero

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Atsushi Sakai]

Attachment: Patch v1.0 [Checkin: Comment 4]

January should be 0, not 1.

Comment 1

[(not currently active) Ted Mielczarek]

Comment on attachment 361935 [details] [diff] [review]
Patch v1.0
[Checkin: Comment 4]

I like how even the Perl manual gets this wrong though, after explaining that it starts from zero:
Calendar time for December 12, 1995, at 10:30 am.
        $time_t = POSIX::mktime( 0, 30, 10, 12, 11, 95 );

+    return (POSIX::mktime(00, 00, 00, $d, $m - 1, $y - 1900) -
+            POSIX::mktime(00, 00, 00, 01, 00, 100)) / $secondstodays;

If you're going to do this, then you're going to need to change that 01 to a 00 on the second line, since we're intending to calculate the days since January 1, 2000. Without your patch we correctly calculate it (because both months are off by 1), but with your patch as it stands we'll by off by a month.

Comment 2

[Atsushi Sakai]

Comment on attachment 361935 [details] [diff] [review]
Patch v1.0
[Checkin: Comment 4]

re-requesting review.

(In reply to comment #1)
> I like how even the Perl manual gets this wrong though, after explaining that
> it starts from zero:
> Calendar time for December 12, 1995, at 10:30 am.
>         $time_t = POSIX::mktime( 0, 30, 10, 12, 11, 95 );

It's not wrong. December is 11.

> If you're going to do this, then you're going to need to change that 01 to a 00

It has been already changed.
-            POSIX::mktime(00, 00, 00, 01, 01, 100)) / $secondstodays;
+            POSIX::mktime(00, 00, 00, 01, 00, 100)) / $secondstodays;

> Without your patch we correctly calculate it (because both months are
> off by 1)

No, it's not true because the length of the month differs between each month.
For example,
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2009-01-31-03-mozilla-central/ calculates as 3318, which is true, but
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2009-02-01-04-mozilla-central/ calculates as 3316(decreased from the previous day!), should be 3319.

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 361935 [details] [diff] [review]
Patch v1.0
[Checkin: Comment 4]

You're right, I can't read. Sorry!

Comment 4

[Serge Gautherie (:sgautherie)]

Comment on attachment 361935 [details] [diff] [review]
Patch v1.0
[Checkin: Comment 4]


http://hg.mozilla.org/mozilla-central/rev/f287843bb4a1

Comment 5

[Atsushi Sakai]

Comment on attachment 361935 [details] [diff] [review]
Patch v1.0
[Checkin: Comment 4]

Requesting approval for 1.9.1.
It has been baked on trunk for a month and no problem is found.

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 361935 [details] [diff] [review]
Patch v1.0
[Checkin: Comment 4]

a191=beltzner

Comment 7

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/443fa8e59842