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 ]
