I know what undefined behavior is, and they do warrant some investigation. But
just like compiler warnings, this stuff needs to be taken with a grain of salt.
I've seen so much serious damage done from well-intended added "fixes" to
compiler warnings and the like that I've grown quite wary.
In this particular case, it *doesn't matter* what happens to that integer,
it'll still be an in32_t integer whose value we need to check, the value is
*always* unreliable. There's usually a way to improve the surrounding code in a
way that makes the issue (whether compiler warning or otherwise) moot to begin
with. It's just that "avoid negation on attacked controlled thing" sounds very
dramatic when *that* is not an actual issue at all. If we fail to verify the
result is legitimate, that would be. This is a bit of a generic problem with
these PR's of yours: there's an excess sense of danger communicated. If code is
being modified primarily to appease some checker / otherwise theoretical issue
then it needs to be mentioned in the commit message really, and similarly
concrete problems should be flagged out as such.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1502#issuecomment-762165341
_______________________________________________
Rpm-maint mailing list
[email protected]
http://lists.rpm.org/mailman/listinfo/rpm-maint
