You make the point indeed I had "overreacted", just tried to participate :-)
>I mght be wrong, but why not just initialize to NULL? I am watching this thread to spot people who should never be OpenBSD developers.. Eheh you re probably right. On 24 September 2014 13:22, Ingo Schwarze <schwa...@usta.de> wrote: > Hi David, > > David Carlier wrote on Wed, Sep 24, 2014 at 10:19:57AM +0100: > > On 24 September 2014 10:10, Matti Karnaattu <mkarnaa...@gmail.com> > wrote: > > >> I noticed that chmod.c have uninitialized variable char *ep > >> that was used. This diff clarify what I mean. > > > I might be wrong, > > You are. > > > but why not just initialize to NULL? > > That is exactly how one must *not*, under any circumstances, ever > react to the findings of an audit. It is very important to not > react in such a way. > > If an audit - no matter whether a manual audit (like here) or > tool-driven static analysis - reveals a potential bug or vulnerability, > *never* proceed to merely sweep it under the carpet without caring > what the code actually does. For example, if your auditing tool > reports "potential use of uninitialized variable", do *not* just > add "foo = NULL" to the top of the function. Look at what the > function does and decide whether it makes the codes safer or more > readable to initialize. > > Adding code to merely shut up an audit is harmful in more than one > way. First, it may hide an actually problem. Second, it may create > a new problem if you happen to pick the wrong value for initialization. > Third, it might in somes cases shift a problem elsewehere where it > might be more harmful or harder to detect. > > I think you have earned yourself a flame, but i'm sorry to say i'm > not in the mood right now. :-) > > > In the case at hand, the code is obviously correct as it stands. > Just search for "ep" from the top. The first line you find is the > declaration, the second line is the strtoul() explicitly initializing > it. Adding a second initialization before would be confusing. Any > future auditor would wonder what the point of that obviously useless > statement is and would probably waste some time trying to figure > out whether it hides some logic oversight of the author of the code, > probably ending up *removing* the "ep = NULL" to improve clarity > for future audits. > > Yours, > Ingo > > > [ incorrect patch snipped ] >
