Even small projects have bugs, or how PVS-Studio checked Blend2D

Introduction

Blend2D

---------------------------------------------------------------------
Language files blank comment code
---------------------------------------------------------------------
C++ 97 12924 9481 43372
C/C++ Header 137 8305 12971 25225

An always-false expression

BLResult blJpegDecoderImplProcessMarker(....) noexcept {
uint32_t h = blMemReadU16uBE(p + 1);
// ....
if (h == 0)
return blTraceError(BL_ERROR_JPEG_UNSUPPORTED_FEATURE);
// ....
impl->delayedHeight = (h == 0); // <=
// ....
}

A typo in the function’s signature

static BL_INLINE bool blIsCubicFlat(const BLPoint p[3], double f) {
if (p[3] == p[0]) {
// ....
}
// ....
}

An extra evaluation due to an incorrect operator

BL_NODISCARD BL_INLINE bool isObject() const noexcept
{
return (data.type > BL_STYLE_TYPE_SOLID) & _isTagged();
}
BL_NODISCARD BL_INLINE bool _isTagged(uint32_t styleType) const noexcept {

A redundant check

class BLRasterWorkerManager {
public:
BLThreadPool* _threadPool;
uint32_t _workerCount;
// ....
}
// ....
void BLRasterWorkerManager::reset() noexcept {
// ....
if (_workerCount) {
// ....
_threadPool->releaseThreads(_workerThreads, _workerCount);
_workerCount = 0;
// ....
}
if (_threadPool) {
_threadPool->release();
_threadPool = nullptr;
}
// ....
}
BL_INLINE BLRasterWorkerManager() noexcept
: // ....
_threadPool(nullptr),
// ....
_workerCount(0),
// ....
{}
BLResult BLRasterWorkerManager::init(....) noexcept {
// ....
uint32_t workerCount = threadCount - 1;
// ....
if (workerCount) {
// ....
BLThreadPool* threadPool = nullptr;
if (initFlags & BL_CONTEXT_CREATE_FLAG_ISOLATED_THREAD_POOL) {
threadPool = blThreadPoolCreate();
if (!threadPool)
return blTraceError(BL_ERROR_OUT_OF_MEMORY);
}
else {
threadPool = blThreadPoolGlobal();
}
// ....
uint32_t n = threadPool->acquireThreads(workerThreads,
workerCount, acquireThreadFlags, &reason);
// ....
if (!n) {
threadPool->release();
threadPool = nullptr;
// ....
}
// ....
_threadPool = threadPool;
// ....
_workerCount = n;
}
else {
// ....
}
}
static BLWrap<BLInternalThreadPool> blGlobalThreadPool;
BLThreadPool* blThreadPoolGlobal() noexcept { return &blGlobalThreadPool; }
uint32_t n = threadPool->acquireThreads(workerThreads, workerCount, 
acquireThreadFlags, &reason);
void BLRasterWorkerManager::reset() noexcept {
// ....
if (_workerCount) {
assert(_threadPool);
for (uint32_t i = 0; i < _workerCount; i++)
_workDataStorage[i]->~BLRasterWorkData();
_threadPool->releaseThreads(_workerThreads, _workerCount);
_workerCount = 0;
_workerThreads = nullptr;
_workDataStorage = nullptr;
_threadPool->release();
_threadPool = nullptr;
}
// ....
}

Using an uninitialized variable

static BLResult BL_CDECL bl_convert_multi_step(...., uint32_t w, ....)
{
for (uint32_t y = h; y; y--) {
uint32_t i = w;
workOpt.origin.x = baseOriginX;
dstData = dstLine;
srcData = srcLine;
while (i) {
uint32_t n = blMin(n, intermediatePixelCount);
srcToIntermediate(&ctx->first, intermediateData, 0,
srcData, srcStride, n, 1, nullptr);
intermediateToDst(&ctx->second, dstData, dstStride,
intermediateData, 0, n, 1, &workOpt);
dstData += n * dstBytesPerPixel;
srcData += n * srcBytesPerPixel;
workOpt.origin.x += int(n);
i -= n;
}
}

An always-true check

static void blPngDeinterlaceBits(....) noexcept {
// ....
uint32_t x = w;
// ....
switch (n) {
case 2: {
// ....
if (x <= 4) break;
if (x >= 5) b = uint32_t(*d5++);
// ....
}
// ....
}
// ....
}
static void blPngDeinterlaceBits(....) noexcept {
....
uint32_t x = w;
....
switch (n) {
case 2: {
// ....
if (x <= 4) break;
b = uint32_t(*d5++);
// ....
}
// ....
}
// ....
}

A copy-paste error

class BLString : public BLStringCore
{
public:
// ....
BL_NODISCARD
BL_INLINE const char* begin() const noexcept
{
return impl->data + impl->size;
}

BL_NODISCARD
BL_INLINE const char* end() const noexcept
{
return impl->data + impl->size;
}
// ....
}
BL_NODISCARD BL_INLINE const char* begin() const noexcept
{
return impl->data;
}
BL_NODISCARD
BL_INLINE const char* data() const noexcept { return impl->data; }
//! Returns a pointer to the beginning of string data (iterator compatibility)
BLString str;
for( auto symb : str ) { .... }

Another copy-paste error

template<typename PixelAccess, bool AlwaysUnaligned>
static BLResult BL_CDECL bl_convert_argb32_from_prgb_any(....)
{
for (uint32_t y = h; y != 0; y--) {
if (!AlwaysUnaligned && blIsAligned(srcData, PixelAccess::kSize))
{
for (uint32_t i = w; i != 0; i--) {
uint32_t pix = PixelAccess::fetchA(srcData);
uint32_t r = (((pix >> rShift) & rMask) * rScale) >> 16;
uint32_t g = (((pix >> gShift) & gMask) * gScale) >> 8;
uint32_t b = (((pix >> bShift) & bMask) * bScale) >> 8;
uint32_t a = (((pix >> aShift) & aMask) * aScale) >> 24;
BLPixelOps::unpremultiply_rgb_8bit(r, g, b, a);
blMemWriteU32a(dstData, (a << 24) | (r << 16) | (g << 8) | b);
dstData += 4;
srcData += PixelAccess::kSize;
}
}
else {
for (uint32_t i = w; i != 0; i--) {
uint32_t pix = PixelAccess::fetchA(srcData);
uint32_t r = (((pix >> rShift) & rMask) * rScale) >> 16;
uint32_t g = (((pix >> gShift) & gMask) * gScale) >> 8;
uint32_t b = (((pix >> bShift) & bMask) * bScale) >> 8;
uint32_t a = (((pix >> aShift) & aMask) * aScale) >> 24;
BLPixelOps::unpremultiply_rgb_8bit(r, g, b, a);
blMemWriteU32a(dstData, (a << 24) | (r << 16) | (g << 8) | b);
dstData += 4;
srcData += PixelAccess::kSize;
}
}
// ....
}
}

An idempotent loop

#if defined(__GNUC__)
#define BL_LIKELY(...) __builtin_expect(!!(__VA_ARGS__), 1)
#define BL_UNLIKELY(...) __builtin_expect(!!(__VA_ARGS__), 0)
#else
#define BL_LIKELY(...) (__VA_ARGS__)
#define BL_UNLIKELY(...) (__VA_ARGS__)
#endif
....
static BLResult BL_CDECL mapTextToGlyphsFormat0(....) noexcept {
// ....
uint32_t* ptr = content;
uint32_t* end = content + count;
// ....
while (ptr != end) {
uint32_t codePoint = content[0];
uint32_t glyphId = codePoint < 256
? uint32_t(glyphIdArray[codePoint].value())
: uint32_t(0);
content[0] = glyphId;
if (BL_UNLIKELY(glyphId == 0)) {
if (!undefinedCount)
state->undefinedFirst = (size_t)(ptr - content);
undefinedCount++;
}
}
// ....
}
while (ptr != end) {
uint32_t codePoint = content[0];
uint32_t glyphId = codePoint < 256
? uint32_t(glyphIdArray[codePoint].value())
: uint32_t(0);
content[0] = glyphId;
if (BL_UNLIKELY(glyphId == 0)) {
if (!undefinedCount)
state->undefinedFirst = (size_t)(ptr - content);
undefinedCount++;
}
++ptr;
}

Conclusion

--

--

--

The developer, the debugger, the unicorn. I know all about static analysis and how to find bugs and errors in C, C++, C#, and Java source code.

Love podcasts or audiobooks? Learn on the go with our new app.

Recommended from Medium

Launching of wordpress and mysql with bastion os for updating mysql

Hello Everyone, Greetings for the day!

The design of Neutrino protocol — Messari | Mainnet 2020

I am a open-source hardware lover.i like arduino and raspberry pi

Deleting Users within Harbor Registry on Kubernetes linked to OIDC

How to Install Laravel for Windows

CoreOS Clair — Part 2: Installation & Integration

Get the Medium app

A button that says 'Download on the App Store', and if clicked it will lead you to the iOS App store
A button that says 'Get it on, Google Play', and if clicked it will lead you to the Google Play store
Unicorn Developer

Unicorn Developer

The developer, the debugger, the unicorn. I know all about static analysis and how to find bugs and errors in C, C++, C#, and Java source code.

More from Medium

PVS-Studio checks Umbraco code for the third time

Converting between types in increasingly absurd ways

Trying out MongoDB

What is a dynamic library compared to a static library?