<html> <head> <base href="https://bugs.freedesktop.org/" /> </head> <body> <div> <a class="bz_bug_link bz_status_NEW " title="NEW --- - Use STACK lint tool to clean code ..." href="https://bugs.freedesktop.org/show_bug.cgi?id=71043#c5">Comment # 5</a> on <a class="bz_bug_link bz_status_NEW " title="NEW --- - Use STACK lint tool to clean code ..." href="https://bugs.freedesktop.org/show_bug.cgi?id=71043">bug 71043</a> from <a class="email" href="mailto:tml@iki.fi" title="Tor Lillqvist <tml@iki.fi>"> Tor Lillqvist</a> <pre>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.</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are on the CC list for the bug.</li> </ul> </body> </html>