Introduction

cd /opt git clone https://review.haiku-os.org/buildtools git clone https://review.haiku-os.org/haiku cd ./haiku mkdir generated.x86_64; cd generated.x86_64 ../configure --distro-compatibility official -j12 \ --build-cross-tools x86_64 ../../buildtools cd ../../buildtools/jam make all cd /opt/haiku/generated.x86_64 pvs-studio-analyzer trace -- /opt/buildtools/jam/bin.linuxx86/jam \ -q -j1 @nightly-anyboot pvs-studio-analyzer analyze -l /mnt/svn/PVS-Studio.lic -r /opt/haiku \ -C x86_64-unknown-haiku-gcc -o /opt/haiku/haiku.log -j12

Uninitialized variables

int auto_fetch(int argc, char *argv[]) { volatile int argpos; int rval; // <= argpos = 0; if (sigsetjmp(toplevel, 1)) { if (connected) disconnect(0, NULL); if (rval > 0) // <= rval = argpos + 1; return (rval); } .... }

struct addrinfo { int ai_flags; int ai_family; int ai_socktype; int ai_protocol; socklen_t ai_addrlen; char *ai_canonname; struct sockaddr *ai_addr; struct addrinfo *ai_next; }; static int sourceroute(struct addrinfo *ai, char *arg, char **cpp, int *lenp, int *protop, int *optp) { static char buf[1024 + ALIGNBYTES]; char *cp, *cp2, *lsrp, *ep; struct sockaddr_in *_sin; #ifdef INET6 struct sockaddr_in6 *sin6; struct ip6_rthdr *rth; #endif struct addrinfo hints, *res; // <= int error; char c; if (cpp == NULL || lenp == NULL) return -1; if (*cpp != NULL) { switch (res->ai_family) { // <= case AF_INET: if (*lenp < 7) return -1; break; .... } } .... }

void BTextView::_ApplyStyleRange(...., const BFont* font, ....) { if (font != NULL) { BFont normalized = *font; _NormalizeFont(&normalized); font = &normalized; } .... fStyles->SetStyleRange(fromOffset, toOffset, fText->Length(), mode, font, color); }

int8 BUnicodeChar::Type(uint32 c) { BUnicodeChar(); return u_charType(c); }

V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 37

V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 49

V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 58

V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 67

V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 77

V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 89

V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 103

V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 115

V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 126

V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 142

V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 152

V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 163

V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 186

V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 196

V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 206

V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 214

V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 222

V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 230

Painter::Painter() : fInternal(fPatternHandler), .... fPatternHandler(), .... { .... }; class Painter { .... private: mutable PainterAggInterface fInternal; // line 336 bool fSubpixelPrecise : 1; bool fValidClipping : 1; bool fDrawingText : 1; bool fAttached : 1; bool fIdentityTransform : 1; Transformable fTransform; float fPenSize; const BRegion* fClippingRegion; drawing_mode fDrawingMode; source_alpha fAlphaSrcMode; alpha_function fAlphaFncMode; cap_mode fLineCapMode; join_mode fLineJoinMode; float fMiterLimit; PatternHandler fPatternHandler; // line 355 mutable AGGTextRenderer fTextRenderer; };

Suspicious #define

#define TQ_LOCK(tq) \ do { \ if ((tq)->tq_spin) \ mtx_lock_spin(&(tq)->tq_mutex); \ else \ mtx_lock(&(tq)->tq_mutex); \ } while (0) #define TQ_ASSERT_LOCKED(tq) mtx_assert(&(tq)->tq_mutex, MA_OWNED) #define TQ_UNLOCK(tq) \ do { \ if ((tq)->tq_spin) \ mtx_unlock_spin(&(tq)->tq_mutex); \ else \ mtx_unlock(&(tq)->tq_mutex); \ } while (0) void grouptask_block(struct grouptask *grouptask) { .... TQ_LOCK(queue); gtask->ta_flags |= TASK_NOENQUEUE; gtaskqueue_drain_locked(queue, gtask); TQ_UNLOCK(queue); }

void grouptask_block(struct grouptask *grouptask) { .... do { if ((queue)->tq_spin) mtx_lock(&(queue)->tq_mutex); else mtx_lock(&(queue)->tq_mutex); } while (0); gtask->ta_flags |= 0x4; gtaskqueue_drain_locked(queue, gtask); do { if ((queue)->tq_spin) mtx_unlock(&(queue)->tq_mutex); else mtx_unlock(&(queue)->tq_mutex); } while (0); }

/* on FreeBSD these are different functions */ #define mtx_lock_spin(x) mtx_lock(x) #define mtx_unlock_spin(x) mtx_unlock(x)

Errors with the free function

void MimeType::_PurgeProperties() { fShort.Truncate(0); fLong.Truncate(0); fPrefApp.Truncate(0); fPrefAppSig.Truncate(0); fSniffRule.Truncate(0); delete fSmallIcon; fSmallIcon = NULL; delete fBigIcon; fBigIcon = NULL; fVectorIcon = NULL; // <= free(fVectorIcon); // <= fExtensions.clear(); fAttributes.clear(); }

static settings_handle * load_driver_settings_from_file(int file, const char *driverName) { .... handle = new_settings(text, driverName); if (handle != NULL) { // everything went fine! return handle; } free(handle); // <= .... }

void* _GetBuffer() { .... void* buffer = malloc(fBufferSize); if (buffer == NULL && !fBuffers.AddItem(buffer)) { free(buffer); throw std::bad_alloc(); } return buffer; }

Errors with the delete operator

class Err { public: .... private: char *fMsg; ssize_t fPos; }; void Err::Unset() { delete fMsg; // <= fMsg = __null; fPos = -1; } void Err::SetMsg(const char *msg) { if (fMsg) { delete fMsg; // <= fMsg = __null; } if (msg) { fMsg = new(std::nothrow) char[strlen(msg)+1]; // <= if (fMsg) strcpy(fMsg, msg); } }

status_t vm_page_write_modified_page_range(....) { .... PageWriteWrapper* wrapperPool = new(malloc_flags(allocationFlags)) PageWriteWrapper[maxPages + 1]; PageWriteWrapper** wrappers = new(malloc_flags(allocationFlags)) PageWriteWrapper*[maxPages]; if (wrapperPool == NULL || wrappers == NULL) { free(wrapperPool); // <= free(wrappers); // <= wrapperPool = stackWrappersPool; wrappers = stackWrappers; maxPages = 1; } .... }

class PageWriteWrapper { public: PageWriteWrapper(); ~PageWriteWrapper(); void SetTo(vm_page* page); bool Done(status_t result); private: vm_page* fPage; struct VMCache* fCache; bool fIsActive; }; PageWriteWrapper::PageWriteWrapper() : fIsActive(false) { } PageWriteWrapper::~PageWriteWrapper() { if (fIsActive) panic("page write wrapper going out of scope but isn't completed"); }

class PCL6Rasterizer : public Rasterizer { public: .... ~PCL6Rasterizer() { delete fOutBuffer; fOutBuffer = NULL; } .... virtual void InitializeBuffer() { fOutBuffer = new uchar[fOutBufferSize]; } private: uchar* fOutBuffer; int fOutBufferSize; };

void Hashtable::MakeEmpty(int8 keyMode,int8 valueMode) { .... for (entry = fTable[index]; entry; entry = next) { switch (keyMode) { case HASH_EMPTY_DELETE: // TODO: destructors are not called! delete (void*)entry->key; break; case HASH_EMPTY_FREE: free((void*)entry->key); break; } switch (valueMode) { case HASH_EMPTY_DELETE: // TODO: destructors are not called! delete entry->value; break; case HASH_EMPTY_FREE: free(entry->value); break; } next = entry->next; delete entry; } .... }

Functions without return value

BReference& operator=(const BReference<const Type>& other) { fReference = other.fReference; }

V591 Non-void function should return a value. Referenceable.h 233

V591 Non-void function should return a value. Referenceable.h 239

void errx(int, const char *, ...) ; char * getoptionvalue(const char *name) { struct option *c; if (name == NULL) errx(1, "getoptionvalue() invoked with NULL name"); c = getoption(name); if (c != NULL) return (c->value); errx(1, "getoptionvalue() invoked with unknown option '%s'", name); /* NOTREACHED */ }

Handling exceptions

size_t Response::ExtractNumber(BDataIO& stream) { BString string = ExtractString(stream); const char* end; size_t number = strtoul(string.String(), (char**)&end, 10); if (end == NULL || end[0] != '\0') ParseException("Invalid number!"); return number; }

int main(int argc, char** argv) { try { return Main().Run(argc, argv); } catch (Exception& exception) { // <= fprintf(stderr, "%s

", exception.what()); return 1; } } int Run(int argc, char** argv) { .... _ParseSyscalls(argv[1]); .... } void _ParseSyscalls(const char* filename) { ifstream file(filename, ifstream::in); if (!file.is_open()) throw new IOException(string("Failed to open '") + filename + "'."); // <= .... }

V1022 An exception was thrown by pointer. Consider throwing it by value instead. gensyscallinfos.cpp 347

V1022 An exception was thrown by pointer. Consider throwing it by value instead. gensyscallinfos.cpp 413

Formal security

#ifndef SAFE_FREE #define SAFE_FREE(a) \ do{if(a != NULL){memset(a,0, sizeof(*a)); free(a); a=NULL;}} while (0) .... #endif DST_KEY * dst_free_key(DST_KEY *f_key) { if (f_key == NULL) return (f_key); if (f_key->dk_func && f_key->dk_func->destroy) f_key->dk_KEY_struct = f_key->dk_func->destroy(f_key->dk_KEY_struct); else { EREPORT(("dst_free_key(): Unknown key alg %d

", f_key->dk_alg)); } if (f_key->dk_KEY_struct) { free(f_key->dk_KEY_struct); f_key->dk_KEY_struct = NULL; } if (f_key->dk_key_name) SAFE_FREE(f_key->dk_key_name); SAFE_FREE(f_key); return (NULL); }

V597 The compiler could delete the 'memset' function call, which is used to flush 'encoded_block' buffer. The memset_s() function should be used to erase the private data. dst_api.c 446

V597 The compiler could delete the 'memset' function call, which is used to flush 'key_st' object. The memset_s() function should be used to erase the private data. dst_api.c 685

V597 The compiler could delete the 'memset' function call, which is used to flush 'in_buff' buffer. The memset_s() function should be used to erase the private data. dst_api.c 916

V597 The compiler could delete the 'memset' function call, which is used to flush 'ce' object. The memset_s() function should be used to erase the private data. fs_cache.c 1078

Comparisons with unsigned variables

status_t DwarfFile::_UnwindCallFrame(....) { .... uint64 remaining = lengthOffset + length - dataReader.Offset(); if (remaining < 0) return B_BAD_DATA; .... }

status_t snooze(bigtime_t amount) { if (amount <= 0) return B_OK; int64 secs = amount / 1000000LL; int64 usecs = amount % 1000000LL; if (secs > 0) { if (sleep((unsigned)secs) < 0) // <= return errno; } if (usecs > 0) { if (usleep((useconds_t)usecs) < 0) return errno; } return B_OK; }

extern unsigned int sleep (unsigned int __seconds); extern int usleep (__useconds_t __useconds);

Dangerous pointers

void XHCI::FreeDevice(Device *device) { uint8 slot = fPortSlots[device->HubPort()]; TRACE("FreeDevice() port %d slot %d

", device->HubPort(), slot); // Delete the device first, so it cleans up its pipes and tells us // what we need to destroy before we tear down our internal state. delete device; DisableSlot(slot); fDcba->baseAddress[slot] = 0; fPortSlots[device->HubPort()] = 0; // <= delete_area(fDevices[slot].trb_area); delete_area(fDevices[slot].input_ctx_area); delete_area(fDevices[slot].device_ctx_area); memset(&fDevices[slot], 0, sizeof(xhci_device)); fDevices[slot].state = XHCI_STATE_DISABLED; }

V774 The 'self' pointer was used after the memory was released. TranslatorRoster.cpp 884

V774 The 'string' pointer was used after the memory was released. RemoteView.cpp 1269

V774 The 'bs' pointer was used after the memory was released. mkntfs.c 4291

V774 The 'bs' pointer was used after the memory was released. mkntfs.c 4308

V774 The 'al' pointer was used after the memory was reallocated. inode.c 1155

static int malo_hal_fwload_helper(struct malo_hal *mh, char *helper) { .... /* tell the card we're done and... */ error = malo_hal_send_helper(mh, 0, NULL, 0, MALO_NOWAIT); // <= NULL .... } static int malo_hal_send_helper(struct malo_hal *mh, int bsize, const void *data, size_t dsize, int waitfor) { mh->mh_cmdbuf[0] = htole16(MALO_HOSTCMD_CODE_DNLD); mh->mh_cmdbuf[1] = htole16(bsize); memcpy(&mh->mh_cmdbuf[4], data , dsize); // <= data .... }

int command_recompress(int argc, const char* const* argv) { .... BFile* inputFileFile = new BFile; error = inputFileFile->SetTo(inputPackageFileName, O_RDONLY); if (error != B_OK) { fprintf(stderr, "Error: Failed to open input file \"%s\": %s

", inputPackageFileName, strerror(error)); return 1; } inputFile = inputFileFile; .... }

RPC::CallbackReply* ReplyBuilder::Reply() { fReply->Stream().InsertUInt(fStatusPosition, _HaikuErrorToNFS4(fStatus)); fReply->Stream().InsertUInt(fOpCountPosition, fOpCount); if (fReply == NULL || fReply->Stream().Error() == B_OK) return fReply; else return NULL; }

static void oce_mq_free(struct oce_mq *mq) { POCE_SOFTC sc = (POCE_SOFTC) mq->parent; struct oce_mbx mbx; struct mbx_destroy_common_mq *fwcmd; if (!mq) return; .... }

Miscellaneous

static void dump_acpi_namespace(acpi_ns_device_info *device, char *root, int indenting) { char output[320]; char tabs[255] = ""; .... strlcat(tabs, "|--- ", sizeof(tabs)); .... while (....) { uint32 type = device->acpi->get_object_type(result); snprintf(output, sizeof(output), "%s%s", tabs, result + depth); switch(type) { case ACPI_TYPE_INTEGER: strncat(output, " INTEGER", sizeof(output)); break; case ACPI_TYPE_STRING: strncat(output, " STRING", sizeof(output)); break; .... } .... } .... }

V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 104

V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 107

V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 110

V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 113

V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 118

V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 119

V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 120

V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 123

V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 126

V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 129

V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 132

V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 135

V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 138

V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 141

V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 144

V645 The 'strncat' function call could lead to the 'features_string' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. VirtioDevice.cpp 283

V645 The 'strncat' function call could lead to the 'features_string' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. VirtioDevice.cpp 284

V645 The 'strncat' function call could lead to the 'features_string' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. VirtioDevice.cpp 285

class DesktopListener : public DoublyLinkedListLinkImpl<DesktopListener> { public: .... virtual bool SetDecoratorSettings(Window* window, const BMessage& settings) = 0; .... }; bool DesktopObservable::SetDecoratorSettings(Window* window, const BMessage& settings) { if (fWeAreInvoking) return false; InvokeGuard invokeGuard(fWeAreInvoking); bool changed = false; for (DesktopListener* listener = fDesktopListenerList.First(); listener != NULL; listener = fDesktopListenerList.GetNext(listener)) changed = changed | listener->SetDecoratorSettings(window, settings); return changed; }

#define PCI_line_size 0x0c /* (1 byte) cache line size in 32 bit words */ static status_t wb840_open(const char* name, uint32 flags, void** cookie) { .... data->wb_cachesize = gPci->read_pci_config(data->pciInfo->bus, data->pciInfo->device, data->pciInfo->function, PCI_line_size, sizeof(PCI_line_size)) & 0xff; .... }

typedef bool BOOL; virtual BOOL IsProfessionalSpdif() { ... } #define ECHOSTATUS_DSP_DEAD 0x12 ECHOSTATUS CEchoGals::ProcessMixerFunction(....) { .... if ( ECHOSTATUS_DSP_DEAD == IsProfessionalSpdif() ) // <= { Status = ECHOSTATUS_DSP_DEAD; } else { pMixerFunction->Data.bProfSpdif = IsProfessionalSpdif(); } .... }

Conclusion

The story of how the PVS-Studio static analyzer and the Haiku OS code met goes back to the year 2015. It was an exciting experiment and useful experience for teams of both projects. Why the experiment? At that moment, we didn't have the analyzer for Linux and we wouldn't have it for another year and a half. Anyway, efforts of enthusiasts from our team have been rewarded: we got acquainted with Haiku developers and increased the code quality, widened our error base with rare bugs made by developers and refined the analyzer. Now you can check the Haiku code for errors easily and quickly.Meet the main characters of our story — the Haiku with open source code and the PVS-Studio static analyzer for C, C++, C# and Java. When we had a dig into the project analysis 4.5 years ago, we had to deal only with the compiled executable analyzer file. All infrastructure for parsing compiler parameters, running a preprocessor, paralleling the analysis and so on was taken from the utility Compiler Monitoring UI , written in C#. That utility was ported in parts to the Mono platform to be run in Linux. The Haiku project is built using the cross compiler under various OSs, except Windows. Once again, I'd like to mention the convenience and documentation completeness related to Haiku building. Also I'd like to thank Haiku developers for their help in building the project.It's much simpler to perform the analysis now. Here is the list of all commands for building and analyzing the project:By the way, the project analysis was implemented in a Docker container. Recently we've prepared new documentation on this topic: Running PVS-Studio in Docker . This can make it very easy for some companies to apply static analysis techniques for their projects. V614 Uninitialized variable 'rval' used. fetch.c 1727Thevariable hasn't been initialized on declaration, so it's comparison with the null value will lead to undefined result. If the circumstances fail, the uncertain value of thevariablecan become a return value of thefunction. V614 Uninitialized pointer 'res' used. commands.c 2873Here is a similar case of using the uninitialized variable, except thatis an uninitialized pointer that takes place here. V506 Pointer to local variable 'normalized' is stored outside the scope of this variable. Such a pointer will become invalid. TextView.cpp 5596The programmer probably needed to normalize the object using an intermediate variable. But now thepointer contains the pointer to theobject, which will be removed after exiting the scope, where the temporary object was created. V603 The object was created but it is not being used. If you wish to call constructor, 'this->BUnicodeChar::BUnicodeChar(....)' should be used. UnicodeChar.cpp 27A very common mistake among C++ programmers is to use the constructor's call supposedly to initialize/nullify class fields. In this case, modification of class fields doesn't happen, but a new unnamed object of this class is created and then immediately destroyed. Unfortunately, there are plenty of such places in the project: V670 The uninitialized class member 'fPatternHandler' is used to initialize the 'fInternal' member. Remember that members are initialized in the order of their declarations inside a class. Painter.cpp 184Another example of incorrect initialization. Class fields are initialized in the order of their declaration in the class itself. In this example, thefield will be the first to initialize using the uninitializedvalue. V523 The 'then' statement is equivalent to the 'else' statement. subr_gtaskqueue.c 191This code snippet doesn't look suspicious until you look at the preprocessor result:The analyzer is truly right —andbranches are identical. But where are theandfunctions? Macrosand thefunction are declared in one file almost next to each other, but nevertheless a replacement took place somewhere here.Searching through the files resulted only inwith the following content:Project developers should check it out whether such a replacement is correct or not. I checked this project in Linux and such replacing seemed suspicious to me. V575 The null pointer is passed into 'free' function. Inspect the first argument. setmime.cpp 727You can pass the null pointer in thefunction, but such usage is definitely suspicious. Thus, the analyzer found mixed up lines of code. First, the code author had to release the memory by thepointer, only after that assign V575 The null pointer is passed into 'free' function. Inspect the first argument. driver_settings.cpp 461This is another example of explicit passing a null pointer to thefunction. This line can be deleted, as the function exits after obtaining the pointer successfully. V575 The null pointer is passed into 'free' function. Inspect the first argument. PackageFileHeapWriter.cpp 166Someone made an error here. The ||operator has to be used instead of &&. Only in this case theexception will be thrown in case if memory allocation (using thefunction) failed. V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] fMsg;'. Err.cpp 65Thepointer is used to allocate memory for an array of characters. Theoperator is used to release the memory instead of V611 The memory was allocated using 'new' operator but was released using the 'free' function. Consider inspecting operation logics behind the 'wrapperPool' variable. vm_page.cpp 3080Hereis a function that calls. Thenconstructs the object here. As theclass is implemented in the following way:the objects destructors of this classwon't be called due to the use of thefunction to release memory. V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] fOutBuffer;'. Check lines: 26, 45. PCL6Rasterizer.h 26It's a common error to use theoperator instead ofIt is easiest to make a mistake when writing a class, as the destructor's code is often far from the memory locations. Here, the programmer incorrectly frees the memory stored by thepointer in the destructor. V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. Hashtable.cpp 207In addition to an incorrect choice betweenand, you can also run into undefined behavior when trying to clear the memory by a pointer to the void type V591 Non-void function should return a value. Referenceable.h 228An overloaded assignment operator lacks a return value. In this case, the operator will return a random value, which can lead to strange errors.Here are similar issues in other code fragments of this class: V591 Non-void function should return a value. main.c 1010A user's comment NOTREACHED doesn't mean anything here. You need to annotate functions as noreturn in order to properly write code for such scenarios. To do this, there are noreturn attributes: standard and compiler-specific. First of all, these attributes are taken into account by compilers for proper code generation or notification of certain types of bugs using warnings. Various static analysis tools also take into account attributes to improve the analysis quality. V596 The object was created but it is not being used. The 'throw' keyword could be missing: throw ParseException(FOO); Response.cpp 659The keywordwas accidentally forgotten here. Thus, theexception won't be generated while the object of this class will be simply destroyed when exiting the scope. After that, the function will continue working as if nothing happened, as if the correct number had been entered. V1022 An exception was thrown by pointer. Consider throwing it by value instead. gensyscallinfos.cpp 316The analyzer detected theexception thrown by pointer. Throwing a pointer leads to the fact that the exception won't be caught. So the exception is eventually caught by reference. In addition, usage of a pointer forces the catching side to call theoperator to destroy the created object, which hadn't been done.A couple of other fragments of code with issues: V597 The compiler could delete the 'memset' function call, which is used to flush 'f_key' object. The memset_s() function should be used to erase the private data. dst_api.c 1018The analyzer has detected suspicious code, meant for secure private data clearing. Unfortunately, themacro which expands into thecalls andassignment doesn't make the code safer, as it's all removed by the compiler right when optimizing withBy the way, it is nothing else but CWE-14 : Compiler Removal of Code to Clear Buffers.Here is the list of places, where clearing of buffers is not performed in fact: V547 Expression 'remaining < 0' is always false. Unsigned type value is never < 0. DwarfFile.cpp 1947The analyzer found an explicit comparison of the unsigned variable with negative values. Perhaps, one should compare thevariable only with null or implement a check for overflow. V547 Expression 'sleep((unsigned) secs) < 0' is always false. Unsigned type value is never < 0. misc.cpp 56To get the main point of the error, let's address signatures ofandfunctions:As we can see, thefunction returns the unsigned value and its usage in the code is incorrect. V774 The 'device' pointer was used after the memory was released. xhci.cpp 1572object isfreed by theoperator. It's quite logical for thefunction. But, for some reason, to release other resources, the already removed object is addressed.Such code is extremely dangerous and can be met in other several places: V522 Dereferencing of the null pointer 'data' might take place. The null pointer is passed into 'malo_hal_send_helper' function. Inspect the third argument. Check lines: 350, 394. if_malohal.c 350Interprocedural analysis revealed the case whenis passed to the function and thepointer with such a value is eventually dereferenced in thefunction. V773 The function was exited without releasing the 'inputFileFile' pointer. A memory leak is possible. command_recompress.cpp 119 PVS-Studio can detect memory leaks . In this example, in case of an error the memory won't be released. Someone might think that in case of errors you shouldn't bother about the memory release, as the program will still end. But it's not always so. It is a requirement for many programs to handle errors correctly and to continue working. V595 The 'fReply' pointer was utilized before it was verified against nullptr. Check lines: 49, 52. ReplyBuilder.cpp 49It's a very common mistake to dereference pointers before checking them. The V595 diagnostic almost always prevails in the number of warnings in a project. This code fragment includes dangerous usage of thepointer. V595 The 'mq' pointer was utilized before it was verified against nullptr. Check lines: 782, 786. oce_queue.c 782A similar example. Thepointer gets dereferenced several lines earlier than it's checked for null. There are a lot of similar places in the project. In some snippets, pointer's usage and check are quite far from each other, so in this article you'll find only a couple of such examples. Developers are welcome to check out other examples in the full analyzer report. V645 The 'strncat' function call could lead to the 'output' buffer overflow. The bounds should not contain the size of the buffer, but a number of characters it can hold. NamespaceDump.cpp 101The difference betweenandfunctions is not very obvious for someone who is unfamiliar with the description of these functions. Thefunction expects the size of the entire buffer as the third argument while thefunction — the size of the free space in a buffer, which requires evaluating a needed value before calling the function. But developers often forget or don't know about it. Passing the entire buffer size to thefunction can lead to buffer overflow, as the function will consider this value as an acceptable number of characters to copy. Thefunction doesn't have such a problem. But you have to pass strings, ending with terminal null so that it worked properly.Here is the entire list of dangerous places with strings: V792 The 'SetDecoratorSettings' function located to the right of the operator '|' will be called regardless of the value of the left operand. Perhaps, it is better to use '||'. DesktopListener.cpp 324Most likely, '|' and '||' operators were muddled. This error leads to unnecessary calls of thefunction. V627 Consider inspecting the expression. The argument of sizeof() is the macro which expands to a number. device.c 72Passing thevalue to theoperator looks suspicious. Perhaps, the author should have evaluated the size of an object, for example, V562 It's odd to compare a bool type value with a value of 18: 0x12 == IsProfessionalSpdif(). CEchoGals_mixer.cpp 533Thefunction returns thetype value. In doing so, the function's result is compared with the numberin the condition.We missed the release of the first Haiku beta last fall, as we were busy releasing PVS-Studio for Java. Still the nature of programming errors is such that they don't disappear if you don't search for them and don't pay attention to the code quality. Project developers used Coverity Scan , but the last run was almost two years ago. This must be upsetting to Haiku users. Even though the analysis has been configured in 2014 using Coverity, it didn't stop us from writing two long articles on errors review in 2015 ( part 1 Another Haiku review of errors is coming out soon for those who read this post up to the end. The full analyzer report will be sent to developers before posting this errors review, so some errors might be fixed by the time you're reading this. To pass the time between the articles, I suggest downloading and trying PVS-Studio for your project.Do you want to try Haiku and you have questions? Haiku developers invite you to the telegram-channel