From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#45562: [PATCH] Fix "comparison always the same" warnings found by lgtm Date: Fri, 01 Jan 2021 20:17:35 +0200 Message-ID: <83lfdcfrnk.fsf@gnu.org> References: <83zh1ugj30.fsf@gnu.org> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="11721"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 45562@debbugs.gnu.org To: Stefan Kangas Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Jan 01 19:18:21 2021 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kvP0H-0002wQ-8K for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 01 Jan 2021 19:18:21 +0100 Original-Received: from localhost ([::1]:44942 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kvP0F-0001wt-Hn for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 01 Jan 2021 13:18:19 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:57136) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kvP02-0001wb-7H for bug-gnu-emacs@gnu.org; Fri, 01 Jan 2021 13:18:07 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:51480) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kvOzy-0007hB-KY for bug-gnu-emacs@gnu.org; Fri, 01 Jan 2021 13:18:05 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1kvOzy-0001AP-GL for bug-gnu-emacs@gnu.org; Fri, 01 Jan 2021 13:18:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 01 Jan 2021 18:18:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 45562 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 45562-submit@debbugs.gnu.org id=B45562.16095250814479 (code B ref 45562); Fri, 01 Jan 2021 18:18:02 +0000 Original-Received: (at 45562) by debbugs.gnu.org; 1 Jan 2021 18:18:01 +0000 Original-Received: from localhost ([127.0.0.1]:34793 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kvOzx-0001AA-BL for submit@debbugs.gnu.org; Fri, 01 Jan 2021 13:18:01 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:35702) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kvOzv-00019y-VD for 45562@debbugs.gnu.org; Fri, 01 Jan 2021 13:18:00 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:34181) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kvOzq-0007eD-Cr; Fri, 01 Jan 2021 13:17:54 -0500 Original-Received: from 84.94.185.95.cable.012.net.il ([84.94.185.95]:2575 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1kvOzp-0006xB-1R; Fri, 01 Jan 2021 13:17:54 -0500 In-Reply-To: (message from Stefan Kangas on Fri, 1 Jan 2021 10:21:24 -0600) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:197141 Archived-At: > From: Stefan Kangas > Date: Fri, 1 Jan 2021 10:21:24 -0600 > Cc: 45562@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)