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>
This commit is contained in:
John Stiles 2020-08-15 23:22:53 -04:00 committed by Skia Commit-Bot
parent d4a040e8b2
commit c1c3c6d70d
21 changed files with 28 additions and 27 deletions

View File

@ -2,6 +2,7 @@ Checks: >
-*,
bugprone-argument-comment,
bugprone-bool-pointer-implicit-conversion,
bugprone-suspicious-string-compare,
bugprone-undelegated-constructor,
bugprone-unused-raii,
bugprone-use-after-move,

View File

@ -1382,7 +1382,7 @@ struct Task {
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());
}

View File

@ -57,7 +57,7 @@ static sk_sp<SkData> load_ktx(const char* filename, ImageInfo* imageInfo) {
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;
}

View File

@ -98,7 +98,7 @@ private:
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;
}

View File

@ -211,7 +211,7 @@ ExternalAnimationPrecompInterceptor::~ExternalAnimationPrecompInterceptor() = de
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;
}

View File

@ -46,7 +46,7 @@ static bool is_orientation_marker(jpeg_marker_struct* marker, SkEncodedOrigin* o
}
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;
}

View File

@ -357,7 +357,7 @@ void GrVkCommandBuffer::setViewport(const GrVkGpu* gpu,
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 @@ void GrVkCommandBuffer::setScissor(const GrVkGpu* gpu,
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::setScissor(const GrVkGpu* gpu,
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));
}

View File

@ -509,8 +509,8 @@ bool SkFontConfigInterfaceDirect::isValidPattern(FcPattern* pattern) {
#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;
}

View File

@ -364,7 +364,7 @@ sk_sp<SkTypeface> SkCustomTypefaceBuilder::Deserialize(SkStream* stream) {
char header[kHeaderSize];
if (stream->read(header, kHeaderSize) != kHeaderSize ||
memcmp(header, gHeaderString, kHeaderSize) != 0)
0 != memcmp(header, gHeaderString, kHeaderSize))
{
return nullptr;
}

View File

@ -304,7 +304,7 @@ const char* SkParse::FindNamedColor(const char* name, size_t len, SkColor* color
return strcmp(name, key) < 0;
});
if (rec == std::end(gColorNames) || strcmp(name, *rec)) {
if (rec == std::end(gColorNames) || 0 != strcmp(name, *rec)) {
return nullptr;
}

View File

@ -59,7 +59,7 @@ static bool operator==(const SkMask& a, const SkMask& b) {
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;

View File

@ -123,7 +123,7 @@ static void test_intersectline(skiatest::Reporter* reporter) {
};
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 @@ static void test_intersectline(skiatest::Reporter* reporter) {
};
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 &&

View File

@ -445,7 +445,7 @@ DEF_TEST(AndroidCodec_animated, r) {
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);

View File

@ -50,7 +50,7 @@ static bool compare_bitmaps(skiatest::Reporter* r, const SkBitmap& bm1, const Sk
}
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;
}

View File

@ -160,7 +160,7 @@ DEF_TEST(SkRecordingAccuracyXfermode, reporter) {
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));

View File

@ -44,7 +44,7 @@ DEF_TEST(serial_procs_image, reporter) {
},
[](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);

View File

@ -154,7 +154,7 @@ public:
}
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"

View File

@ -117,7 +117,7 @@ void vec_test(skiatest::Reporter* r, const char* src) {
// 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",

View File

@ -63,7 +63,7 @@ static bool equal(const SkVertices* vert0, const SkVertices* vert1) {
}
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;
}
}

View File

@ -436,7 +436,7 @@ bool equal_pixels(const SkPixmap& a, const SkPixmap& b) {
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;
}
}

View File

@ -478,7 +478,7 @@ bool CreateVkBackendContext(GrVkGetProc getProc,
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 @@ bool CreateVkBackendContext(GrVkGetProc getProc,
// 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);
}
}