GNU bug report logs -
#45562
[PATCH] Fix "comparison always the same" warnings found by lgtm
Previous Next
Reported by: Stefan Kangas <stefan <at> marxist.se>
Date: Thu, 31 Dec 2020 08:34:01 UTC
Severity: wishlist
Tags: patch
Fixed in version 28.1
Done: Lars Ingebrigtsen <larsi <at> gnus.org>
Bug is archived. No further changes may be made.
Full log
View this message in rfc822 format
Eli Zaretskii <eliz <at> gnu.org> writes:
>> The attached patch fixes some warnings found by lgtm.com.
>
> Thanks. IME, these tools have quite a low signal-to-noise ratio. In
> this case:
Thanks for the review! Indeed, this is why I asked for some comments.
>> --- a/src/buffer.c
>> +++ b/src/buffer.c
>> @@ -5238,8 +5238,7 @@ init_buffer_once (void)
>> PDUMPER_REMEMBER_SCALAR (buffer_local_flags);
>>
>> /* Need more room? */
>> - if (idx >= MAX_PER_BUFFER_VARS)
>> - emacs_abort ();
>> + eassert (idx < MAX_PER_BUFFER_VARS);
>
> This is wrong, because eassert compiles to nothing in the production
> build, so it is only good for situations where continuing without
> aborting will do something reasonable, or if it will crash anyway in
> the very next source line. In this case, there's no way we can
> continue, and the programmer evidently wanted us to abort rather than
> continue and let us crash later.
Right. But we know the value of both idx and MAX_PER_BUFFER_VARS at
compile time. So while I understand your argument, it is arguably a
judgment call whether or not it is worth making this check also in
production builds. IMHO, the eassert has the (minor) benefit of making
the intention clearer.
That said, AFAICT we call this function only once per lifetime. So I'm
happy to leave this out if you prefer.
>> --- a/src/fns.c
>> +++ b/src/fns.c
>> @@ -3847,8 +3847,6 @@ base64_decode_1 (const char *from, char *to, ptrdiff_t length,
>> if (c == '=')
>> continue;
>>
>> - if (v1 < 0)
>> - return -1;
>> value += v1 - 1;
>>
>> c = value & 0xff;
>
> I don't think I see why removing the test and the failure return would
> be TRT. What did I miss?
Because we have above:
do { ... } while (v1 < 0);
So unless I am missing something the test is always false and we will
never reach the return.
This bug report was last modified 4 years and 1 day ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.