Enable ClangTidy flag bugprone-suspicious-string-compare.
https://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-string-compare.html
Find suspicious usage of runtime string comparison functions.
This check is valid in C and C++.
Checks for calls with implicit comparator and proposed to
explicitly add it:
if (strcmp(...)) // Implicitly compare to zero
if (!strcmp(...)) // Won't warn
if (strcmp(...) != 0) // Won't warn
Checks that compare function results (i,e, strcmp) are compared to valid
constant. The resulting value is
< 0 when lower than,
> 0 when greater than,
== 0 when equals.
A common mistake is to compare the result to 1 or -1:
if (strcmp(...) == -1) // Incorrect usage of the returned value.
Additionally, the check warns if the results value is implicitly cast
to a suspicious non-integer type. It’s happening when the returned
value is used in a wrong context:
if (strcmp(...) < 0.) // Incorrect usage of the returned value.
Change-Id: I001b88d06cc4f3eb5846103885be675f9b78e126
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310761
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
diff --git a/.clang-tidy b/.clang-tidy
index cb38913..178f4a5 100644
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -2,6 +2,7 @@
-*,
bugprone-argument-comment,
bugprone-bool-pointer-implicit-conversion,
+ bugprone-suspicious-string-compare,
bugprone-undelegated-constructor,
bugprone-unused-raii,
bugprone-use-after-move,
diff --git a/dm/DM.cpp b/dm/DM.cpp
index 65e83de..345a8e7 100644
--- a/dm/DM.cpp
+++ b/dm/DM.cpp
@@ -1382,7 +1382,7 @@
sk_mkdir(path.c_str());
path = SkOSPath::Join(path.c_str(), task.src.tag.c_str());
sk_mkdir(path.c_str());
- if (strcmp(task.src.options.c_str(), "") != 0) {
+ if (0 != strcmp(task.src.options.c_str(), "")) {
path = SkOSPath::Join(path.c_str(), task.src.options.c_str());
sk_mkdir(path.c_str());
}
diff --git a/gm/exoticformats.cpp b/gm/exoticformats.cpp
index 9cd1925..02f919b 100644
--- a/gm/exoticformats.cpp
+++ b/gm/exoticformats.cpp
@@ -57,7 +57,7 @@
0xAB, 0x4B, 0x54, 0x58, 0x20, 0x31, 0x31, 0xBB, 0x0D, 0x0A, 0x1A, 0x0A
};
- if (memcmp(header, kExpectedIdentifier, kKTXIdentifierSize)) {
+ if (0 != memcmp(header, kExpectedIdentifier, kKTXIdentifierSize)) {
return nullptr;
}
diff --git a/modules/skottie/src/animator/VectorKeyframeAnimator.cpp b/modules/skottie/src/animator/VectorKeyframeAnimator.cpp
index 008b751..386093c 100644
--- a/modules/skottie/src/animator/VectorKeyframeAnimator.cpp
+++ b/modules/skottie/src/animator/VectorKeyframeAnimator.cpp
@@ -98,7 +98,7 @@
auto* dst = fTarget->data();
if (lerp_info.isConstant()) {
- if (std::memcmp(dst, v0, fVecLen * sizeof(float))) {
+ if (0 != std::memcmp(dst, v0, fVecLen * sizeof(float))) {
std::copy(v0, v0 + fVecLen, dst);
return true;
}
diff --git a/modules/skottie/utils/SkottieUtils.cpp b/modules/skottie/utils/SkottieUtils.cpp
index 0da67d0..7f699ff 100644
--- a/modules/skottie/utils/SkottieUtils.cpp
+++ b/modules/skottie/utils/SkottieUtils.cpp
@@ -211,7 +211,7 @@
sk_sp<skottie::ExternalLayer> ExternalAnimationPrecompInterceptor::onLoadPrecomp(
const char[], const char name[], const SkSize& size) {
- if (strncmp(name, fPrefix.c_str(), fPrefix.size())) {
+ if (0 != strncmp(name, fPrefix.c_str(), fPrefix.size())) {
return nullptr;
}
diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index f79c8cc..9aa7d2f 100644
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -46,7 +46,7 @@
}
constexpr uint8_t kExifSig[] { 'E', 'x', 'i', 'f', '\0' };
- if (memcmp(marker->data, kExifSig, sizeof(kExifSig))) {
+ if (0 != memcmp(marker->data, kExifSig, sizeof(kExifSig))) {
return false;
}
diff --git a/src/gpu/vk/GrVkCommandBuffer.cpp b/src/gpu/vk/GrVkCommandBuffer.cpp
index e8d5d6b..c950407 100644
--- a/src/gpu/vk/GrVkCommandBuffer.cpp
+++ b/src/gpu/vk/GrVkCommandBuffer.cpp
@@ -357,7 +357,7 @@
const VkViewport* viewports) {
SkASSERT(fIsActive);
SkASSERT(1 == viewportCount);
- if (memcmp(viewports, &fCachedViewport, sizeof(VkViewport))) {
+ if (0 != memcmp(viewports, &fCachedViewport, sizeof(VkViewport))) {
GR_VK_CALL(gpu->vkInterface(), CmdSetViewport(fCmdBuffer,
firstViewport,
viewportCount,
@@ -372,7 +372,7 @@
const VkRect2D* scissors) {
SkASSERT(fIsActive);
SkASSERT(1 == scissorCount);
- if (memcmp(scissors, &fCachedScissor, sizeof(VkRect2D))) {
+ if (0 != memcmp(scissors, &fCachedScissor, sizeof(VkRect2D))) {
GR_VK_CALL(gpu->vkInterface(), CmdSetScissor(fCmdBuffer,
firstScissor,
scissorCount,
@@ -384,7 +384,7 @@
void GrVkCommandBuffer::setBlendConstants(const GrVkGpu* gpu,
const float blendConstants[4]) {
SkASSERT(fIsActive);
- if (memcmp(blendConstants, fCachedBlendConstant, 4 * sizeof(float))) {
+ if (0 != memcmp(blendConstants, fCachedBlendConstant, 4 * sizeof(float))) {
GR_VK_CALL(gpu->vkInterface(), CmdSetBlendConstants(fCmdBuffer, blendConstants));
memcpy(fCachedBlendConstant, blendConstants, 4 * sizeof(float));
}
diff --git a/src/ports/SkFontConfigInterface_direct.cpp b/src/ports/SkFontConfigInterface_direct.cpp
index b888287..54d2a4d 100644
--- a/src/ports/SkFontConfigInterface_direct.cpp
+++ b/src/ports/SkFontConfigInterface_direct.cpp
@@ -509,8 +509,8 @@
#ifdef SK_FONT_CONFIG_INTERFACE_ONLY_ALLOW_SFNT_FONTS
const char* font_format = get_string(pattern, FC_FONTFORMAT);
if (font_format
- && strcmp(font_format, kFontFormatTrueType) != 0
- && strcmp(font_format, kFontFormatCFF) != 0)
+ && 0 != strcmp(font_format, kFontFormatTrueType)
+ && 0 != strcmp(font_format, kFontFormatCFF))
{
return false;
}
diff --git a/src/utils/SkCustomTypeface.cpp b/src/utils/SkCustomTypeface.cpp
index 2fbdd75..00e0be7 100644
--- a/src/utils/SkCustomTypeface.cpp
+++ b/src/utils/SkCustomTypeface.cpp
@@ -364,7 +364,7 @@
char header[kHeaderSize];
if (stream->read(header, kHeaderSize) != kHeaderSize ||
- memcmp(header, gHeaderString, kHeaderSize) != 0)
+ 0 != memcmp(header, gHeaderString, kHeaderSize))
{
return nullptr;
}
diff --git a/src/utils/SkParseColor.cpp b/src/utils/SkParseColor.cpp
index 9e3708e..7260365 100644
--- a/src/utils/SkParseColor.cpp
+++ b/src/utils/SkParseColor.cpp
@@ -304,7 +304,7 @@
return strcmp(name, key) < 0;
});
- if (rec == std::end(gColorNames) || strcmp(name, *rec)) {
+ if (rec == std::end(gColorNames) || 0 != strcmp(name, *rec)) {
return nullptr;
}
diff --git a/tests/AAClipTest.cpp b/tests/AAClipTest.cpp
index e492ca9..88fbf03 100644
--- a/tests/AAClipTest.cpp
+++ b/tests/AAClipTest.cpp
@@ -59,7 +59,7 @@
const char* aptr = (const char*)a.fImage;
const char* bptr = (const char*)b.fImage;
for (int y = 0; y < h; ++y) {
- if (memcmp(aptr, bptr, wbytes)) {
+ if (0 != memcmp(aptr, bptr, wbytes)) {
return false;
}
aptr += wbytes;
diff --git a/tests/ClipperTest.cpp b/tests/ClipperTest.cpp
index d518c8d..53d3ba2 100644
--- a/tests/ClipperTest.cpp
+++ b/tests/ClipperTest.cpp
@@ -123,7 +123,7 @@
};
for (i = 0; i < SK_ARRAY_COUNT(gFull); i += 2) {
bool valid = SkLineClipper::IntersectLine(&gFull[i], gR, dst);
- if (!valid || memcmp(&gFull[i], dst, sizeof(dst))) {
+ if (!valid || 0 != memcmp(&gFull[i], dst, sizeof(dst))) {
SkDebugf("++++ [%d] %g %g -> %g %g\n", i/2, dst[0].fX, dst[0].fY, dst[1].fX, dst[1].fY);
}
REPORTER_ASSERT(reporter, valid && !memcmp(&gFull[i], dst, sizeof(dst)));
@@ -142,7 +142,7 @@
};
for (i = 0; i < SK_ARRAY_COUNT(gPartial); i += 4) {
bool valid = SkLineClipper::IntersectLine(&gPartial[i], gR, dst);
- if (!valid || memcmp(&gPartial[i+2], dst, sizeof(dst))) {
+ if (!valid || 0 != memcmp(&gPartial[i+2], dst, sizeof(dst))) {
SkDebugf("++++ [%d] %g %g -> %g %g\n", i/2, dst[0].fX, dst[0].fY, dst[1].fX, dst[1].fY);
}
REPORTER_ASSERT(reporter, valid &&
diff --git a/tests/CodecAnimTest.cpp b/tests/CodecAnimTest.cpp
index 9f7e115..016f773 100644
--- a/tests/CodecAnimTest.cpp
+++ b/tests/CodecAnimTest.cpp
@@ -445,7 +445,7 @@
REPORTER_ASSERT(r, result == SkCodec::kSuccess);
for (int y = 0; y < info.height(); ++y) {
- if (memcmp(bm.getAddr32(0, y), bm2.getAddr32(0, y), info.minRowBytes())) {
+ if (0 != memcmp(bm.getAddr32(0, y), bm2.getAddr32(0, y), info.minRowBytes())) {
ERRORF(r, "pixel mismatch for sample size %i, frame %i resulting in "
"dimensions %i x %i line %i\n",
sampleSize, i, info.width(), info.height(), y);
diff --git a/tests/CodecPartialTest.cpp b/tests/CodecPartialTest.cpp
index fba55d1..152795f 100644
--- a/tests/CodecPartialTest.cpp
+++ b/tests/CodecPartialTest.cpp
@@ -50,7 +50,7 @@
}
const size_t rowBytes = info.minRowBytes();
for (int i = 0; i < info.height(); i++) {
- if (memcmp(bm1.getAddr(0, i), bm2.getAddr(0, i), rowBytes)) {
+ if (0 != memcmp(bm1.getAddr(0, i), bm2.getAddr(0, i), rowBytes)) {
ERRORF(r, "Bitmaps have different pixels, starting on line %i!", i);
return false;
}
diff --git a/tests/RecordingXfermodeTest.cpp b/tests/RecordingXfermodeTest.cpp
index 676198a..16e63ea 100644
--- a/tests/RecordingXfermodeTest.cpp
+++ b/tests/RecordingXfermodeTest.cpp
@@ -160,7 +160,7 @@
REPORTER_ASSERT(reporter,
0 == memcmp(goldenBM.getPixels(), pictureBM.getPixels(), pixelsSize));
#else
- if (memcmp(goldenBM.getPixels(), pictureBM.getPixels(), pixelsSize)) {
+ if (0 != memcmp(goldenBM.getPixels(), pictureBM.getPixels(), pixelsSize)) {
numErrors++;
errors.appendf("For SkXfermode %d %s: SkPictureRecorder bitmap is wrong\n",
iMode, SkBlendMode_Name(mode));
diff --git a/tests/SerialProcsTest.cpp b/tests/SerialProcsTest.cpp
index 4702552..0c0c520 100644
--- a/tests/SerialProcsTest.cpp
+++ b/tests/SerialProcsTest.cpp
@@ -44,7 +44,7 @@
},
[](const void* data, size_t length, void* ctx) -> sk_sp<SkImage> {
State* state = (State*)ctx;
- if (length != strlen(state->fStr)+1 || memcmp(data, state->fStr, length)) {
+ if (length != strlen(state->fStr)+1 || 0 != memcmp(data, state->fStr, length)) {
return nullptr;
}
return sk_ref_sp(state->fImg);
diff --git a/tests/SkRuntimeEffectTest.cpp b/tests/SkRuntimeEffectTest.cpp
index 05b7e55..fec8485 100644
--- a/tests/SkRuntimeEffectTest.cpp
+++ b/tests/SkRuntimeEffectTest.cpp
@@ -154,7 +154,7 @@
}
GrColor expected[4] = {TL, TR, BL, BR};
- if (memcmp(actual, expected, sizeof(actual)) != 0) {
+ if (0 != memcmp(actual, expected, sizeof(actual))) {
REPORT_FAILURE(fReporter, "Runtime effect didn't match expectations",
SkStringPrintf("\n"
"Expected: [ %08x %08x %08x %08x ]\n"
diff --git a/tests/SkSLInterpreterTest.cpp b/tests/SkSLInterpreterTest.cpp
index 3541ded..2d4045b 100644
--- a/tests/SkSLInterpreterTest.cpp
+++ b/tests/SkSLInterpreterTest.cpp
@@ -117,7 +117,7 @@
// Transpose striped outputs back
transpose(out_v);
- if (memcmp(out_s, out_v, sizeof(out_s)) != 0) {
+ if (0 != memcmp(out_s, out_v, sizeof(out_s))) {
printf("for program: %s\n", src);
for (int i = 0; i < 4; ++i) {
printf("(%g %g %g %g) -> (%g %g %g %g), expected (%g %g %g %g)\n",
diff --git a/tests/VerticesTest.cpp b/tests/VerticesTest.cpp
index 5f2cbbc..d187d1c 100644
--- a/tests/VerticesTest.cpp
+++ b/tests/VerticesTest.cpp
@@ -63,7 +63,7 @@
}
size_t totalCustomDataSize = v0.vertexCount() * v0.customDataSize();
if (totalCustomDataSize) {
- if (memcmp(v0.customData(), v1.customData(), totalCustomDataSize) != 0) {
+ if (0 != memcmp(v0.customData(), v1.customData(), totalCustomDataSize)) {
return false;
}
}
diff --git a/tools/ToolUtils.cpp b/tools/ToolUtils.cpp
index 4c1dbbc..c91b981 100644
--- a/tools/ToolUtils.cpp
+++ b/tools/ToolUtils.cpp
@@ -436,7 +436,7 @@
for (int y = 0; y < a.height(); ++y) {
const char* aptr = (const char*)a.addr(0, y);
const char* bptr = (const char*)b.addr(0, y);
- if (memcmp(aptr, bptr, a.width() * a.info().bytesPerPixel())) {
+ if (0 != memcmp(aptr, bptr, a.width() * a.info().bytesPerPixel())) {
return false;
}
}
diff --git a/tools/gpu/vk/VkTestUtils.cpp b/tools/gpu/vk/VkTestUtils.cpp
index 41c0f2e..8fe2b57 100644
--- a/tools/gpu/vk/VkTestUtils.cpp
+++ b/tools/gpu/vk/VkTestUtils.cpp
@@ -478,7 +478,7 @@
instanceLayerNames.push_back(instanceLayers[i].layerName);
}
for (int i = 0; i < instanceExtensions.count(); ++i) {
- if (strncmp(instanceExtensions[i].extensionName, "VK_KHX", 6)) {
+ if (strncmp(instanceExtensions[i].extensionName, "VK_KHX", 6) != 0) {
instanceExtensionNames.push_back(instanceExtensions[i].extensionName);
}
}
@@ -652,11 +652,11 @@
// Don't use experimental extensions since they typically don't work with debug layers and
// often are missing dependecy requirements for other extensions. Additionally, these are
// often left behind in the driver even after they've been promoted to real extensions.
- if (strncmp(deviceExtensions[i].extensionName, "VK_KHX", 6) &&
- strncmp(deviceExtensions[i].extensionName, "VK_NVX", 6)) {
+ if (0 != strncmp(deviceExtensions[i].extensionName, "VK_KHX", 6) &&
+ 0 != strncmp(deviceExtensions[i].extensionName, "VK_NVX", 6)) {
if (!hasKHRBufferDeviceAddress ||
- strcmp(deviceExtensions[i].extensionName, "VK_EXT_buffer_device_address")) {
+ 0 != strcmp(deviceExtensions[i].extensionName, "VK_EXT_buffer_device_address")) {
deviceExtensionNames.push_back(deviceExtensions[i].extensionName);
}
}