From owner-svn-src-all@FreeBSD.ORG Mon Jun 28 13:45:47 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id C784D106566B; Mon, 28 Jun 2010 13:45:47 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail06.syd.optusnet.com.au (mail06.syd.optusnet.com.au [211.29.132.187]) by mx1.freebsd.org (Postfix) with ESMTP id 5AA708FC15; Mon, 28 Jun 2010 13:45:46 +0000 (UTC) Received: from besplex.bde.org (c122-106-145-25.carlnfd1.nsw.optusnet.com.au [122.106.145.25]) by mail06.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id o5SDjivH026243 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 28 Jun 2010 23:45:45 +1000 Date: Mon, 28 Jun 2010 23:45:44 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Gavin Atkinson In-Reply-To: <201006280830.o5S8UA3o048905@svn.freebsd.org> Message-ID: <20100628221613.J1018@besplex.bde.org> References: <201006280830.o5S8UA3o048905@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r209567 - head/usr.bin/lock X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Jun 2010 13:45:47 -0000 On Mon, 28 Jun 2010, Gavin Atkinson wrote: > Log: > Make WARNS=6 safe. This mainly breaks the warning. > Modified: head/usr.bin/lock/lock.c > ============================================================================== > --- head/usr.bin/lock/lock.c Mon Jun 28 08:10:55 2010 (r209566) > +++ head/usr.bin/lock/lock.c Mon Jun 28 08:30:10 2010 (r209567) > @@ -65,6 +65,7 @@ __FBSDID("$FreeBSD$"); > #include > #include > #include > +#include Include to support bogusness below. > #include > #include > #include > @@ -257,9 +258,9 @@ hi(int signo __unused) > if (no_timeout) { > (void)putchar('\n'); > } else { > - (void)printf("timeout in %ld:%ld minutes\n", > - (nexttime - timval.tv_sec) / 60, > - (nexttime - timval.tv_sec) % 60); > + (void)printf("timeout in %jd:%jd minutes\n", > + (intmax_t)(nexttime - timval.tv_sec) / 60, > + (intmax_t)(nexttime - timval.tv_sec) % 60); Printing time differences using intmax_t is silly. They don't need to work for more than a few days here, but even casting to 16-bit ints lets them work for 32767 minutes = 546 hours here, while the natural casts here (of timeval.tv_sec to long) lets them work for 65536 times longer than that = 4082 years even with 32-bit longs. Any casts here risk breaking the warnings about type mismatches and resulting overflows, and in fact there are many in this program. Here there is just the promotion of timval.tv_sec to time_t causing the printf args to normally not match the printf format. Elsewhere there are overflow bugs caused by incomplete conversion to time_t, and worse. Mainly here: % nexttime = timval.tv_sec + (sectimeout * 60); This has about 10 style, type mismatch and overflow bugs, counting other bugs involving sectimeout, starting with sectimeout not actually being the seconds timeout (it is the minutes timeout, and scaling it to a seconds timeout gives overflow bugs). If the program gets this far without triggering the bugs, then it has few risks of more, since the residual timeout is <= the original timeout which must be small to work. Bruce