From owner-svn-src-head@freebsd.org Fri Apr 21 08:10:25 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id CA8EFD494E1; Fri, 21 Apr 2017 08:10:25 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail106.syd.optusnet.com.au (mail106.syd.optusnet.com.au [211.29.132.42]) by mx1.freebsd.org (Postfix) with ESMTP id 628DB1C49; Fri, 21 Apr 2017 08:10:25 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from besplex.bde.org (c122-106-153-191.carlnfd1.nsw.optusnet.com.au [122.106.153.191]) by mail106.syd.optusnet.com.au (Postfix) with ESMTPS id D7DEC3C6324; Fri, 21 Apr 2017 18:10:15 +1000 (AEST) Date: Fri, 21 Apr 2017 18:10:08 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Michael Tuexen cc: Bruce Evans , Cy Schubert , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r317208 - head/sys/netinet In-Reply-To: <8BBC38A0-DDBA-4C04-9654-98755B3E4E13@freebsd.org> Message-ID: <20170421173453.J1735@besplex.bde.org> References: <201704210143.v3L1h99s037727@slippy.cwsent.com> <20170421131041.G966@besplex.bde.org> <8BBC38A0-DDBA-4C04-9654-98755B3E4E13@freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=VbSHBBh9 c=1 sm=1 tr=0 a=Tj3pCpwHnMupdyZSltBt7Q==:117 a=Tj3pCpwHnMupdyZSltBt7Q==:17 a=kj9zAlcOel0A:10 a=PO7r1zJSAAAA:8 a=6I5d2MoRAAAA:8 a=Mk1zjkTLLVCeTCkj1yEA:9 a=CjuIK1q_8ugA:10 a=IjZwj45LgO3ly-622nXo:22 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 21 Apr 2017 08:10:25 -0000 On Fri, 21 Apr 2017, Michael Tuexen wrote: >> On 21. Apr 2017, at 05:16, Bruce Evans 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