* [PATCH] sim: add default cases to two switches in sim-options.c
@ 2021-05-02 14:17 Simon Marchi
2021-05-02 15:01 ` Mike Frysinger
0 siblings, 1 reply; 3+ messages in thread
From: Simon Marchi @ 2021-05-02 14:17 UTC (permalink / raw)
To: gdb-patches
This is the next compilation error I hit when I build all targets with
Clang:
/home/simark/src/binutils-gdb/sim/aarch64/../common/sim-options.c:234:12: error: no case matching constant switch condition '0' [-Werror] switch (WITH_ENVIRONMENT)
^~~~~~~~~~~~~~~~ ./config.h:215:26: note: expanded from macro 'WITH_ENVIRONMENT'
#define WITH_ENVIRONMENT ALL_ENVIRONMENT ^~~~~~~~~~~~~~~
/home/simark/src/binutils-gdb/sim/aarch64/../common/sim-options.c:276:15: error: no case matching constant switch condition '0' [-Werror] switch (WITH_ALIGNMENT)
^~~~~~~~~~~~~~ /home/simark/src/binutils-gdb/sim/aarch64/../common/sim-config.h:220:24: note: expanded from macro 'WITH_ALIGNMENT'
#define WITH_ALIGNMENT 0
^
This is a little bit special because these are switches on compile-time
value. But regardless, the idea is that we logically can't reach the
switches if WITH_ENVIRONMENT == 0 or WITH_ALIGNMENT == 0, so the code is
correct.
In addition to getting rid of the compiler warning, adding default cases
to these switches ensure that if we do get in an unexpected situation,
it is caught. In GDB, I'd use gdb_assert_not_reached, I don't know if
there is something similar in sim so I went with abort.
sim/common/ChangeLog:
* sim-options.c (standard_option_handler): Add default cases to
switches.
Change-Id: Ie237d67a201caa6b72de0d17cc815193417156b6
---
sim/common/sim-options.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/sim/common/sim-options.c b/sim/common/sim-options.c
index 1522cac9379f..1efb21fc62b8 100644
--- a/sim/common/sim-options.c
+++ b/sim/common/sim-options.c
@@ -236,6 +236,7 @@ standard_option_handler (SIM_DESC sd, sim_cpu *cpu, int opt,
case USER_ENVIRONMENT: type = "user"; break;
case VIRTUAL_ENVIRONMENT: type = "virtual"; break;
case OPERATING_ENVIRONMENT: type = "operating"; break;
+ default: abort ();
}
sim_io_eprintf (sd, "Simulator compiled for the %s environment only.\n",
type);
@@ -284,6 +285,7 @@ standard_option_handler (SIM_DESC sd, sim_cpu *cpu, int opt,
case FORCED_ALIGNMENT:
sim_io_eprintf (sd, "Simulator compiled for forced alignment only.\n");
break;
+ default: abort ();
}
return SIM_RC_FAIL;
--
2.30.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] sim: add default cases to two switches in sim-options.c
2021-05-02 14:17 [PATCH] sim: add default cases to two switches in sim-options.c Simon Marchi
@ 2021-05-02 15:01 ` Mike Frysinger
2021-05-02 15:06 ` Simon Marchi
0 siblings, 1 reply; 3+ messages in thread
From: Mike Frysinger @ 2021-05-02 15:01 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
On 02 May 2021 10:17, Simon Marchi via Gdb-patches wrote:
> This is the next compilation error I hit when I build all targets with
> Clang:
>
> /home/simark/src/binutils-gdb/sim/aarch64/../common/sim-options.c:234:12: error: no case matching constant switch condition '0' [-Werror] switch (WITH_ENVIRONMENT)
> ^~~~~~~~~~~~~~~~ ./config.h:215:26: note: expanded from macro 'WITH_ENVIRONMENT'
> #define WITH_ENVIRONMENT ALL_ENVIRONMENT ^~~~~~~~~~~~~~~
> /home/simark/src/binutils-gdb/sim/aarch64/../common/sim-options.c:276:15: error: no case matching constant switch condition '0' [-Werror] switch (WITH_ALIGNMENT)
> ^~~~~~~~~~~~~~ /home/simark/src/binutils-gdb/sim/aarch64/../common/sim-config.h:220:24: note: expanded from macro 'WITH_ALIGNMENT'
> #define WITH_ALIGNMENT 0
> ^
>
> This is a little bit special because these are switches on compile-time
> value. But regardless, the idea is that we logically can't reach the
> switches if WITH_ENVIRONMENT == 0 or WITH_ALIGNMENT == 0, so the code is
> correct.
lgtm, thanks
> In addition to getting rid of the compiler warning, adding default cases
> to these switches ensure that if we do get in an unexpected situation,
> it is caught. In GDB, I'd use gdb_assert_not_reached, I don't know if
> there is something similar in sim so I went with abort.
abort() is what we have atm for things like this. if it were due to a user
error (passing wrong values), we'd do something else, but this is a compile
time assertion.
-mike
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] sim: add default cases to two switches in sim-options.c
2021-05-02 15:01 ` Mike Frysinger
@ 2021-05-02 15:06 ` Simon Marchi
0 siblings, 0 replies; 3+ messages in thread
From: Simon Marchi @ 2021-05-02 15:06 UTC (permalink / raw)
To: gdb-patches
On 2021-05-02 11:01 a.m., Mike Frysinger wrote:
> On 02 May 2021 10:17, Simon Marchi via Gdb-patches wrote:
>> This is the next compilation error I hit when I build all targets with
>> Clang:
>>
>> /home/simark/src/binutils-gdb/sim/aarch64/../common/sim-options.c:234:12: error: no case matching constant switch condition '0' [-Werror] switch (WITH_ENVIRONMENT)
>> ^~~~~~~~~~~~~~~~ ./config.h:215:26: note: expanded from macro 'WITH_ENVIRONMENT'
>> #define WITH_ENVIRONMENT ALL_ENVIRONMENT ^~~~~~~~~~~~~~~
>> /home/simark/src/binutils-gdb/sim/aarch64/../common/sim-options.c:276:15: error: no case matching constant switch condition '0' [-Werror] switch (WITH_ALIGNMENT)
>> ^~~~~~~~~~~~~~ /home/simark/src/binutils-gdb/sim/aarch64/../common/sim-config.h:220:24: note: expanded from macro 'WITH_ALIGNMENT'
>> #define WITH_ALIGNMENT 0
>> ^
>>
>> This is a little bit special because these are switches on compile-time
>> value. But regardless, the idea is that we logically can't reach the
>> switches if WITH_ENVIRONMENT == 0 or WITH_ALIGNMENT == 0, so the code is
>> correct.
>
> lgtm, thanks
>
>> In addition to getting rid of the compiler warning, adding default cases
>> to these switches ensure that if we do get in an unexpected situation,
>> it is caught. In GDB, I'd use gdb_assert_not_reached, I don't know if
>> there is something similar in sim so I went with abort.
>
> abort() is what we have atm for things like this. if it were due to a user
> error (passing wrong values), we'd do something else, but this is a compile
> time assertion.
Indeed, I agree. Pushed, thanks!
Simon
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-05-02 15:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-02 14:17 [PATCH] sim: add default cases to two switches in sim-options.c Simon Marchi
2021-05-02 15:01 ` Mike Frysinger
2021-05-02 15:06 ` Simon Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).