[Rpm-maint] [rpm-software-management/rpm] Fix signed vs unsigned comparisons (PR #3515)

[Panu Matilainen]

@pmatilai commented on this pull request.



> @@ -337,7 +337,7 @@ static rpmRC parseForVerify(char * buf, int def, FileEntry entry)
     if ((p = strstr(buf, name)) == NULL)
 	return RPMRC_OK;
 
-    for (pe = p; (pe-p) < strlen(name); pe++)
+    for (pe = p; (pe-p) < (ssize_t) strlen(name); pe++)

Another observation on this is all these strlen() casts to signed is actually an anti-pattern, because in this context we *know* pe is always >= p so the difference is always unsigned and casting it to signed makes it look otherwise.

Casting en masse is a clear sign that here's an issue that needs a different solution. Casts should only really be used as the last resort on isolated spots where they tell the reader "watchout, there's something special going on." And what happens in *here* is nothing special, it's just ancient C code that needs a better overall solution.

Finally, we shouldn't be adding C-style casts at all anymore. For the initial C++ build conversion there was simply no choice, but we're in permanent C++ land now and should use the C++ forms: https://en.cppreference.com/w/cpp/language/explicit_cast

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/3515#pullrequestreview-2545929202
You are receiving this because you are subscribed to this thread.

Message ID: <rpm-software-management/rpm/pull/3515/review/2545929202 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20250112/94446272/attachment.htm>