mktime() bug fix

Jeff Johnston jjohnstn@redhat.com
Mon Sep 1 10:40:00 GMT 2008


Craig,

  Patch applied; partly manually.  In the future, please submit your 
patchs as attachments.
Please also have your ChangeLog entry in the note as well.

Thanks,

-- Jeff J.

Howland Craig D (Craig) wrote:
>      The mktime() function has an error wherein it does not work
> properly
> in all cases if the user gives a positive tm_isdst value other than 1.
> This is at odds with the 1999 C standard declaring all positive values
> to
> mean the same thing.  (There are not problems when it is called with
> tm_isdst <= 0.)  The bug only causes an operational error if the user
> calls mktime() with tm_isdst > 1 when the final resolved date is
> actually
> standard time.
>      I have attached an updated mktime.c (which started with the
> released
> 16.0 version) that corrects the problem in the mktime.tgz archive, along
> with a ChangeLog.  In the fix I kept a side-effect of mktime() that
> changes
> any positive tm_isdst value into a 1 when it is done when the result is
> daylight time.  In the diff below, you can see that a comment was
> extended
> to point out that no correction is made for negative values.  There was
> no
> change needed in the code for this, but it is only an addition to the
> comment to make more clear what is being done.
>      Also attached are a test program and its output (run on my embedded
> target) for the 16.0 mktime() and my corrected mktime()--test_mktime.c,
> test.16.0, and test.new, respectively.  (The test program is intended
> for
> my embedded target, and needs minor tweaking to run standalone.)  It can
> be seen in the third test case that the Newlib 16.0 mktime() fails to
> adjust the user-supplied time by the DST offset when tm_isdst=2.  While
> not shown in the test outputs that I have sent, it does when tm_isdst=1.
>      Out of curiousity, I ran the test on Cygwin (which appears to be
> using Newlib 14.0) and on RHEL3.  The former surprisingly did not
> display the problem, although the latter did.
> 				Craig Howland
>
>  
> $ diff -u -p mktime.c.orig mktime.c
> --- mktime.c.orig	2005-02-25 17:31:20.000000000 -0500
> +++ mktime.c	2008-08-27 16:36:42.406250000 -0400
> @@ -9,6 +9,8 @@
>   * the fields of the structure are set to represent the specified
> calendar
>   * time. Returns the specified calendar time. If the calendar time can
> not be
>   * represented, returns the value (time_t) -1.
> + *
> + * Modifications:	Fixed tm_isdst usage - 27 August 2008 Craig
> Howland.
>   */
>  
>  /*
> @@ -157,7 +159,7 @@ mktime (tim_p)
>  {
>    time_t tim = 0;
>    long days = 0;
> -  int year, isdst;
> +  int year, isdst, tm_isdst;
>    __tzinfo_type *tz = __gettzinfo ();
>  
>    /* validate structure */
> @@ -202,7 +204,9 @@ mktime (tim_p)
>    /* compute total seconds */
>    tim += (days * _SEC_IN_DAY);
>  
> -  isdst = tim_p->tm_isdst;
> +  /* Convert user positive into 1 */
> +  tm_isdst = tim_p->tm_isdst > 0  ?  1 : tim_p->tm_isdst;
> +  isdst = tm_isdst;
>  
>    if (_daylight)
>      {
> @@ -225,8 +229,10 @@ mktime (tim_p)
>  	      isdst = (tz->__tznorth
>  		       ? (tim >= startdst_dst && tim < startstd_std)
>  		       : (tim >= startdst_dst || tim < startstd_std));
> -	      /* if user committed and was wrong, perform correction */
> -	      if ((isdst ^ tim_p->tm_isdst) == 1)
> +	      /* if user committed and was wrong, perform correction,
> but no
> +	       * correction to make if user has given negative value
> (which
> +	       * asks mktime() to determine if DST is in effect or not)
> */
> +	      if (tm_isdst >= 0  &&  (isdst ^ tm_isdst) == 1)
>  		{
>  		  /* we either subtract or add the difference between
>  		     time zone offsets, depending on which way the user
> got it
>   



More information about the Newlib mailing list