GNU bug report logs - #45562
[PATCH] Fix "comparison always the same" warnings found by lgtm

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 45562 <at> debbugs.gnu.org
Subject: bug#45562: [PATCH] Fix "comparison always the same" warnings found by lgtm
Date: Fri, 01 Jan 2021 20:17:35 +0200

> From: Stefan Kangas <stefan <at> marxist.se>
> Date: Fri, 1 Jan 2021 10:21:24 -0600
> Cc: 45562 <at> debbugs.gnu.org
> 
> >> -  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.

A user who builds his/her own Emacs could modify the source to add
some buffer-local variable and overflow MAX_PER_BUFFER_VARS.  If they
build with optimizations, we still want to abort, right?

> >> --- 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.

Or maybe the test is in error, and it should say

  if (v1 == 0)

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.