0

I have come across a code during review and when i used cpp check it states an null pointer dereference error. I am unable to figure out the reason. Below is the code:

CopyMemory(NULL, dummyMatrixManager.GetConstDataPtr(), dummyMatrixManager.GetNumberOfElements() * sizeof(tFloat)); 

void CopyMemory( tFloat* pDst, const tFloat* pSrc, const tSize nBytes )
{
    // copy data if pointer to this memory is valid
    if (NULL != pDst)
    {
        memcpy(pDst, pSrc, nBytes);
    }
    else
    {
        LOG_ERROR("No Data copied because memory was not properly allocated. Destination pointer was set to NULL.");
    }
}

Thank you

Joseph D.
  • 11,804
  • 3
  • 34
  • 67
  • 1
    Strange code. Especially this condition: `(NULL != pDst)`. – Ron Jun 21 '18 at 08:54
  • @Ron: Yes..very strange –  Jun 21 '18 at 08:57
  • 1
    `NULL` is and has been standard C++ since its inception. Not sure why commenters are having trouble with it. @Ron What's strange about checking if `NULL` is passed in? –  Jun 21 '18 at 08:57
  • @hvd I am used to seeing literals/rvalues on the right hand side. That part is confusing to me. – Ron Jun 21 '18 at 08:58
  • 1
    @Ron There is a fairly popular style that prefers constants on the left side of `==` comparisons, to ensure the code will fail to compile if the programmer accidentally types `=` instead. Even though I don't use that style myself, I don't think there's anything strange about it. –  Jun 21 '18 at 08:59
  • 2
    @Ron is from the olden days when compilers didn't warn about `=` in if statements. Putting NULL on the left gave a compilation error. – Richard Critten Jun 21 '18 at 09:00
  • I see, that clarifies it for me. Appreciate it. Thanks all. – Ron Jun 21 '18 at 09:01
  • Why are you checking against NULL? The idiomatic way would be `if(pDst) {...` – ravnsgaard Jun 21 '18 at 09:35

2 Answers2

3

void CopyMemory( tFloat* pDst, const tFloat* pSrc, const tSize nBytes ) has 2 parameters passed as pointers, pDst and pSrc. However, before calling memcpy(pDst, pSrc, nBytes), just pDst is checked against NULL, not pSrc too.

George Spatacean
  • 130
  • 1
  • 5
  • It does not help. Cppcheck still shows error at the CopyMemory function call line. –  Jun 21 '18 at 08:57
1

I am a Cppcheck developer. The warning might be wrong, but it's hard to say when I can't reproduce.

I hope you are using a recent version of Cppcheck (1.84 is latest).

The default text output does not say so much why Cppcheck thinks there is a NULL pointer. Could you try the --template=gcc? You should be able to see how Cppcheck reached the conclusion that there is a null pointer then.

Best regards, Daniel Marjamäki

Daniel Marjamäki
  • 2,907
  • 15
  • 16