📜 ⬆️ ⬇️

How to shoot yourself in the foot in C and C ++. Haiku OS Recipe Collection

The story of the meeting of the PVS-Studio static analyzer with the code of the Haiku operating system goes back to 2015. It was an interesting experiment and useful experience for the teams of both projects. Why an experiment? There was no analyzer for Linux then and there will not be another year and a half. But the work of the enthusiasts of our team was rewarded: we met with Haiku developers and improved the quality of the code, replenished the database with rare programmer errors and refined the analyzer. Checking the Haiku code for errors now is quick and easy.

Picture 3


Introduction


The heroes of our history are the open-source Haiku operating system and the PVS-Studio static analyzer for C, C ++, C # and Java. When 4.5 years ago we started analyzing the project, we only had to work with the compiled analyzer executable file. The entire infrastructure for parsing compilation parameters, launching a preprocessor, parallelizing analysis, etc. was taken from the Compiler Monitoring UI utility in C #, which was ported in parts to the Mono platform to run on Linux. The Haiku project itself is assembled using a cross-compiler under various operating systems, except Windows. Once again, I want to note the convenience and completeness of the Haiku assembly documentation, and also thank the Haiku developers for their help in building the project.

Now analysis is much easier. The list of all the commands for building and analyzing the project looks like this:
')
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 

By the way, the analysis of the project was carried out in the Docker container. Recently, we have prepared new documentation on this topic: Running PVS-Studio in Docker . This can greatly simplify the use of static project analysis techniques for some companies.

Uninitialized variables


V614 Uninitialized variable 'rval' used. fetch.c 1727

 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); } .... } 

The rval variable was not initialized when declared, so comparing it with a null value will lead to an undefined result. If circumstances fail, the undefined value of the rval variable may become the return value of the auto_fetch function.

V614 Uninitialized pointer 'res' used. commands.c 2873

 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; .... } } .... } 

A similar case of using an uninitialized variable, only here is the uninitialized pointer res .

V506 Pointer to local variable 'normalized' is stored outside the scope of this variable. Such a pointer will become invalid. TextView.cpp 5596

 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); } 

Probably, the programmer needed to normalize the object through an intermediate variable. But now, in the font pointer, the pointer to the normalized temporary object was saved , which will be destroyed after leaving the scope in which this 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 27

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

A very common mistake among C ++ programmers is to use the constructor call supposedly to initialize / zero the class fields. In this case, there is no modification of the fields of the class, but a new unnamed object of this class is created, which is immediately destroyed. Unfortunately, there are a lot of such places:


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 184

 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; }; 

Another example of incorrect initialization. Class fields are initialized in the order they are declared in the class itself. In this example, the fInternal field will be initialized first using the uninitialized value fPatternHandler .

Suspicious #define


V523 The 'then' statement is equivalent to the 'else' statement. subr_gtaskqueue.c 191

 #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); } 

The code snippet does not look suspicious until you look at the result of the preprocessor:

 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); } 

The analyzer is really right - the if and else branches are identical. But where did the mtx_lock_spin and mtx_unlock_spin functions go ? The macros TQ_LOCK , TQ_UNLOCK and the grouptask_block function are declared in the same file and are almost nearby, however, somewhere there was a substitution.

A search on the contents of the files found only mutex.h with the following contents:

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

Whether such a substitution is correct or not is worth checking for the project developers. I checked the project on Linux, and such a substitution seemed suspicious to me.

Errors with the free function


V575 The null pointer is passed into 'free' function. Inspect the first argument. setmime.cpp 727

 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(); } 

You can pass a null pointer to the free function, but such an entry is clearly suspicious. So, the analyzer found the confused lines of code. First, you had to free the memory using the pointer fVectorIcon , and only then set it to NULL .

V575 The null pointer is passed into 'free' function. Inspect the first argument. driver_settings.cpp 461

 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); // <= .... } 

Another example of explicitly passing a null pointer to the free function. This line can be deleted since upon successful receipt of the pointer, the function exits.

V575 The null pointer is passed into 'free' function. Inspect the first argument. PackageFileHeapWriter.cpp 166

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

Here is a mistake. Instead of the && operator, the || operator must be used. Only in this case will an exception be thrown std :: bad_alloc () if it was not possible to allocate memory using the malloc function.

Errors with delete operator


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 65

 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); } } 

The fMsg pointer is used to allocate memory for storing an array of characters, and the delete operator is used to free memory, instead of delete [] .

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 3080

 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; } .... } 

Here malloc_flags is the function that makes malloc . And then placement-new constructs the object here. And since the PageWriteWrapper class is implemented as follows:

 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"); } 

then destructors of objects of this class will not be called due to the use of the free function to free 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 26

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

Using the delete operator instead of delete [] is a very common mistake. The easiest way to make a mistake when writing a class is because Destructor code is often located far from memory allocation locations. Here, the programmer incorrectly frees memory in the destructor by the pointer fOutBuffer .

V772 Calling a 'delete' operator for a void pointer will cause undefined behavior. Hashtable.cpp 207

 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; } .... } 

In addition to the wrong choice between delete / delete [] and free , you can add undefined behavior to the program in another way - try to clear the memory by a pointer to an undefined type (void *) .

Functions without return value


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

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

The overridden assignment operator does not have enough return value. In this case, the operator will return a random value, which can lead to strange errors.

Similar problems in other code snippets of this class:


V591 Non-void function should return a value. main.c 1010

 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 */ } 

A NOTREACHED user comment here means nothing. To correctly write code for such scenarios, you need to mark up functions like noreturn. There are noreturn attributes for this: standard and compiler specific. First of all, such attributes are taken into account by compilers for proper code generation or notification of some types of errors with the help of warings. Various static analysis tools also take attributes into account to improve the quality of analysis.

Exception handling


V596 The object was created but it is not being used. The 'throw' keyword could be missing: throw ParseException (FOO); Response.cpp 659

 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; } 

Here, the throw keyword is accidentally forgotten. Thus, a ParseException will not be thrown , and an object of this class will simply be destroyed when it leaves the scope. After which the function will continue its work as if nothing had happened, as if the correct number was entered.

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

 int main(int argc, char** argv) { try { return Main().Run(argc, argv); } catch (Exception& exception) { // <= fprintf(stderr, "%s\n", 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 + "'."); // <= .... } 

The analyzer detected an IOException thrown by the pointer. Throwing a pointer causes the exception to not be caught, so the exception is eventually caught by reference. Also, using a pointer forces the interceptor to call the delete operator to destroy the created object, which is also not done.

Two more problem areas of code:


Formal safety


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 1018

 #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\n", 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); } 

The analyzer detected suspicious code designed to safely clean private data. Unfortunately, the SAFE_FREE macro, which is expanded into memset , free calls and NULL assignment, does not make the code safer, because all this is removed by the compiler during O2 optimization.

By the way, this is nothing like CWE-14 : Compiler Removal of Code to Clear Buffers.

The whole list of places where buffers are not actually cleared:


Unsigned Comparisons


V547 Expression 'remaining <0' is always false. Unsigned type value is never <0. DwarfFile.cpp 1947

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

The analyzer found an explicit comparison of an unsigned variable with negative values. Perhaps you should compare the remaining variable with only zero or implement an overflow check.

V547 Expression 'sleep ((unsigned) secs) <0' is always false. Unsigned type value is never <0. misc.cpp 56

 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; } 

To understand what the error is, let us turn to the signatures of the sleep and usleep functions:

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

As we see, the sleep function returns a value of an unsigned type and its use in the code is incorrect.

Dangerous pointers


V774 The 'device' pointer was used after the memory was released. xhci.cpp 1572

 void XHCI::FreeDevice(Device *device) { uint8 slot = fPortSlots[device->HubPort()]; TRACE("FreeDevice() port %d slot %d\n", 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; } 

A device object is cleared by the delete operator. This is a logical action for a function called FreeDevice . But, for some reason, to release other resources in the code, there is again an appeal to an already remote object.

Such a code is extremely dangerous and is found in several other 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 350

 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 .... } 

Interprocedural analysis revealed a situation where NULL is passed to the function and the data pointer with this value is subsequently dereferenced in the memcpy function.

V773 The function was exited without releasing the 'inputFileFile' pointer. A memory leak is possible. command_recompress.cpp 119

 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\n", inputPackageFileName, strerror(error)); return 1; } inputFile = inputFileFile; .... } 

PVS-Studio can detect memory leaks . The memory for inputFileFile is not freed up here in case of some kind of error. Someone believes that in case of errors, you can not bother with freeing up memory - the program will still end. But it's not always the case. Correctly handle errors and continue to work - a requirement for so many programs.

V595 The 'fReply' pointer was utilized before it was verified against nullptr. Check lines: 49, 52. ReplyBuilder.cpp 49

 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; } 

How often do developers dereference pointers before checking them. Diagnostics V595 is almost always a leader in the number of warnings in a project. The dangerous use of the fReply pointer in this code snippet .

V595 The 'mq' pointer was utilized before it was verified against nullptr. Check lines: 782, 786. oce_queue.c 782

 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; .... } 

A similar example. The mg pointer is dereferenced a few lines earlier than being checked for a null value. There are many similar places in the project. In some places, using and checking pointers is far removed from each other, so two examples are included in the article. The rest will be able to see the developers in the full analyzer report.

Miscellaneous errors


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 101

 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; .... } .... } .... } 

The difference between the strlcat and strncat functions is not entirely obvious to a person new to the description of these functions. The strlcat function takes the size of the entire buffer as the third argument, and the strncat function takes the size of the free space in the buffer , which requires calculating the desired value before calling the function. But developers often forget or do not know about it. Passing the strncat function to the size of the entire buffer can lead to a buffer overflow, because the function will regard this value as the number of characters allowed for copying. The strlcat function does not have this problem, but for it to work correctly, it is necessary to pass lines that end in terminal zero.

The whole list of dangerous places with lines:


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 324

 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; } 

Most likely, the operators '|' and '||'. Such an error leads to unnecessary calls to the SetDecoratorSettings function .

V627 Consider inspecting the expression. The argument of sizeof () is the macro which expands to a number. device.c 72

 #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; .... } 

Passing the value 0x0c to the sizeof operator looks suspicious. Perhaps you should calculate the size of some object, for example, data . V562 It's odd to compare a bool type value with a value of 18: 0x12 == IsProfessionalSpdif (). CEchoGals_mixer.cpp 533



 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(); } .... } 

The IsProfessionalSpdif function returns a value of type bool , while in the condition the result of the function is compared with the number 0x12 .

Conclusion


We missed the release of the first Haiku beta last fall, as were busy releasing PVS-Studio for the Java language. But the nature of programming errors is such that they don’t go anywhere unless you look for them and do not pay attention to the quality of the code at all. The developers tried to use Coverity Scan , but the last run of the analysis was almost two years ago. This should upset Haiku users. Although the analysis using Coverity was configured in 2014, this did not stop us from writing two large articles with error reviews in 2015 ( part 1 , part 2 ).

Those who have read to the end are waiting for another review of Haiku code of no less volume with new types of errors. A full analyzer report will be sent to the developers before publishing code reviews, so some errors can be fixed. To pass the time between publications, I suggest downloading and trying PVS-Studio on your project.

Want to try Haiku and have any questions? Haiku developers invite you to the telegram channel .



If you want to share this article with an English-speaking audience, then please use the link to the translation: Svyatoslav Razmyslov. How to shoot yourself in the foot in C and C ++. Haiku OS Cookbook

Source: https://habr.com/ru/post/461255/


All Articles