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.
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:
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;
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;
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:
- 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
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;
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:
#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;
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) {
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;
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);
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:
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. Referenceable.h 233
- V591 Non-void function should return a value. Referenceable.h 239
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); }
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) {
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:
- 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 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:
- 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
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)
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);
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:
- 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
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) { .... error = malo_hal_send_helper(mh, 0, NULL, 0, MALO_NOWAIT);
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:
- 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
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 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() )
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