GNU bug report logs - #75451
scratch/igc: Enable CHECK_STRUCTS

Previous Next

Full log

Message #73 received at 75451 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Gerd Möllmann <gerd.moellmann <at> gmail.com>,
 Pip Cet <pipcet <at> protonmail.com>, Gerd Moellmann <gerd <at> gnu.org>,
 75451 <at> debbugs.gnu.org
Subject: Re: bug#75451: scratch/igc: Enable CHECK_STRUCTS
Date: Sun, 19 Jan 2025 21:03:33 -0800

On 2025-01-19 18:01, Stefan Kangas wrote:
> In my attached patch,
> there is one removal of 'default: eassume(false)'; how can I know if
> it's worth keeping or not?

There are a few reasons to have such a line. First, to pacify a GCC 
false alarm. Second, to make the code clearer to a human reader. Third 
(less likely), to generate more efficient code. If no reason applies I 
would omit such lines.


>    int type = XFIXNUM (dump_pop (&lreloc));
>    reloc.emacs_offset = dump_off_from_lisp (dump_pop (&lreloc));
>    dump_check_emacs_off (reloc.emacs_offset);
> -  switch (type)
> +  switch ((enum emacs_reloc_type) type)

When it's easy, let's avoid casts like that as they're too powerful: you 
can cast pointers by mistake. Here it's easy to omit the cast, and 
declare the local variable 'type' to be of type 'enum emacs_reloc_type' 
rather than of type 'int'. (I assume you know for other reasons that the 
value is in range for the enum - otherwise the cast wouldn't be valid 
either.) In other words, just change the first line to:

   enum emacs_reloc_type type = XFIXNUM (dump_pop (&lreloc));

This way, the last line can remain unchanged and there is no need for a 
cast.

Previous Next

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