Date: Fri, 21 Apr 2017 18:10:08 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Michael Tuexen <tuexen@freebsd.org> Cc: Bruce Evans <brde@optusnet.com.au>, Cy Schubert <Cy.Schubert@komquats.com>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r317208 - head/sys/netinet Message-ID: <20170421173453.J1735@besplex.bde.org> In-Reply-To: <8BBC38A0-DDBA-4C04-9654-98755B3E4E13@freebsd.org> References: <201704210143.v3L1h99s037727@slippy.cwsent.com> <20170421131041.G966@besplex.bde.org> <8BBC38A0-DDBA-4C04-9654-98755B3E4E13@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 21 Apr 2017, Michael Tuexen wrote: >> On 21. Apr 2017, at 05:16, Bruce Evans <brde@optusnet.com.au> wrote: >> >> On Thu, 20 Apr 2017, Cy Schubert wrote: >> >> Please trim quotes. >> >>> In message <201704201919.v3KJJYko052651@repo.freebsd.org>, Michael Tuexen >>> write >>> s: >> >> [>> ... 5 lines trimmed] >> >>>> Log: >>>> Syncoockies can be used in combination with the syncache. If the cache >>>> overflows, syncookies are used. >> >> [>> ... 16 lines trimmed] >> >>>> Modified: head/sys/netinet/tcp_syncache.c >>>> ============================================================================= >>>> = >>>> --- head/sys/netinet/tcp_syncache.c Thu Apr 20 19:14:52 2017 (r31720 >>>> 7) >>>> +++ head/sys/netinet/tcp_syncache.c Thu Apr 20 19:19:33 2017 (r31720 >>>> 8) >>>> @@ -260,6 +260,7 @@ syncache_init(void) >>>> &V_tcp_syncache.hashbase[i].sch_mtx, 0); >>>> V_tcp_syncache.hashbase[i].sch_length = 0; >>>> V_tcp_syncache.hashbase[i].sch_sc = &V_tcp_syncache; >>>> + V_tcp_syncache.hashbase[i].sch_last_overflow = INT64_MIN; >>> ... >>> This line produced the following on i386: >>> >>> /opt/src/svn-current/sys/netinet/tcp_syncache.c:263:50: error: implicit >>> conversion from 'long long' to 'time_t' (aka 'int') changes value from >>> -9223372036854775808 to 0 [-Werror,-Wconstant-conversion] >>> V_tcp_syncache.hashbase[i].sch_last_overflow = INT64_MIN; >>> ~ ^~~~~~~~~ >>> ./x86/_stdint.h:91:41: note: expanded from macro 'INT64_MIN' >>> #define INT64_MIN (-0x7fffffffffffffffLL-1) >>> ~~~~~~~~~~~~~~~~~~~~~^~ >>> >>> Looks like it needs a time_t cast. >> >> A cast would just break the warning. INT64_MIN has nothing to do with >> time_t. The expression (time_t)INT64_MIN would be more obviously garbage >> since in the above it is not clear that sch_last_overflow has a type >> unrelated to the constant. > Fixed in https://svnweb.freebsd.org/changeset/base/317244 >> >> INT64_MIN is also broken if time_t is 64 bits but unsigned. Then it is >> converted to half of plus infinity instead of the intended minus infinity. > The patch assumes that time_t is signed, which is true for FreeBSD on all > platforms. Or am I missing a platform? Only future platforms. i386 should use time_t = uint32_t to fully support years 2038-2106 instead of time_t = int32_t to partially support years 1902-1970 (even time 0 (the Epoch) and other early hours in 1970 are not fully supported now and would be broken by unsigned time_t, since subtraction of the timezone offset from 0 gives negative values with signed time_t and overflowing values with unsigned time_t). (time_t)-1 is special, so the time 1 second before the Epoch cannot work with signed time_t. This value works better with unsigned time_t because it is not in the middle of the range, but times before the Epoch are just unrepresentable with unsigned time_t. Changing the signedness of time_t would break the ABI less than changing its size, but it still causes problems with buggy software which assumes that time_t is signed or encodes special values in it. Negative times are at best unspecified by POSIX. They give a large range of magic out of band values below 0, provided nothing assumes that the system is better designed than POSIX so supports times before the Epoch. Even the Standard C library is not that bad, except POSIX forces a bad design for time_t so mktime() and friends are restricted to times. If time_t is unsigned, then no times before the Epoch can work, and if it is signed then times before the Epoch are unportable. netinet could use non-negative times far in the past as out-of-band values if there are any. But it mostly uses monotonic times. The Epoch for monotonic times starts at boot time (modulo other bugs), so there are not enough times far enough in the past shortly after booting. If time_t is unsigned, there just aren't enough, and if it is signed there are only enough by using unportable negative times. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170421173453.J1735>