classic VM: timezone correction in sqUnixUtcWithOffset()

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

classic VM: timezone correction in sqUnixUtcWithOffset()

stes
 
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256


Hi,

Here's another fix for sqUnixMain.c in the SVN (subversion) repository.

This is for timezone correction in sqUnixUtcWithOffset().

Basically there is code for the Linux/glibc case which is correct:

 #if defined(HAVE_TM_GMTOFF)
   *offset= localtime(&seconds)->tm_gmtoff;
 #else

The above code is correct but it depends on tm_gmtoff (glibc) as on Linux.

The incorrect code is for platforms that use the plain standard C library,
without the glibc extension (the #else clause).

The timezone (HAVE_TIMEZONE) is itself also extension which is enabled
by default for Solaris because it is _XOPEN_SOURCE but not _STRICT_STDC.

- -  {
- -    struct tm *local= localtime(&seconds);
- -    struct tm *gmt= gmtime(&seconds);
- -    int d= local->tm_yday - gmt->tm_yday;
- -    int h= ((d < -1 ? 24 : 1 < d ? -24 : d * 24) + local->tm_hour - gmt->tm_hour);
- -    int m= h * 60 + local->tm_min - gmt->tm_min;
- -    *offset= m * 60;
- -  }

The above is incorrect because localtime() and gmtime() will overwrite the
same buffer, so the gmtime() call erases the value of localtime().

I have a patch in attach that uses the same logic as plugins/LocalePlugin/sqLocalePlugin.c and as in convertToSqueakTime() which is doing it right.

+#ifdef HAVE_TIMEZONE
+  *offset = ((daylight) * 60 * 60) - timezone;
+#else
+#error: cannot determine timezone correction
 #endif
+#endif
   return 0;
 }
 
Please apply the patch in attach to subversion ...

I think it is safe to apply this patch because (1) it does not change
the #if HAVE_TM_GMTOFF case (the Linux case is unchanged) and (2) it is
consistent with plugins/LocalePlugin/sqLocalePlugin.c and convertToSqueakTime() and (3) the code with the difference between localtime() and gmtime() is certainly wrong.

Thanks,
David Stes

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJgha+cAAoJENdFDkXGicizymEIAIqI8cicPesACVW3zXFzWWl9
m1L8BU8CAlgP4omO+scOtza+rj7Fy4I5tg+XFNX8u1cEtJIgYj2fvEpoOqUsYkbv
/kzlQX850TGWKYFA5ya3RE7XRI3fx5hqqP2ntEd3Nb0Ve9yFsmlo3KxKRSkJMKfm
9Jwgrpu549AqwR+Hl+XkJxIiJ5xBotJyzipyB6ySXGqpCYs4qVJl0nM00ozVNY7n
W7AhNh6mWX1pbvZLH9xPVO0qSRLNewQ9sGBbaGIAaQm7HScSHpM9DtDE3jY/FF0J
CdNH84hKYdN7z8am1C1IDX95V4sILORyFaaa0F+QnOBFASMW1ltfkx+up6yHzXU=
=4EbY
-----END PGP SIGNATURE-----

sqUnixMain-timezone.patch (936 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: classic VM: timezone correction in sqUnixUtcWithOffset()

David T. Lewis
 
Hi David,

Thanks for the fix and for the clear explanation of the problem. I pushed
your patch to SVN r3801:

  http://squeakvm.org/cgi-bin/viewvc.cgi/squeak/trunk/platforms/unix/vm/sqUnixMain.c?r1=3800&r2=3801

Dave


On Sun, Apr 25, 2021 at 08:09:56PM +0200, [hidden email] wrote:

>  
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
>
> Hi,
>
> Here's another fix for sqUnixMain.c in the SVN (subversion) repository.
>
> This is for timezone correction in sqUnixUtcWithOffset().
>
> Basically there is code for the Linux/glibc case which is correct:
>
>  #if defined(HAVE_TM_GMTOFF)
>    *offset= localtime(&seconds)->tm_gmtoff;
>  #else
>
> The above code is correct but it depends on tm_gmtoff (glibc) as on Linux.
>
> The incorrect code is for platforms that use the plain standard C library,
> without the glibc extension (the #else clause).
>
> The timezone (HAVE_TIMEZONE) is itself also extension which is enabled
> by default for Solaris because it is _XOPEN_SOURCE but not _STRICT_STDC.
>
> - -  {
> - -    struct tm *local= localtime(&seconds);
> - -    struct tm *gmt= gmtime(&seconds);
> - -    int d= local->tm_yday - gmt->tm_yday;
> - -    int h= ((d < -1 ? 24 : 1 < d ? -24 : d * 24) + local->tm_hour - gmt->tm_hour);
> - -    int m= h * 60 + local->tm_min - gmt->tm_min;
> - -    *offset= m * 60;
> - -  }
>
> The above is incorrect because localtime() and gmtime() will overwrite the
> same buffer, so the gmtime() call erases the value of localtime().
>
> I have a patch in attach that uses the same logic as plugins/LocalePlugin/sqLocalePlugin.c and as in convertToSqueakTime() which is doing it right.
>
> +#ifdef HAVE_TIMEZONE
> +  *offset = ((daylight) * 60 * 60) - timezone;
> +#else
> +#error: cannot determine timezone correction
>  #endif
> +#endif
>    return 0;
>  }
>  
> Please apply the patch in attach to subversion ...
>
> I think it is safe to apply this patch because (1) it does not change
> the #if HAVE_TM_GMTOFF case (the Linux case is unchanged) and (2) it is
> consistent with plugins/LocalePlugin/sqLocalePlugin.c and convertToSqueakTime() and (3) the code with the difference between localtime() and gmtime() is certainly wrong.
>
> Thanks,
> David Stes
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
>
> iQEcBAEBCAAGBQJgha+cAAoJENdFDkXGicizymEIAIqI8cicPesACVW3zXFzWWl9
> m1L8BU8CAlgP4omO+scOtza+rj7Fy4I5tg+XFNX8u1cEtJIgYj2fvEpoOqUsYkbv
> /kzlQX850TGWKYFA5ya3RE7XRI3fx5hqqP2ntEd3Nb0Ve9yFsmlo3KxKRSkJMKfm
> 9Jwgrpu549AqwR+Hl+XkJxIiJ5xBotJyzipyB6ySXGqpCYs4qVJl0nM00ozVNY7n
> W7AhNh6mWX1pbvZLH9xPVO0qSRLNewQ9sGBbaGIAaQm7HScSHpM9DtDE3jY/FF0J
> CdNH84hKYdN7z8am1C1IDX95V4sILORyFaaa0F+QnOBFASMW1ltfkx+up6yHzXU=
> =4EbY
> -----END PGP SIGNATURE-----