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: Thu, 31 Dec 2020 16:12:51 +0200 Message-ID: <83zh1ugj30.fsf@gnu.org> References: Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="29707"; 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 Thu Dec 31 15:14:12 2020 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 1kuyiS-0007eA-0o for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 31 Dec 2020 15:14:12 +0100 Original-Received: from localhost ([::1]:59934 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kuyiQ-0000Qj-Hm for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 31 Dec 2020 09:14:10 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:41928) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kuyiI-0000QZ-L7 for bug-gnu-emacs@gnu.org; Thu, 31 Dec 2020 09:14:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:56395) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kuyiI-0000D7-E6 for bug-gnu-emacs@gnu.org; Thu, 31 Dec 2020 09:14:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1kuyiI-0006T9-8w for bug-gnu-emacs@gnu.org; Thu, 31 Dec 2020 09:14: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: Thu, 31 Dec 2020 14:14: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.160942400024795 (code B ref 45562); Thu, 31 Dec 2020 14:14:02 +0000 Original-Received: (at 45562) by debbugs.gnu.org; 31 Dec 2020 14:13:20 +0000 Original-Received: from localhost ([127.0.0.1]:39702 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kuyhc-0006Rq-9u for submit@debbugs.gnu.org; Thu, 31 Dec 2020 09:13:20 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:48554) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kuyha-0006Rc-Ly for 45562@debbugs.gnu.org; Thu, 31 Dec 2020 09:13:19 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:50621) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kuyhV-0008Nd-8O; Thu, 31 Dec 2020 09:13:13 -0500 Original-Received: from 84.94.185.95.cable.012.net.il ([84.94.185.95]:1338 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1kuyhU-0001DO-67; Thu, 31 Dec 2020 09:13:12 -0500 In-Reply-To: (message from Stefan Kangas on Thu, 31 Dec 2020 00:33:06 -0800) 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:197088 Archived-At: > From: Stefan Kangas > Date: Thu, 31 Dec 2020 00:33:06 -0800 > > 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: > --- a/src/alloc.c > +++ b/src/alloc.c > @@ -4001,7 +4001,7 @@ memory_full (size_t nbytes) > { > if (i == 0) > free (spare_memory[i]); > - else if (i >= 1 && i <= 4) > + else if (i <= 4) > lisp_align_free (spare_memory[i]); > else > lisp_free (spare_memory[i]); This is an optimization better left to the compiler, IMO. > --- 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. > --- 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? > --- a/src/window.c > +++ b/src/window.c > @@ -5708,7 +5708,7 @@ window_scroll_pixel_based (Lisp_Object window, int n, bool whole, bool noerror) > && start_pos > BEGV) > move_it_by_lines (&it, -1); > } > - else if (dy > 0) > + else /* if (dy > 0) */ I don't necessarily object, but this is again an optimization that compilers are better at than people. > --- a/src/xfaces.c > +++ b/src/xfaces.c > @@ -2228,7 +2228,7 @@ merge_face_vectors (struct window *w, > else if (i != LFACE_FONT_INDEX && ! EQ (to[i], from[i])) > { > to[i] = from[i]; > - if (i >= LFACE_FAMILY_INDEX && i <= LFACE_SLANT_INDEX) > + if (i <= LFACE_SLANT_INDEX) This change hard-codes the assumption about the numerical value of LFACE_FAMILY_INDEX, so it'd be a time bomb waiting to blow up. For example, imagine that the enumeration is modified such that the value of LFACE_FAMILY_INDEX changes, or we are using a compiler with a different scheme of assigning numerical values to enumeration constants.