Author: Aleksandr Chibisov

ReactOS is intensively developing, and its codebase is steadily growing in size. On February 16, 2016, a new version of the operating system was released, and we thought it was a good occasion to run it through our static analyzer one more time. The new scan was done with PVS-Studio, version 6.02.

Introduction

I decided to try my hand at writing articles on static analysis, and for this one I picked ReactOS project.

ReactOS is a modern open-source operating system based on the principles underlying Windows NT and Wine. The project’s primary goal is to create a system fully compatible with application and device drivers made for Windows.

We already scanned this project before. For the previous results, see the following articles:

Since the last scan, the number of projects inside the solution has grown to 963.

The new analysis revealed 4027 general warnings in 5396 project files. The developers obviously paid attention to the results of the previous scan, as the number of bugs has dropped. However, the project keeps developing, and new errors are inevitable. That’s why code analysis must be carried out regularly.

Obviously, examining all the warnings in detail is impossible, so in this article we will discuss only the most interesting and suspicious fragments found. I’m sure I have missed some errors, not to mention the warnings on64-bit errors and micro-optimizations, which I haven’t checked at all.

The PVS-Studio demo version is not sufficient for scanning a large project like React OS, but OOO “Program Verification Systems” granted me a license so that I could analyze open-source projects.

Typos

PVS-Studio analyzer is good at detecting typos of all kinds, which can be easily lost in large codebases. Let’s see what we’ve got this time.

Value reassignment

Int xmlNanoFTPList(....)

{

....

closesocket(ctxt->dataFd); ctxt->dataFd = INVALID_SOCKET; //<-

ctxt->dataFd = INVALID_SOCKET; //<-

....

}

V519 The ‘ctxt->dataFd’ variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1790, 1791. nanoftp.c 1791

There is an apparent error here that deals with assigning one and the same value to a variable in two adjacent lines. Such errors occur when programmers ignore the code formatting rules. Ordinary code review does not easily reveal errors of this type, as statements occupy one line and look like a single fragment.

There’s nothing bad about this error, of course; it’s just a duplicate assignment. Nevertheless, it’s still an error, so I thought I should mention it.

The same error dealing with duplicate code can be found in the project a few more times:

V519 The ‘ctxt->dataFd’ variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1804, 1805. nanoftp.c 1805

V519 The ‘ctxt->dataFd’ variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1935, 1936. nanoftp.c 1936

Incorrect operator

static INT CommandDumpSector(....)

{

....

SectorCount.QuadPart = DiskGeometry.Cylinders.QuadPart *

DiskGeometry.TracksPerCylinder, //<-

DiskGeometry.SectorsPerTrack;

....

}

V521 Such expressions using the ‘,’ operator are dangerous. Make sure the expression is correct. cmdcons.c 430

When computing the value of a variable, the comma operator is used instead of the multiplication operator in a fragment where several values should be multiplied. As a result, the variable is assigned the valueDiskGeometry.SectorsPerTrack while the result of multiplying the first two operands is simply ignored. Errors like this usually occur when the same variable is used many times in several adjacent lines and the programmer copies it together with the comma from a previous check instead of typing it all over again.

Extra assignment

LRESULT CDeviceManager::OnNotify(_In_ LPARAM lParam)

{

....

UINT_PTR idButton = lpttt->hdr.idFrom;

switch (idButton)

{

....

}

idButton = idButton; //<-

....

}

V570 The ‘idButton’ variable is assigned to itself. mainwindow.cpp 546

A variable is assigned to itself. This operation occurs in one of the last lines of the function body, with all the required operations over idButton already done, so it is either that the value was meant to be assigned to some other global variable so that another function could continue the computations, or that the programmer was not attentive enough and wrote an extra operation by mistake.

Errors in conditions

Various errors found in conditions result in incorrect data processing and infinite loops. Sometimes they result in regularly processing certain code fragments that were not intended to be executed at all or were intended to be executed only under certain conditions.

Identical operands in a condition

#define GCS_HELPTEXTA 0x00000001

#define GCS_VALIDATEA 0x00000002

#define GCS_HELPTEXTW 0x00000005

#define GCS_VALIDATEW 0x00000006

HRESULT WINAPI CDefaultContextMenu::GetCommandString(....)

{

if (uFlags == GCS_HELPTEXTA || uFlags == GCS_HELPTEXTW)

{

....

}

....

if (uFlags == GCS_VALIDATEA || uFlags == GCS_VALIDATEA) //<-

return S_OK;

....

}

V501 There are identical sub-expressions ‘uFlags == 0x00000002’ to the left and to the right of the ‘||’ operator. cdefaultcontextmenu.cpp 1793

The function works both with ordinary strings and Unicode strings, so every check is done twice. A typo in one of the conditions causes an ordinary string to be checked twice, ignoring the corresponding Unicode string. To fix the error, the second part of the condition needs to be changed.

The fixed code should look like this:

if (uFlags == GCS_VALIDATEA || uFlags == GCS_VALIDATEW)

return S_OK;

Idle condition

SOCKET WSPAPI

WSPAccept(....)

{

....

if (CallBack == CF_REJECT)

{

*lpErrno = WSAECONNREFUSED;

return INVALID_SOCKET;

}

else

{

*lpErrno = WSAECONNREFUSED;

return INVALID_SOCKET;

}

....

}

V523 The ‘then’ statement is equivalent to the ‘else’ statement. dllmain.c 1325

This is another kind of errors found in conditions. In the example above, the same block of statements will be executed regardless of whether the condition is true or false. I don’t know the specifics of this code, so I can only make some assumptions about how to solve the issue. Perhaps the code blocks should be revised, or maybe it’s just that the condition should be deleted so that the code is always executed without unnecessary checks.

Similar code fragments were found in other modules as well:

V523 The ‘then’ statement is equivalent to the ‘else’ statement. crecyclebin.cpp 1357

V523 The ‘then’ statement is equivalent to the ‘else’ statement. mapdesc.cc 733

V523 The ‘then’ statement is equivalent to the ‘else’ statement. pattern.c 2058

V523 The ‘then’ statement is equivalent to the ‘else’ statement. console.c 143

Logical error

HDEVINFO WINAPI SetupDiGetClassDevsExW(....)

{

....

if (flags & DIGCF_ALLCLASSES)

{

....

}

else if (!IsEqualIID(&list->ClassGuid, &GUID_NULL))//<-

{

if (IsEqualIID(&list->ClassGuid, &GUID_NULL)) //<-

pClassGuid = &list->ClassGuid;

....

}

....

}

V637 Two opposite conditions were encountered. The second condition is always false. Check lines: 2532, 2535. devinst.c 2532

Diagnostic V637 triggered by this code indicates the presence of an unreasonable condition. If the first condition is true, the second will always be false, and the corresponding code block will never get control anyway. It is an example of redundant code.

Another diagnostic revealed a similar defect:

static int xmlStreamCompile(xmlPatternPtr comp) {

....

case XML_OP_ELEM:

....

if ((comp->nbStep == i + 1) && //<-

(flags & XML_STREAM_STEP_DESC))

{

if (comp->nbStep == i + 1) { //<-

stream->flags |= XML_STREAM_FINAL_IS_ANY_NODE;

}

}

....

}

V571 Recurring check. The ‘comp->nbStep == i + 1’ condition was already verified in line 1649. pattern.c 1655

The second check will always evaluate to true and is not necessary, as this code will get control independently of it.

Redundant expression

PVOID NTAPI RtlLookupElementGenericTableFull(....)

{

....

/* Check if we found anything */

if ((*SearchResult == TableEmptyTree) ||

(*SearchResult != TableFoundNode))

{

return NULL;

}

....

}

V590 Consider inspecting this expression. The expression is excessive or contains a misprint. generictable.c 278

The fragment above contains a check with one extra expression. If the first expression is true, the second won’t be checked. But if the second expression does get checked, it will also evaluate to true. Therefore, checking only the second expression is enough, so the code should be simplified as follows:

if (*SearchResult != TableFoundNode)

{

return NULL;

}

Unused function result

#define IOleInPlaceSite_GetWindow(This,phwnd)\

( (This)->lpVtbl -> GetWindow(This,phwnd) )



static void create_shell_embedding_hwnd(WebBrowser *This)

{

HWND parent = NULL;

....

if(SUCCEEDED(hres)) {

IOleInPlaceSite_GetWindow(inplace, &parent); //<-

IOleInPlaceSite_Release(inplace);

}

....

}

V530 The return value of function ‘GetWindow’ is required to be utilized. oleobject.c 113

The function in the code above looks for a certain window in relation to the inplace window, but regardless of whether or not the window is found, invoking this function makes no sense because its result is not used anywhere.

The same error can be found in one more place:

V530 The return value of function ‘GetWindow’ is required to be utilized. oleobject.c 185

Suspicious loop

FF_T_SINT32 FF_BlockRead(....) {

FF_T_SINT32 slRetVal = 0;

....

if(pIoman->pBlkDevice->fnpReadBlocks)

{

....

slRetVal = pIoman->pBlkDevice->fnpReadBlocks(pBuffer,

ulSectorLBA, ulNumSectors, pIoman->pBlkDevice->pParam);

....

if(FF_GETERROR(slRetVal) == FF_ERR_DRIVER_BUSY) {

FF_Sleep(FF_DRIVER_BUSY_SLEEP);

}

} while(FF_GETERROR(slRetVal) == FF_ERR_DRIVER_BUSY); //<- return slRetVal;

}

This time, a single anomaly triggered three different warnings at once:

V712 Be advised that compiler may delete this cycle or make it infinity. Use volatile variable(s) or synchronization primitives to avoid this. ff_ioman.c 539

V715 The ‘while’ operator has empty body. Suspicious pattern detected: ‘if (expr) {…} while (expr);’. ff_ioman.c 539

V547 Expression ‘(slRetVal & 0x0000FFFF) == — 10’ is always false. ff_ioman.c 539

A condition is followed by a bodyless loop whose conditional expression will always evaluate to false, so the loop won’t iterate even once. I don’t know what exactly this code was meant to do, but my guess is that it is a do …. while sequence with the do statement accidentally omitted.

The analyzer found one more instance of the same code:

V712 Be advised that compiler may delete this cycle or make it infinity. Use volatile variable(s) or synchronization primitives to avoid this. ff_ioman.c 564

V715 The ‘while’ operator has empty body. Suspicious pattern detected: ‘if (expr) {…} while (expr);’. ff_ioman.c 564

V547 Expression ‘(slRetVal & 0x0000FFFF) == — 10’ is always false. ff_ioman.c 564

Incorrect function argument

void UpdateStatusBar(void)

{

TCHAR szStatusText[128];

....

ZeroMemory(szStatusText,

sizeof(szStatusText) / sizeof(TCHAR)); //<-

....

}

V575 Buffer’s size in bytes should be passed to the ‘memset’ function as the third argument instead of the number of processed elements. solitaire.cpp 153

The ZeroMemory function fills the block of memory allocated for an array with zeroes. In our example, the function receives, as an argument, the actual number of array elements instead of the size of the block to be cleared, which mistake results in zeroing only part of the target memory block. To fix this error, the code should be modified as follows:

ZeroMemory(szStatusText,

sizeof(szStatusText));

Potential null-pointer dereferencing

The analyzer reported quite a lot of code fragments that deal with a situation when a pointer is first used and only then is tested for null. Such code works well until a null pointer is encountered.

NET_API_STATUS WINAPI NetUserEnum(....)

{

done:

if (ApiStatus == NERR_Success &&

EnumContext->Index < EnumContext->Count) //<-

ApiStatus = ERROR_MORE_DATA; if (EnumContext != NULL) //<-

*totalentries = EnumContext->Count;

}

V595 The ‘EnumContext’ pointer was utilized before it was verified against nullptr. Check lines: 2557, 2560. user.c 2557

The authors have fixed a lot of errors of this type since the last scan. However, a lot of new ones also appeared, so it’s just another good argument for using static analysis regularly.

Here is a complete list of diagnostic messages for this bug pattern in ReactOS:

V595 The ‘EnumContext’ pointer was utilized before it was verified against nullptr. Check lines: 1166, 1169. local_group.c 1166

V595 The ‘GuiData’ pointer was utilized before it was verified against nullptr. Check lines: 1458, 1465. conwnd.c 1458

V595 The ‘PartitionList->CurrentPartition’ pointer was utilized before it was verified against nullptr. Check lines: 1653, 1656. usetup.c 1653

V595 The ‘PolicyAccountDomainInfo’ pointer was utilized before it was verified against nullptr. Check lines: 255, 258. sidcache.c 255

V595 The ‘PropertySheetHeader’ pointer was utilized before it was verified against nullptr. Check lines: 1176, 1180. devclass.c 1176

V595 The ‘This->pITextStoreACP’ pointer was utilized before it was verified against nullptr. Check lines: 127, 131. context.c 127

V595 The ‘addr’ pointer was utilized before it was verified against nullptr. Check lines: 2110, 2113. builtin.c 2110

V595 The ‘argv’ pointer was utilized before it was verified against nullptr. Check lines: 354, 358. rundll32.c 354

V595 The ‘context’ pointer was utilized before it was verified against nullptr. Check lines: 93, 98. texture.c 93

V595 The ‘driverInfo’ pointer was utilized before it was verified against nullptr. Check lines: 211, 227. driver.c 211

V595 The ‘ds’ pointer was utilized before it was verified against nullptr. Check lines: 257, 262. main.cpp 257

V595 The ‘ds’ pointer was utilized before it was verified against nullptr. Check lines: 303, 308. main.cpp 303

V595 The ‘entry’ pointer was utilized before it was verified against nullptr. Check lines: 431, 440. cmenufocusmanager.cpp 431

V595 The ‘entry’ pointer was utilized before it was verified against nullptr. Check lines: 572, 575. cursoricon.c 572

V595 The ‘lpszTemp’ pointer was utilized before it was verified against nullptr. Check lines: 1167, 1170. taskmgr.c 1167

V595 The ‘m_shellPidl’ pointer was utilized before it was verified against nullptr. Check lines: 748, 749. ntobjfolder.cpp 748

V595 The ‘m_shellPidl’ pointer was utilized before it was verified against nullptr. Check lines: 803, 804. regfolder.cpp 803

V595 The ‘new_env’ pointer was utilized before it was verified against nullptr. Check lines: 432, 450. env.c 432

V595 The ‘pFM2’ pointer was utilized before it was verified against nullptr. Check lines: 617, 627. regsvr.c 617

V595 The ‘pPatterns’ pointer was utilized before it was verified against nullptr. Check lines: 497, 518. info.c 497

V595 The ‘pServiceStatus’ pointer was utilized before it was verified against nullptr. Check lines: 194, 200. query.c 194

V595 The ‘pTLib’ pointer was utilized before it was verified against nullptr. Check lines: 7863, 7865. typelib.c 7863

V595 The ‘patterns’ pointer was utilized before it was verified against nullptr. Check lines: 1747, 1768. info.c 1747

V595 The ‘pcFetched’ pointer was utilized before it was verified against nullptr. Check lines: 211, 216. mediatype.c 211

V595 The ‘pcchOut’ pointer was utilized before it was verified against nullptr. Check lines: 2248, 2250. url.c 2248

V595 The ‘pidlChild’ pointer was utilized before it was verified against nullptr. Check lines: 162, 183. shlfolder.cpp 162

V595 The ‘plpOptions[curStream]’ pointer was utilized before it was verified against nullptr. Check lines: 1603, 1604. api.c 1603

V595 The ‘ppszNames’ pointer was utilized before it was verified against nullptr. Check lines: 238, 245. find.c 238

V595 The ‘ppszNames’ pointer was utilized before it was verified against nullptr. Check lines: 464, 485. find.c 464

V595 The ‘prbel’ pointer was utilized before it was verified against nullptr. Check lines: 226, 244. recyclebin.c 226

V595 The ‘psp’ pointer was utilized before it was verified against nullptr. Check lines: 2718, 2727. propsheet.c 2718

V595 The ‘pszList’ pointer was utilized before it was verified against nullptr. Check lines: 613, 630. dialogs.cpp 613

V595 The ‘subtypeinfo’ pointer was utilized before it was verified against nullptr. Check lines: 5559, 5564. typelib.c 5559

V595 The ‘typeInfo’ pointer was utilized before it was verified against nullptr. Check lines: 943, 945. typelib.c 943

V595 The ‘user_dirids’ pointer was utilized before it was verified against nullptr. Check lines: 192, 203. dirid.c 192

V595 The ‘wki’ pointer was utilized before it was verified against nullptr. Check lines: 129, 133. netid.c 129

V595 The ‘wki’ pointer was utilized before it was verified against nullptr. Check lines: 163, 167. netid.c 163

V595 The ‘wki’ pointer was utilized before it was verified against nullptr. Check lines: 299, 302. netid.c 299

V595 The ‘Globals.win_list’ pointer was utilized before it was verified against nullptr. Check lines: 542, 564. winhelp.c 542

V595 The ‘NewSubsystem’ pointer was utilized before it was verified against nullptr. Check lines: 501, 503. smsubsys.c 501

V595 The ‘This->doc_host’ pointer was utilized before it was verified against nullptr. Check lines: 834, 847. shellbrowser.c 834

V595 The ‘atom->ranges’ pointer was utilized before it was verified against nullptr. Check lines: 817, 818. xmlregexp.c 817

V595 The ‘attr’ pointer was utilized before it was verified against nullptr. Check lines: 2860, 2864. xmlschemas.c 2860

V595 The ‘atts’ pointer was utilized before it was verified against nullptr. Check lines: 8640, 8649. parser.c 8640

V595 The ‘compiland’ pointer was utilized before it was verified against nullptr. Check lines: 294, 301. symbol.c 294

V595 The ‘compiland’ pointer was utilized before it was verified against nullptr. Check lines: 494, 499. symbol.c 494

V595 The ‘compress_formats[i].guid’ pointer was utilized before it was verified against nullptr. Check lines: 946, 950. jpegformat.c 946

V595 The ‘converter’ pointer was utilized before it was verified against nullptr. Check lines: 2319, 2324. info.c 2319

V595 The ‘ctx->myDoc’ pointer was utilized before it was verified against nullptr. Check lines: 13108, 13111. parser.c 13108

V595 The ‘ctxt’ pointer was utilized before it was verified against nullptr. Check lines: 1130, 1131. xinclude.c 1130

V595 The ‘ctxt->incTab[nr]’ pointer was utilized before it was verified against nullptr. Check lines: 1420, 1429. xinclude.c 1420

V595 The ‘ctxt->input’ pointer was utilized before it was verified against nullptr. Check lines: 11364, 11367. parser.c 11364

V595 The ‘ctxt->input’ pointer was utilized before it was verified against nullptr. Check lines: 6734, 6738. htmlparser.c 6734

V595 The ‘ctxt->input’ pointer was utilized before it was verified against nullptr. Check lines: 6802, 6810. parser.c 6802

V595 The ‘ctxt->input’ pointer was utilized before it was verified against nullptr. Check lines: 6864, 6872. parser.c 6864

V595 The ‘ctxt->myDoc’ pointer was utilized before it was verified against nullptr. Check lines: 13667, 13683. parser.c 13667

V595 The ‘ctxt->myDoc’ pointer was utilized before it was verified against nullptr. Check lines: 819, 831. sax2.c 819

V595 The ‘ctxt->nsTab’ pointer was utilized before it was verified against nullptr. Check lines: 1590, 1597. parser.c 1590

V595 The ‘current_line’ pointer was utilized before it was verified against nullptr. Check lines: 558, 563. edit.c 558

V595 The ‘depth_stencil’ pointer was utilized before it was verified against nullptr. Check lines: 348, 351. device.c 348

V595 The ‘device->hwbuf’ pointer was utilized before it was verified against nullptr. Check lines: 909, 919. mixer.c 909

V595 The ‘es’ pointer was utilized before it was verified against nullptr. Check lines: 5284, 5300. edit.c 5284

V595 The ‘formats[i].guid’ pointer was utilized before it was verified against nullptr. Check lines: 1416, 1420. pngformat.c 1416

V595 The ‘formats[i].guid’ pointer was utilized before it was verified against nullptr. Check lines: 1562, 1566. tiffformat.c 1562

V595 The ‘formats[i].guid’ pointer was utilized before it was verified against nullptr. Check lines: 166, 170. bmpencode.c 166

V595 The ‘func’ pointer was utilized before it was verified against nullptr. Check lines: 373, 376. symbol.c 373

V595 The ‘lpFindInfo’ pointer was utilized before it was verified against nullptr. Check lines: 6310, 6314. listview.c 6310

V595 The ‘lpItem’ pointer was utilized before it was verified against nullptr. Check lines: 4223, 4248. listview.c 4223

V595 The ‘lpNumberOfBytesRead’ pointer was utilized before it was verified against nullptr. Check lines: 150, 180. rw.c 150

V595 The ‘lpiID’ pointer was utilized before it was verified against nullptr. Check lines: 343, 364. exticon.c 343

V595 The ‘lpszCommandLine’ pointer was utilized before it was verified against nullptr. Check lines: 283, 288. crtexe.c 283

V595 The ‘lpwh’ pointer was utilized before it was verified against nullptr. Check lines: 1374, 1388. ftp.c 1374

V595 The ‘lpwszComponent’ pointer was utilized before it was verified against nullptr. Check lines: 1465, 1468. internet.c 1465

V595 The ‘lpwszConnectionName’ pointer was utilized before it was verified against nullptr. Check lines: 1256, 1258. internet.c 1256

V595 The ‘name’ pointer was utilized before it was verified against nullptr. Check lines: 125, 133. hash.c 125

V595 The ‘oldctxt’ pointer was utilized before it was verified against nullptr. Check lines: 13444, 13448. parser.c 13444

V595 The ‘oldctxt’ pointer was utilized before it was verified against nullptr. Check lines: 13675, 13693. parser.c 13675

V595 The ‘optarg’ pointer was utilized before it was verified against nullptr. Check lines: 645, 646. tnconfig.cpp 645

V595 The ‘pData’ pointer was utilized before it was verified against nullptr. Check lines: 239, 255. clipboard.c 239

V595 The ‘pMapper’ pointer was utilized before it was verified against nullptr. Check lines: 938, 965. createdevenum.c 938

V595 The ‘pThis’ pointer was utilized before it was verified against nullptr. Check lines: 1368, 1375. atlwin.h 1368

V595 The ‘pThis’ pointer was utilized before it was verified against nullptr. Check lines: 1550, 1557. atlwin.h 1550

V595 The ‘param’ pointer was utilized before it was verified against nullptr. Check lines: 535, 537. effect.c 535

Uninitialized variables

static HRESULT parse_canonicalize(....)

{

const WCHAR **pptr;

....

if(uri->scheme_start > -1 && uri->path_start > -1) {

ptr = uri->canon_uri+uri->scheme_start+uri->scheme_len+1;

pptr = &ptr;

}

reduce_path = !(flags & URL_DONT_SIMPLIFY) &&

ptr && check_hierarchical(pptr);

}

V614 Potentially uninitialized pointer ‘pptr’ used. Consider checking the first actual argument of the ‘check_hierarchical’ function. uri.c 6838

If the condition is found to be false, the pptr variable will remain uninitialized by the time the check_hierarchical function is invoked.

Similar errors were found in several other fragments:

V614 Potentially uninitialized pointer ‘name’ used. Consider checking the third actual argument of the ‘disp_get_id’ function. engine.c 928

V614 Potentially uninitialized pointer ‘name_str’ used. Consider checking the first actual argument of the ‘jsstr_release’ function. engine.c 929

V614 Potentially uninitialized pointer ‘FileHandle’ used. Consider checking the first actual argument of the ‘CloseHandle’ function. dosfiles.c 402

V614 Potentially uninitialized pointer ‘FileHandle’ used. Consider checking the first actual argument of the ‘CloseHandle’ function. dosfiles.c 482

V614 Potentially uninitialized pointer ‘pwszClsid’ used. Consider checking the third actual argument of the ‘RegGetValueW’ function. cfsfolder.cpp 1666

V614 Potentially uninitialized pointer ‘FilePart’ used. smutil.c 405

Bitwise comparison

typedef enum CorTokenType

{

mdtModule = 0x00000000,

mdtTypeRef = 0x01000000,

....

mdtModuleRef = 0x1a000000,

mdtTypeSpec = 0x1b000000,

mdtAssembly = 0x20000000,

mdtAssemblyRef = 0x23000000,

....

} CorTokenType; #define TypeFromToken(tk) ((ULONG32)((tk) & 0xff000000))

#define TableFromToken(tk) (TypeFromToken(tk) >> 24) static inline ULONG get_table_size(....)

{

INT tables;

....

tables = max(

assembly->tables[TableFromToken(mdtModule)].rows, //<-

assembly->tables[TableFromToken(mdtModuleRef)].rows);

....

}

V616 The ‘(mdtModule)’ named constant with the value of 0 is used in the bitwise operation. assembly.c 156

In the example above, bitwise comparison is done over a constant that is initially a zero. Therefore, this operation makes no sense as it produces a null value all the time.

Conclusion

ReactOS is a constantly developing project, and like any other such project, it tends to accumulate a large number of errors. Regular static analysis by PVS-Studio will help reveal not only already existing bugs but also suspicious fragments that may become bugs later.