| Message ID | 20201126112915.525285-10-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Remove GCC < 4.8 checks | expand |
On Thu, 26 Nov 2020 at 11:30, <marcandre.lureau@redhat.com> wrote: > > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > QEMU requires either GCC or Clang, which both advertize __GNUC__. > Drop MSVC fallback path. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > audio/audio.c | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) > > diff --git a/audio/audio.c b/audio/audio.c > index 46578e4a58..d7a00294de 100644 > --- a/audio/audio.c > +++ b/audio/audio.c > @@ -122,13 +122,7 @@ int audio_bug (const char *funcname, int cond) > > #if defined AUDIO_BREAKPOINT_ON_BUG > # if defined HOST_I386 > -# if defined __GNUC__ > - __asm__ ("int3"); > -# elif defined _MSC_VER > - _asm _emit 0xcc; > -# else > - abort (); > -# endif > + __asm__ ("int3"); > # else > abort (); > # endif > -- > 2.29.0 I would prefer to just drop this attempt to emit an inline breakpoint insn (which won't work on non-x86 hosts and seems to me to have no benefit over just calling abort(), which will also drop you into the debugger) and simply make it abort() if AUDIO_BREAKPOINT_ON_BUG is defined. Gerd, do you have an opinion ? thanks -- PMM
Hi On Thu, Nov 26, 2020 at 4:08 PM Peter Maydell <peter.maydell@linaro.org> wrote: > On Thu, 26 Nov 2020 at 11:30, <marcandre.lureau@redhat.com> wrote: > > > > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > > > QEMU requires either GCC or Clang, which both advertize __GNUC__. > > Drop MSVC fallback path. > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > --- > > audio/audio.c | 8 +------- > > 1 file changed, 1 insertion(+), 7 deletions(-) > > > > diff --git a/audio/audio.c b/audio/audio.c > > index 46578e4a58..d7a00294de 100644 > > --- a/audio/audio.c > > +++ b/audio/audio.c > > @@ -122,13 +122,7 @@ int audio_bug (const char *funcname, int cond) > > > > #if defined AUDIO_BREAKPOINT_ON_BUG > > # if defined HOST_I386 > > -# if defined __GNUC__ > > - __asm__ ("int3"); > > -# elif defined _MSC_VER > > - _asm _emit 0xcc; > > -# else > > - abort (); > > -# endif > > + __asm__ ("int3"); > > # else > > abort (); > > # endif > > -- > > 2.29.0 > > I would prefer to just drop this attempt to emit an inline > breakpoint insn (which won't work on non-x86 hosts and seems > to me to have no benefit over just calling abort(), which will > also drop you into the debugger) and simply make it abort() if > AUDIO_BREAKPOINT_ON_BUG is defined. Gerd, do you have an > opinion ? > > Oh yeah.. But audio/ is a rabbit hole, so I had to refrain myself doing more cleanups.
Hi, > > #if defined AUDIO_BREAKPOINT_ON_BUG > > # if defined HOST_I386 > > + __asm__ ("int3"); > > # else > > abort (); > > # endif > > -- > > 2.29.0 > > I would prefer to just drop this attempt to emit an inline > breakpoint insn (which won't work on non-x86 hosts and seems > to me to have no benefit over just calling abort(), which will > also drop you into the debugger) and simply make it abort() if > AUDIO_BREAKPOINT_ON_BUG is defined. Gerd, do you have an > opinion ? kraxel@sirius ~/projects/qemu# git grep AUDIO_BREAKPOINT_ON_BUG audio/audio.c:#if defined AUDIO_BREAKPOINT_ON_BUG Seems this is unused, so just remove it? take care, Gerd
On Fri, Nov 27, 2020 at 11:15 AM Gerd Hoffmann <kraxel@redhat.com> wrote: > Hi, > > > > #if defined AUDIO_BREAKPOINT_ON_BUG > > > # if defined HOST_I386 > > > + __asm__ ("int3"); > > > # else > > > abort (); > > > # endif > > > -- > > > 2.29.0 > > > > I would prefer to just drop this attempt to emit an inline > > breakpoint insn (which won't work on non-x86 hosts and seems > > to me to have no benefit over just calling abort(), which will > > also drop you into the debugger) and simply make it abort() if > > AUDIO_BREAKPOINT_ON_BUG is defined. Gerd, do you have an > > opinion ? > > kraxel@sirius ~/projects/qemu# git grep AUDIO_BREAKPOINT_ON_BUG > audio/audio.c:#if defined AUDIO_BREAKPOINT_ON_BUG > > Seems this is unused, so just remove it? > > I think the original intention was to build with CFLAGS set manually for debugging this limited case. (Iirc, we have other such #ifdef lurking around)
diff --git a/audio/audio.c b/audio/audio.c index 46578e4a58..d7a00294de 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -122,13 +122,7 @@ int audio_bug (const char *funcname, int cond) #if defined AUDIO_BREAKPOINT_ON_BUG # if defined HOST_I386 -# if defined __GNUC__ - __asm__ ("int3"); -# elif defined _MSC_VER - _asm _emit 0xcc; -# else - abort (); -# endif + __asm__ ("int3"); # else abort (); # endif