[Bug 71043] Use STACK lint tool to clean code ...

https://bugs.freedesktop.org/show_bug.cgi?id=71043

--- Comment #5 from Tor Lillqvist <tml@iki.fi> ---
Whee, yes, the results looks indeed quite interesting, even if it takes a
(short) while to understand what they are trying to say. Just look at the first
thing reported, in sal/osl/unx/pipe.c:

in __osl_createPipeImpl(): 

    pPipeImpl = (oslPipe)calloc(1, sizeof(struct oslPipeImpl));
    pPipeImpl->m_nRefCount =1;
    ...
    return pPipeImpl;

Note that there is no check whether calloc() fails. The code immediately
dereferences the returned pointer (which might in theory be NULL, in which case
the dereferencing will cause a crash of course). The pointer is returned as the
function value.

So let's look at the reported call site of __osl_createPipeImpl():

        pAcceptedPipe= __osl_createPipeImpl();

        OSL_ASSERT(pAcceptedPipe);
        if(pAcceptedPipe==NULL)
        {
            close(s);
            return NULL;
        }

Ha! The checks whether pAcceptedPipe is NULL are totally pointless as we have
already dereferenced it inside __osl_createPipeImpl(), so it can't be NULL, and
the whole if statement will be optimised away by a modern compiler.

Now, what to do in this particular case then is a question of taste, more or
less.

Personally I think that if a memory allocation (of a small amount of memory)
fails at this place in the code, which as far as I know is invoked mainly
(only?) very early when LO is starting, something is very wrong and there is no
point in even trying to recover gracefully from the memory allocation failure.
So let __osl_createPipeImpl() be as is and remove the pointless checks for NULL
return after the calls to it.

But I assume there is an opposite opinion, too, that each and every memory
allocation should be checked and the code should do its utmost to fail
gracefully... In that case, the check for memory allocation failure should be
moved inside __osl_createPipeImpl() right after the call to calloc(), and in
case of failure, NULL should be returned immediately. Hopefully then the return
value of __osl_createPipeImpl() is checked in each case.

-- 
You are receiving this mail because:
You are on the CC list for the bug.