Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 7 Mar 2002 15:16:11 +0000 (GMT)
From:      Mike Silbersack <silby@silby.com>
To:        Mark Murray <mark@grondar.za>
Cc:        Alfred Perlstein <bright@mu.org>, <cvs-committers@FreeBSD.ORG>, <cvs-all@FreeBSD.ORG>
Subject:   Re: cvs commit: src/usr.bin/rwall rwall.c 
Message-ID:  <20020307150621.A3443-100000@patrocles.silby.com>
In-Reply-To: <200203072049.g27KnKRV073901@grimreaper.grondar.org>

next in thread | previous in thread | raw e-mail | index | archive | help

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




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020307150621.A3443-100000>