| msg283937 - (view) |
Author: Decorater (Decorater) * |
Date: 2016-12-24 10:41 |
| I fixed some Possible Loss of Data warnings. but only the ones for timemodule. There are still some others when building the 64 bit version however. I added some comments that can be removed in another patch if desired otherwise it should look good for now for this module. |
|
|
| msg284460 - (view) |
Author: Steve Dower (steve.dower) *  |
Date: 2017-01-02 04:44 |
| Any reason we can't make gmtoff a time_t instead of an int? If we're going to truncate values to get rid of the warnings, I'd like to also see either proof that it will never exceed the size of an int (which may be a simple comment, but it's not obvious that this is the case from what appears in the patch), or an assertion when it does overflow. But since we're presumably passing the value back into Python as an int, expanding the destination variable to fit all possible values is best. |
|
|
| msg284553 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2017-01-03 12:39 |
| Downcasting to int doesn't seem good to me. gmtoff_time_t.patch uses time_t instead of an int to store gmtoff. It raises an OverflowError if the value doesn't fit into a C long (at least max is at least 2^31). I guess that the issue only impacts Windows, I expect that most platforms have a tm_zone field in the "tm" structure? /* Define to 1 if `tm_zone' is a member of `struct tm'. */ #define HAVE_STRUCT_TM_TM_ZONE 1 |
|
|
| msg284554 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2017-01-03 12:41 |
| The warning was introduced by a recent change: changeset: 103680:a96101dd105c user: Alexander Belopolsky <alexander.belopolsky@gmail.com> date: Sun Sep 11 22:55:16 2016 -0400 files: Doc/library/time.rst Misc/NEWS Modules/timemodule.c description: Closes #25283: Make tm_gmtoff and tm_zone available on all platforms. |
|
|
| msg284555 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2017-01-03 12:47 |
| Not checking for integer overflow is more likely to hide bugs. Handling time is an hard problem, see a recent funny (or not) story from Cloudfare with the latest leap second added at midnight the 2016-12-31: https://blog.cloudflare.com/how-and-why-the-leap-second-affected-cloudflare-dns/ Extract: "RRDNS is written in Go and uses Go’s time.Now() function to get the time. Unfortunately, this function does not guarantee monotonicity. Go currently doesn’t offer a monotonic time source (see issue 12914 for discussion)." Hopefully, Python has time.monotonic() since Python 3.3 ;-) |
|
|
| msg293861 - (view) |
Author: Decorater (Decorater) * |
Date: 2017-05-17 17:36 |
| I think the patch that haypo added should help take care of the warning. It does however only warned when building for 64 bit. |
|
|
| msg293874 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2017-05-17 18:55 |
| commit 0d659e5614cad512a1940125135b443b3eecb5d7 Author: Victor Stinner <victor.stinner@gmail.com> Date: Tue Apr 25 01:22:42 2017 +0200 |
|
|
| msg293889 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2017-05-17 21:48 |
| I backported the fix to 3.6. So all warnings on timemodule.c should now be fixed. Thanks for the bug report and the patch Decorater! commit 69f3a5ac28041fac86897e0c90d98ad9fd6fa3f7 Author: Victor Stinner <victor.stinner@gmail.com> Date: Wed May 17 14:45:45 2017 -0700 |
|
|