Hacker News Reader

Comment by bbwharris | Hacker News Reader

Forgive me for asking but what is wrong in particular with this single line?

To me it reads fine and the idea is that is frees that data address if its occupied. I'm sure I'm not the only one who is curious why this is bad.

View on HN

The sensitive data that was saved in that address is still there. Memory has been freed so the os can use is again but the actual data is still there is memory untill get get overwritten by something else...

The program will work with no problems, but sensitive data that has been used then freed is available for retrieval when bugs like heartbleed are found.

As the article suggests the right way is to clean the data from memory ( by overwriting it with something else) before freeing it.

I've been looking at this recently, part of the problem with that approach is that compilers will often optimise out an overwrite if they can't see anything happening afterwards.

For instance if you set a stack-resident buffer that contained a key to all zeros using memset, then simply exit the scope, most optimisations will detect it as unnecessary (wtf? this never gets read back, who cares?) and ditch the line.

Search for memset_s (part of the C11 standard) for a clear function that can survive optimisers.

Gotcha, I wasn't thinking about that.

For one, free() is a no-op on a null.

My concerns are well addressed in the patch:

           if (b->d != NULL)
           {

There's no need to explicitly include NULL in there, it buys you nothing.

Insisting on braces is a style concern, though it does seem to protect people from themselves to some extent.

If you're careful to use NULL only in pointer expressions, it can be used to indicate that a pointer is being tested, as opposed to any other sort of integer or boolean flag. You can justify it on grounds similar to Hungarian notation, giving the reader some contextual information that might otherwise require a look at a declaration somewhere else.

Code should be written for the benefit of humans, not compilers.

I agree, but the humans also need to be able to understand the code. I've worked in places where the ternary operator was banned because 'some people might not understand it'. This is silly.

Yeah, that's pretty goofy. Being intimidated by the ternary operator is how you know you should have gone into marketing instead.

I don't think you should be testing for truth implicitly like that, try

if ((bool(b->d != NULL) == true) { }

much better to be explicit, don't you think?

Assuming this is satire, LOL :)

Reply on news.ycombinator.com