Discussion:
[PATCH] quotearg: fix compilation failure due to FALLTHROUGH misuse
(too old to reply)
Jim Meyering
2017-05-26 04:27:51 UTC
Permalink
Raw Message
From 6c720446ab4f450b86fe61113096d09f64029de4 Mon Sep 17 00:00:00 2001
From: Jim Meyering <***@fb.com>
Date: Thu, 25 May 2017 21:25:37 -0700
Subject: [PATCH] quotearg: fix compilation failure due to FALLTHROUGH misuse

* lib/quotearg.c (quotearg_buffer_restyled): Revert one FALLTHROUGH
macro back to /* fall through */ comment. The macro can apply only
to a following case statement. Reported by Assaf Gordon.
---
ChangeLog | 7 +++++++
lib/quotearg.c | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 3e78d5a..2252a4a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2017-05-25 Jim Meyering <***@fb.com>
+
+ quotearg: fix compilation failure due to FALLTHROUGH misuse
+ * lib/quotearg.c (quotearg_buffer_restyled): Revert one FALLTHROUGH
+ macro back to /* fall through */ comment. The macro can apply only
+ to a following case statement. Reported by Assaf Gordon.
+
2017-05-25 Paul Eggert <***@cs.ucla.edu>

intprops: port to recent icc
diff --git a/lib/quotearg.c b/lib/quotearg.c
index f1bdf31..06172c1 100644
--- a/lib/quotearg.c
+++ b/lib/quotearg.c
@@ -513,7 +513,7 @@ quotearg_buffer_restyled (char *buffer, size_t buffersize,
if (quoting_style == shell_always_quoting_style
&& elide_outer_quotes)
goto force_outer_quoting_style;
- FALLTHROUGH;
+ /* fall through */
c_escape:
if (backslash_escapes)
{
--
2.9.4
Bernhard Voelker
2017-05-28 10:44:34 UTC
Permalink
Raw Message
From 6c720446ab4f450b86fe61113096d09f64029de4 Mon Sep 17 00:00:00 2001
Date: Thu, 25 May 2017 21:25:37 -0700
Subject: [PATCH] quotearg: fix compilation failure due to FALLTHROUGH misuse
* lib/quotearg.c (quotearg_buffer_restyled): Revert one FALLTHROUGH
macro back to /* fall through */ comment. The macro can apply only
to a following case statement. Reported by Assaf Gordon.
...
diff --git a/lib/quotearg.c b/lib/quotearg.c
index f1bdf31..06172c1 100644
--- a/lib/quotearg.c
+++ b/lib/quotearg.c
@@ -513,7 +513,7 @@ quotearg_buffer_restyled (char *buffer, size_t buffersize,
if (quoting_style == shell_always_quoting_style
&& elide_outer_quotes)
goto force_outer_quoting_style;
- FALLTHROUGH;
+ /* fall through */
if (backslash_escapes)
{
Sorry, you lost me:
"c_escape:" /is/ a case statement, so what's the matter here?
The only difference I can see here is that there is a conditional
goto before the FALLTHROUGH macro. I'm far away from being an
expert in this region, but this looks like a compiler bug to me.
WDYT?

Furthermore, the macro is now an empty statement for older
compilers. I've no idea what all the static analyzers in the world
are expecting here, but shouldn't the macro more look like the old
version for such tools?

#ifndef FALLTHROUGH
# if __GNUC__ < 7
-# define FALLTHROUGH ((void) 0)
+# define FALLTHROUGH /* fall through */
# else
# define FALLTHROUGH __attribute__ ((__fallthrough__))
# endif

(Okay, there'd still be an empty ';' statement afterward.)

Thanks & have a nice day,
Berny
Jim Meyering
2017-05-29 02:51:37 UTC
Permalink
Raw Message
On Sun, May 28, 2017 at 3:44 AM, Bernhard Voelker
Post by Bernhard Voelker
Post by Jim Meyering
goto force_outer_quoting_style;
- FALLTHROUGH;
+ /* fall through */
if (backslash_escapes)
{
"c_escape:" /is/ a case statement, so what's the matter here?
Hi Bernie,

That "c_escape:" is a goto label (not a case label), so putting the
attribute before it would evoke a compilation error from newer gcc.
Bernhard Voelker
2017-05-29 21:30:29 UTC
Permalink
Raw Message
Post by Jim Meyering
On Sun, May 28, 2017 at 3:44 AM, Bernhard Voelker
Post by Bernhard Voelker
Post by Jim Meyering
goto force_outer_quoting_style;
- FALLTHROUGH;
+ /* fall through */
if (backslash_escapes)
{
"c_escape:" /is/ a case statement, so what's the matter here?
That "c_escape:" is a goto label (not a case label), so putting the
attribute before it would evoke a compilation error from newer gcc.
ah, right. So your initial patch to remove the fall through
marker was correct as we don't need them for goto labels ...
like e.g. 5 lines above.

Sorry for the confusion. I'll get my glasses in a couple of days.

Thanks & have a nice day,
Berny

Loading...