From owner-cvs-all Thu Mar 7 13:11:50 2002 Delivered-To: cvs-all@freebsd.org Received: from patrocles.silby.com (d133.as21.nwbl0.wi.voyager.net [169.207.139.199]) by hub.freebsd.org (Postfix) with ESMTP id B7D3137B400; Thu, 7 Mar 2002 13:11:40 -0800 (PST) Received: from patrocles.silby.com (localhost [127.0.0.1]) by patrocles.silby.com (8.12.2/8.12.2) with ESMTP id g27FGCNu003819; Thu, 7 Mar 2002 15:16:12 GMT (envelope-from silby@silby.com) Received: from localhost (silby@localhost) by patrocles.silby.com (8.12.2/8.12.2/Submit) with ESMTP id g27FGBa5003816; Thu, 7 Mar 2002 15:16:11 GMT X-Authentication-Warning: patrocles.silby.com: silby owned process doing -bs Date: Thu, 7 Mar 2002 15:16:11 +0000 (GMT) From: Mike Silbersack To: Mark Murray Cc: Alfred Perlstein , , Subject: Re: cvs commit: src/usr.bin/rwall rwall.c In-Reply-To: <200203072049.g27KnKRV073901@grimreaper.grondar.org> Message-ID: <20020307150621.A3443-100000@patrocles.silby.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Thu, 7 Mar 2002, Mark Murray wrote: > What Alfred said. > > The more you can get the compiler to help you, the less stupid mistakes > you make. > > When calling foreign functions, mixing modules, getting creative with > data structures, etc, the amount of work that you can unload to the compiler > by turning on this level of warning can save boatloads of work. > > I've had _many_ bugs pointed out to me by turning on maximum warnings. > > My opinion is that for shared code, this amount of proactive help is > so wise as to be (almost) compulsory. > > M > -- > o Mark Murray I guess warnings can be helpful, but I'm still not convinced that making the compiler happy necessarily creates better code. However, I'll grant you that doing probably does help for when you want to run more advanced lint tools through in the future. What concerns me is that you don't get these changes reviewed. In most cases, reviewing isn't needed. When Warner checks in pccard changes, you know that he tried them out on 20 different systems and found that the change is important. When Matt changes the VM, you know that he's run a bunch of test programs and has proved that the change is beneficial. (And if the changes turn out to be wrong, it's clear who to mail the pointy hat to.) However, there's no possible way that you've re-tested all these utilities after having done the warns cleanup to ensure that behavior has remained unchanged. As pointed out wrt the if statement in rwall.c, you could be introducing bugs in the process of "fixing" the program. Although it may be a hassle, these sweeping changes should be reviewed by at least one person before going in. Perhaps you could do the cleanup in stages, with easy stuff (ansification, function declaration, etc) first, then more complex stuff second, with good comments. Diffing the binary could prove the first stage to be safe, and peer review could help the second part. Mike "Silby" Silbersack To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message