Hacker News Reader

Comment by cremno | Hacker News Reader

>So do implement it then to avoid the compilation and/or linking errors, but make it crash intentionally if called.

    void SimpleReferenceObject::operator delete[](void * /* pPtr */)
    {
      free(NULL);
    }

What will cause the crash? Passing a null pointer to free() is fine. Something specific to C++?

View on HN

It seems to have been introduced at the following commit: http://cgit.freedesktop.org/libreoffice/core/commit/?id=f3d9...

I think even MSVC2008 should accept free(NULL) without crashing.

My guess is that this was a case of tweaking the source code until the buggy compiler accepted it (it's within a "#ifdef _MSC_VER" block, so it's a MSVC-only workaround). He probably originally wrote the function containing something like std::abort(), but the compiler rejected it. So he changed it to a dummy (do-nothing) call to free(), and missed updating the comment he had written in the previous attempt.

Per the spec, free(NULL) is fine and well defined, essentially a no-op. Not sure what they're thinking. abort would have been an option.... And hell, were they trying to invoke UB and assuming that would cause a crash? Wrong on multiple levels.

Spec is fine and all, but free(NULL) reliably crashes on Solaris and probably other systems.

The malloc(3C) man pages since 5.5.1 claim otherwise.

I also think that code will run fine. The comment in LibreOffice's source and this blog post are incorrect.

I also wonder how far PVS-Studio goes in deciding that 'expression is always true' in this fragment:

  if( (pSymDef->GetType() != SbxEMPTY) ||
      (pSymDef->GetType() != SbxNULL) )

Is ::GetType() marked as pure in the LibreOffice source code, does PVS-Studio check the source code of its implementation(s), does PVS-Studio have heuristics for function names that likely are pure, or does it just assume any function is pure?

The GetType() is const function for simple return type:

    class SbiSymDef {
      ....
      SbxDataType GetType() const { return eType; }
      ....
    };

Call free(NULL); does not lead to any trouble. However, if you say that regularly use such statmen in your code, I'll be surprised. Analyzer surprised too. PVS suspects that it is not okay, and maybe we're dealing with some kind of typo or something else.

Yes, free(NULL) is a questionable statement. The source code comment (not the analyzer) just irritated me.

I wonder what they thought was wrong with `abort`.

Reply on news.ycombinator.com