Cppcheck Possible null pointer dereference:

Question

i am just using cppcheck the code is working properly just cppcheck gives this errors.

void WorkerThread(WorkBuffer* m_buffer)
{
    std::cout << "Thread : " << m_buffer->m_id << ".....Starting" << std::endl;

    if (NULL == m_buffer)
        std::cout << "Thread : " << m_buffer->m_id << "......work buffer is null" << std::endl;


    while(!shut_down_flag)
    {
        int k = 0;
        //Sleep(1);
        SleepSystemUsec(100000);
        std::cout << "Thread : " << m_buffer->m_id << "....in while loop" << std::endl;
    } // of while(!shut_down_flag)

    std::cout << "Thread : " << m_buffer->m_id << ".....Request from main thread so ending working thread ...." << std::endl;
};

error : : Possible null pointer dereference: m_buffer - otherwise it is redundant to check it against null.

Sicklebill

More code please & CPPCheck command line options supplied. – Sabbatarian

You're checking if m_buffer is NULL after you've already used it. The message is pointing out (correctly) that if it could be NULL (and if not, why are you checking?) you should find out before using it in the line above. – Octroi

@PaulRoub: That should be an answer not a comment. – Quarles

Answer 1

if (NULL == m_buffer) 

makes sure m_buffer is NULL, and then you derefence it with

std::cout << "Thread : " << m_buffer->m_id << "......work buffer is null" << std::endl;
                            ^^^^^^^^^^^^^^^

this, which is only legal if m_buffer is not NULL (more precisely, only if it points to a correctly constructed WorkBuffer).

If NULL is a possible input for your function, you need to check for it before the very first dereference and then either make it point to something valid or leave the function without dereferencing.

Tosh

couldn get what you said. would you please provide some details – Sicklebill

@Sicklebill Could you be a little bit more specific with what you don't get? – Tosh

In fact, the cppcheck warning is for the previous m_buffer->m_id << ".....Starting" before the check. It doesn't spot this error. – Peeved

@Peeved Well this is the same problem as the other one (covered by my "check for it before the very first dereference " remark): Either the pointer can be NULL, then check before you use it, or it cannot, then you never need to check. – Tosh

Answer 2

Not only is your condition backwards:

if m_buffer is NULL:
  do things that dereference m_buffer
(huh?!)

but you have no checks on any of the other output statements.

Quarles