* [PATCH] afs: Move comments after /* fallthrough */
@ 2019-08-15 2:36 Joe Perches
2019-08-15 13:40 ` David Howells
2019-08-15 22:34 ` Nick Desaulniers
0 siblings, 2 replies; 9+ messages in thread
From: Joe Perches @ 2019-08-15 2:36 UTC (permalink / raw)
To: David Howells
Cc: Nathan Chancellor, Nick Desaulniers, Nathan Huckleberry,
linux-afs, linux-kernel
Make the code a bit easier for a script to appropriately convert
case statement blocks with /* fallthrough */ comments to a macro by
moving comments describing the next case block to the case statement.
Signed-off-by: Joe Perches <joe@perches.com>
---
fs/afs/cmservice.c | 10 +++-------
fs/afs/fsclient.c | 51 +++++++++++++++++----------------------------------
fs/afs/vlclient.c | 50 +++++++++++++++++++++++++-------------------------
fs/afs/yfsclient.c | 51 +++++++++++++++++----------------------------------
4 files changed, 62 insertions(+), 100 deletions(-)
diff --git a/fs/afs/cmservice.c b/fs/afs/cmservice.c
index b86195e4dc6c..2270fe9325da 100644
--- a/fs/afs/cmservice.c
+++ b/fs/afs/cmservice.c
@@ -282,10 +282,8 @@ static int afs_deliver_cb_callback(struct afs_call *call)
case 0:
afs_extract_to_tmp(call);
call->unmarshall++;
-
- /* extract the FID array and its count in two steps */
/* fall through */
- case 1:
+ case 1: /* extract the FID array and its count in two steps */
_debug("extract FID count");
ret = afs_extract_data(call, true);
if (ret < 0)
@@ -329,9 +327,8 @@ static int afs_deliver_cb_callback(struct afs_call *call)
afs_extract_to_tmp(call);
call->unmarshall++;
- /* extract the callback array and its count in two steps */
/* fall through */
- case 3:
+ case 3: /* extract the callback array & count in two steps */
_debug("extract CB count");
ret = afs_extract_data(call, true);
if (ret < 0)
@@ -651,9 +648,8 @@ static int afs_deliver_yfs_cb_callback(struct afs_call *call)
afs_extract_to_tmp(call);
call->unmarshall++;
- /* extract the FID array and its count in two steps */
/* Fall through */
- case 1:
+ case 1: /* extract the FID array and its count in two steps */
_debug("extract FID count");
ret = afs_extract_data(call, true);
if (ret < 0)
diff --git a/fs/afs/fsclient.c b/fs/afs/fsclient.c
index 114f281f3687..d9dc1bdfa695 100644
--- a/fs/afs/fsclient.c
+++ b/fs/afs/fsclient.c
@@ -341,8 +341,7 @@ static int afs_deliver_fs_fetch_data(struct afs_call *call)
}
/* Fall through */
- /* extract the returned data length */
- case 1:
+ case 1: /* extract the returned data length */
_debug("extract data length");
ret = afs_extract_data(call, true);
if (ret < 0)
@@ -369,8 +368,7 @@ static int afs_deliver_fs_fetch_data(struct afs_call *call)
ASSERTCMP(size, <=, PAGE_SIZE);
/* Fall through */
- /* extract the returned data */
- case 2:
+ case 2: /* extract the returned data */
_debug("extract data %zu/%llu",
iov_iter_count(&call->iter), req->remain);
@@ -411,8 +409,7 @@ static int afs_deliver_fs_fetch_data(struct afs_call *call)
afs_extract_to_buf(call, (21 + 3 + 6) * 4);
/* Fall through */
- /* extract the metadata */
- case 4:
+ case 4: /* extract the metadata */
ret = afs_extract_data(call, false);
if (ret < 0)
return ret;
@@ -1476,8 +1473,7 @@ static int afs_deliver_fs_get_volume_status(struct afs_call *call)
afs_extract_to_buf(call, 12 * 4);
/* Fall through */
- /* extract the returned status record */
- case 1:
+ case 1: /* extract the returned status record */
_debug("extract status");
ret = afs_extract_data(call, true);
if (ret < 0)
@@ -1489,8 +1485,7 @@ static int afs_deliver_fs_get_volume_status(struct afs_call *call)
afs_extract_to_tmp(call);
/* Fall through */
- /* extract the volume name length */
- case 2:
+ case 2: /* extract the volume name length */
ret = afs_extract_data(call, true);
if (ret < 0)
return ret;
@@ -1505,8 +1500,7 @@ static int afs_deliver_fs_get_volume_status(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* extract the volume name */
- case 3:
+ case 3: /* extract the volume name */
_debug("extract volname");
ret = afs_extract_data(call, true);
if (ret < 0)
@@ -1519,8 +1513,7 @@ static int afs_deliver_fs_get_volume_status(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* extract the offline message length */
- case 4:
+ case 4: /* extract the offline message length */
ret = afs_extract_data(call, true);
if (ret < 0)
return ret;
@@ -1535,8 +1528,7 @@ static int afs_deliver_fs_get_volume_status(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* extract the offline message */
- case 5:
+ case 5: /* extract the offline message */
_debug("extract offline");
ret = afs_extract_data(call, true);
if (ret < 0)
@@ -1550,8 +1542,7 @@ static int afs_deliver_fs_get_volume_status(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* extract the message of the day length */
- case 6:
+ case 6: /* extract the message of the day length */
ret = afs_extract_data(call, true);
if (ret < 0)
return ret;
@@ -1566,8 +1557,7 @@ static int afs_deliver_fs_get_volume_status(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* extract the message of the day */
- case 7:
+ case 7: /* extract the message of the day */
_debug("extract motd");
ret = afs_extract_data(call, false);
if (ret < 0)
@@ -1862,8 +1852,7 @@ static int afs_deliver_fs_get_capabilities(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* Extract the capabilities word count */
- case 1:
+ case 1: /* Extract the capabilities word count */
ret = afs_extract_data(call, true);
if (ret < 0)
return ret;
@@ -1876,8 +1865,7 @@ static int afs_deliver_fs_get_capabilities(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* Extract capabilities words */
- case 2:
+ case 2: /* Extract capabilities words */
ret = afs_extract_data(call, false);
if (ret < 0)
return ret;
@@ -2034,8 +2022,7 @@ static int afs_deliver_fs_inline_bulk_status(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* Extract the file status count and array in two steps */
- case 1:
+ case 1: /* Extract the file status count & array in two steps */
_debug("extract status count");
ret = afs_extract_data(call, true);
if (ret < 0)
@@ -2074,8 +2061,7 @@ static int afs_deliver_fs_inline_bulk_status(struct afs_call *call)
afs_extract_to_tmp(call);
/* Fall through */
- /* Extract the callback count and array in two steps */
- case 3:
+ case 3: /* Extract the callback count & array in two steps */
_debug("extract CB count");
ret = afs_extract_data(call, true);
if (ret < 0)
@@ -2207,8 +2193,7 @@ static int afs_deliver_fs_fetch_acl(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* extract the returned data length */
- case 1:
+ case 1: /* extract the returned data length */
ret = afs_extract_data(call, true);
if (ret < 0)
return ret;
@@ -2225,8 +2210,7 @@ static int afs_deliver_fs_fetch_acl(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* extract the returned data */
- case 2:
+ case 2: /* extract the returned data */
ret = afs_extract_data(call, true);
if (ret < 0)
return ret;
@@ -2235,8 +2219,7 @@ static int afs_deliver_fs_fetch_acl(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* extract the metadata */
- case 3:
+ case 3: /* extract the metadata */
ret = afs_extract_data(call, false);
if (ret < 0)
return ret;
diff --git a/fs/afs/vlclient.c b/fs/afs/vlclient.c
index cfb0ac4bd039..c0b327ff0005 100644
--- a/fs/afs/vlclient.c
+++ b/fs/afs/vlclient.c
@@ -193,10 +193,10 @@ static int afs_deliver_vl_get_addrs_u(struct afs_call *call)
sizeof(struct afs_uuid__xdr) + 3 * sizeof(__be32));
call->unmarshall++;
- /* Extract the returned uuid, uniquifier, nentries and
- * blkaddrs size */
/* Fall through */
- case 1:
+ case 1: /* Extract the returned uuid, uniquifier, nentries and
+ * blkaddrs size
+ */
ret = afs_extract_data(call, true);
if (ret < 0)
return ret;
@@ -220,8 +220,8 @@ static int afs_deliver_vl_get_addrs_u(struct afs_call *call)
count = min(call->count, 4U);
afs_extract_to_buf(call, count * sizeof(__be32));
- /* Fall through - and extract entries */
- case 2:
+ /* Fall through */
+ case 2: /* extract entries */
ret = afs_extract_data(call, call->count > 4);
if (ret < 0)
return ret;
@@ -323,8 +323,8 @@ static int afs_deliver_vl_get_capabilities(struct afs_call *call)
afs_extract_to_tmp(call);
call->unmarshall++;
- /* Fall through - and extract the capabilities word count */
- case 1:
+ /* Fall through */
+ case 1: /* extract the capabilities word count */
ret = afs_extract_data(call, true);
if (ret < 0)
return ret;
@@ -336,8 +336,8 @@ static int afs_deliver_vl_get_capabilities(struct afs_call *call)
call->unmarshall++;
afs_extract_discard(call, count * sizeof(__be32));
- /* Fall through - and extract capabilities words */
- case 2:
+ /* Fall through */
+ case 2: /* extract capabilities words */
ret = afs_extract_data(call, false);
if (ret < 0)
return ret;
@@ -431,12 +431,12 @@ static int afs_deliver_yfsvl_get_endpoints(struct afs_call *call)
case 0:
afs_extract_to_buf(call, sizeof(uuid_t) + 3 * sizeof(__be32));
call->unmarshall = 1;
-
- /* Extract the returned uuid, uniquifier, fsEndpoints count and
- * either the first fsEndpoint type or the volEndpoints
- * count if there are no fsEndpoints. */
/* Fall through */
- case 1:
+ case 1: /* Extract the returned uuid, uniquifier,
+ * fsEndpoints count and either the first fsEndpoint
+ * type or the volEndpoints count if there are no
+ * fsEndpoints.
+ */
ret = afs_extract_data(call, true);
if (ret < 0)
return ret;
@@ -476,8 +476,8 @@ static int afs_deliver_yfsvl_get_endpoints(struct afs_call *call)
afs_extract_to_buf(call, size);
call->unmarshall = 2;
- /* Fall through - and extract fsEndpoints[] entries */
- case 2:
+ /* Fall through */
+ case 2: /* extract fsEndpoints[] entries */
ret = afs_extract_data(call, true);
if (ret < 0)
return ret;
@@ -525,12 +525,12 @@ static int afs_deliver_yfsvl_get_endpoints(struct afs_call *call)
afs_extract_to_buf(call, 1 * sizeof(__be32));
call->unmarshall = 3;
- /* Extract the type of volEndpoints[0]. Normally we would
- * extract the type of the next endpoint when we extract the
- * data of the current one, but this is the first...
- */
/* Fall through */
- case 3:
+ case 3: /* Extract the type of volEndpoints[0].
+ * Normally we would extract the type of the
+ * next endpoint when we extract the data of
+ * the current one, but this is the first...
+ */
ret = afs_extract_data(call, true);
if (ret < 0)
return ret;
@@ -556,8 +556,8 @@ static int afs_deliver_yfsvl_get_endpoints(struct afs_call *call)
afs_extract_to_buf(call, size);
call->unmarshall = 4;
- /* Fall through - and extract volEndpoints[] entries */
- case 4:
+ /* Fall through */
+ case 4: /* extract volEndpoints[] entries */
ret = afs_extract_data(call, true);
if (ret < 0)
return ret;
@@ -592,8 +592,8 @@ static int afs_deliver_yfsvl_get_endpoints(struct afs_call *call)
afs_extract_discard(call, 0);
call->unmarshall = 5;
- /* Fall through - Done */
- case 5:
+ /* Fall through */
+ case 5: /* Done */
ret = afs_extract_data(call, false);
if (ret < 0)
return ret;
diff --git a/fs/afs/yfsclient.c b/fs/afs/yfsclient.c
index 2575503170fc..77cf878e6a25 100644
--- a/fs/afs/yfsclient.c
+++ b/fs/afs/yfsclient.c
@@ -452,8 +452,7 @@ static int yfs_deliver_fs_fetch_data64(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* extract the returned data length */
- case 1:
+ case 1: /* extract the returned data length */
_debug("extract data length");
ret = afs_extract_data(call, true);
if (ret < 0)
@@ -480,8 +479,7 @@ static int yfs_deliver_fs_fetch_data64(struct afs_call *call)
ASSERTCMP(size, <=, PAGE_SIZE);
/* Fall through */
- /* extract the returned data */
- case 2:
+ case 2: /* extract the returned data */
_debug("extract data %zu/%llu",
iov_iter_count(&call->iter), req->remain);
@@ -525,8 +523,7 @@ static int yfs_deliver_fs_fetch_data64(struct afs_call *call)
sizeof(struct yfs_xdr_YFSVolSync));
/* Fall through */
- /* extract the metadata */
- case 4:
+ case 4: /* extract the metadata */
ret = afs_extract_data(call, false);
if (ret < 0)
return ret;
@@ -1434,8 +1431,7 @@ static int yfs_deliver_fs_get_volume_status(struct afs_call *call)
afs_extract_to_buf(call, sizeof(struct yfs_xdr_YFSFetchVolumeStatus));
/* Fall through */
- /* extract the returned status record */
- case 1:
+ case 1: /* extract the returned status record */
_debug("extract status");
ret = afs_extract_data(call, true);
if (ret < 0)
@@ -1447,8 +1443,7 @@ static int yfs_deliver_fs_get_volume_status(struct afs_call *call)
afs_extract_to_tmp(call);
/* Fall through */
- /* extract the volume name length */
- case 2:
+ case 2: /* extract the volume name length */
ret = afs_extract_data(call, true);
if (ret < 0)
return ret;
@@ -1463,8 +1458,7 @@ static int yfs_deliver_fs_get_volume_status(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* extract the volume name */
- case 3:
+ case 3: /* extract the volume name */
_debug("extract volname");
ret = afs_extract_data(call, true);
if (ret < 0)
@@ -1477,8 +1471,7 @@ static int yfs_deliver_fs_get_volume_status(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* extract the offline message length */
- case 4:
+ case 4: /* extract the offline message length */
ret = afs_extract_data(call, true);
if (ret < 0)
return ret;
@@ -1493,8 +1486,7 @@ static int yfs_deliver_fs_get_volume_status(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* extract the offline message */
- case 5:
+ case 5: /* extract the offline message */
_debug("extract offline");
ret = afs_extract_data(call, true);
if (ret < 0)
@@ -1508,8 +1500,7 @@ static int yfs_deliver_fs_get_volume_status(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* extract the message of the day length */
- case 6:
+ case 6: /* extract the message of the day length */
ret = afs_extract_data(call, true);
if (ret < 0)
return ret;
@@ -1524,8 +1515,7 @@ static int yfs_deliver_fs_get_volume_status(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* extract the message of the day */
- case 7:
+ case 7: /* extract the message of the day */
_debug("extract motd");
ret = afs_extract_data(call, false);
if (ret < 0)
@@ -1817,8 +1807,7 @@ static int yfs_deliver_fs_inline_bulk_status(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* Extract the file status count and array in two steps */
- case 1:
+ case 1: /* Extract the file status count & array in two steps */
_debug("extract status count");
ret = afs_extract_data(call, true);
if (ret < 0)
@@ -1857,8 +1846,7 @@ static int yfs_deliver_fs_inline_bulk_status(struct afs_call *call)
afs_extract_to_tmp(call);
/* Fall through */
- /* Extract the callback count and array in two steps */
- case 3:
+ case 3: /* Extract the callback count & array in two steps */
_debug("extract CB count");
ret = afs_extract_data(call, true);
if (ret < 0)
@@ -1990,8 +1978,7 @@ static int yfs_deliver_fs_fetch_opaque_acl(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* Extract the file ACL length */
- case 1:
+ case 1: /* Extract the file ACL length */
ret = afs_extract_data(call, true);
if (ret < 0)
return ret;
@@ -2012,8 +1999,7 @@ static int yfs_deliver_fs_fetch_opaque_acl(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* Extract the file ACL */
- case 2:
+ case 2: /* Extract the file ACL */
ret = afs_extract_data(call, true);
if (ret < 0)
return ret;
@@ -2022,8 +2008,7 @@ static int yfs_deliver_fs_fetch_opaque_acl(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* Extract the volume ACL length */
- case 3:
+ case 3: /* Extract the volume ACL length */
ret = afs_extract_data(call, true);
if (ret < 0)
return ret;
@@ -2044,8 +2029,7 @@ static int yfs_deliver_fs_fetch_opaque_acl(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* Extract the volume ACL */
- case 4:
+ case 4: /* Extract the volume ACL */
ret = afs_extract_data(call, true);
if (ret < 0)
return ret;
@@ -2057,8 +2041,7 @@ static int yfs_deliver_fs_fetch_opaque_acl(struct afs_call *call)
call->unmarshall++;
/* Fall through */
- /* extract the metadata */
- case 5:
+ case 5: /* extract the metadata */
ret = afs_extract_data(call, false);
if (ret < 0)
return ret;
--
2.15.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] afs: Move comments after /* fallthrough */
2019-08-15 2:36 [PATCH] afs: Move comments after /* fallthrough */ Joe Perches
@ 2019-08-15 13:40 ` David Howells
2019-08-15 23:46 ` Joe Perches
2019-08-16 10:36 ` David Howells
2019-08-15 22:34 ` Nick Desaulniers
1 sibling, 2 replies; 9+ messages in thread
From: David Howells @ 2019-08-15 13:40 UTC (permalink / raw)
To: Joe Perches
Cc: dhowells, Nathan Chancellor, Nick Desaulniers,
Nathan Huckleberry, linux-afs, linux-kernel
Joe Perches <joe@perches.com> wrote:
> Make the code a bit easier for a script to appropriately convert
> case statement blocks with /* fallthrough */ comments to a macro by
> moving comments describing the next case block to the case statement.
This doesn't sound good. Can you give an illustration of what a resulting
case might look like? Say taking the following as an example:
> @@ -282,10 +282,8 @@ static int afs_deliver_cb_callback(struct afs_call *call)
> case 0:
> afs_extract_to_tmp(call);
> call->unmarshall++;
> -
> - /* extract the FID array and its count in two steps */
> /* fall through */
> - case 1:
> + case 1: /* extract the FID array and its count in two steps */
> _debug("extract FID count");
> ret = afs_extract_data(call, true);
> if (ret < 0)
David
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] afs: Move comments after /* fallthrough */
2019-08-15 2:36 [PATCH] afs: Move comments after /* fallthrough */ Joe Perches
2019-08-15 13:40 ` David Howells
@ 2019-08-15 22:34 ` Nick Desaulniers
2019-08-15 22:55 ` Joe Perches
1 sibling, 1 reply; 9+ messages in thread
From: Nick Desaulniers @ 2019-08-15 22:34 UTC (permalink / raw)
To: Joe Perches
Cc: David Howells, Nathan Chancellor, Nathan Huckleberry, linux-afs, LKML
On Wed, Aug 14, 2019 at 7:36 PM Joe Perches <joe@perches.com> wrote:
>
> Make the code a bit easier for a script to appropriately convert
> case statement blocks with /* fallthrough */ comments to a macro by
> moving comments describing the next case block to the case statement.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> fs/afs/cmservice.c | 10 +++-------
> fs/afs/fsclient.c | 51 +++++++++++++++++----------------------------------
> fs/afs/vlclient.c | 50 +++++++++++++++++++++++++-------------------------
> fs/afs/yfsclient.c | 51 +++++++++++++++++----------------------------------
So these changes are across just fs/afs, how many patches like this
would you need across the whole tree to solve this problem?
> 4 files changed, 62 insertions(+), 100 deletions(-)
>
> diff --git a/fs/afs/cmservice.c b/fs/afs/cmservice.c
> index b86195e4dc6c..2270fe9325da 100644
> --- a/fs/afs/cmservice.c
> +++ b/fs/afs/cmservice.c
> @@ -282,10 +282,8 @@ static int afs_deliver_cb_callback(struct afs_call *call)
> case 0:
> afs_extract_to_tmp(call);
> call->unmarshall++;
> -
> - /* extract the FID array and its count in two steps */
> /* fall through */
> - case 1:
> + case 1: /* extract the FID array and its count in two steps */
Could these instead be on their own line as the first line within this
case? (I don't feel particularly strongly about this).
...
> @@ -220,8 +220,8 @@ static int afs_deliver_vl_get_addrs_u(struct afs_call *call)
> count = min(call->count, 4U);
> afs_extract_to_buf(call, count * sizeof(__be32));
>
> - /* Fall through - and extract entries */
Yikes! Mixing fall through and other comments...yeah that would be
hard to globally find and replace.
> - case 2:
> + /* Fall through */
> + case 2: /* extract entries */
> ret = afs_extract_data(call, call->count > 4);
> if (ret < 0)
> return ret;
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] afs: Move comments after /* fallthrough */
2019-08-15 22:34 ` Nick Desaulniers
@ 2019-08-15 22:55 ` Joe Perches
0 siblings, 0 replies; 9+ messages in thread
From: Joe Perches @ 2019-08-15 22:55 UTC (permalink / raw)
To: Nick Desaulniers
Cc: David Howells, Nathan Chancellor, Nathan Huckleberry, linux-afs, LKML
On Thu, 2019-08-15 at 15:34 -0700, Nick Desaulniers wrote:
> On Wed, Aug 14, 2019 at 7:36 PM Joe Perches <joe@perches.com> wrote:
> > Make the code a bit easier for a script to appropriately convert
> > case statement blocks with /* fallthrough */ comments to a macro by
> > moving comments describing the next case block to the case statement.
> >
> > Signed-off-by: Joe Perches <joe@perches.com>
> > ---
> > fs/afs/cmservice.c | 10 +++-------
> > fs/afs/fsclient.c | 51 +++++++++++++++++----------------------------------
> > fs/afs/vlclient.c | 50 +++++++++++++++++++++++++-------------------------
> > fs/afs/yfsclient.c | 51 +++++++++++++++++----------------------------------
>
> So these changes are across just fs/afs, how many patches like this
> would you need across the whole tree to solve this problem?
No idea. I only looked at afs when Nathan Chancellor showed
there were 350 or so changes necessary in the kernel tree.
The afs entries were 50 of them so I just looked and saw why.
I haven't looked at all the others.
https://gist.github.com/nathanchance/ffbd71b48ba197837e1bdd9bb863b85f
But probably most of the others are missing a fallthrough to
a break like:
switch {foo} {
case 1:
<bar>;
default:
break;
}
where gcc does not emit a warning but clang apparently does.
I do think gcc should emit a warning here too so I filed a
gcc bugzilla entry.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91432
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] afs: Move comments after /* fallthrough */
2019-08-15 13:40 ` David Howells
@ 2019-08-15 23:46 ` Joe Perches
2019-08-16 10:36 ` David Howells
1 sibling, 0 replies; 9+ messages in thread
From: Joe Perches @ 2019-08-15 23:46 UTC (permalink / raw)
To: David Howells
Cc: Nathan Chancellor, Nick Desaulniers, Nathan Huckleberry,
linux-afs, linux-kernel
On Thu, 2019-08-15 at 14:40 +0100, David Howells wrote:
> Joe Perches <joe@perches.com> wrote:
>
> > Make the code a bit easier for a script to appropriately convert
> > case statement blocks with /* fallthrough */ comments to a macro by
> > moving comments describing the next case block to the case statement.
>
> This doesn't sound good. Can you give an illustration of what a resulting
> case might look like? Say taking the following as an example:
>
> > @@ -282,10 +282,8 @@ static int afs_deliver_cb_callback(struct afs_call *call)
> > case 0:
> > afs_extract_to_tmp(call);
> > call->unmarshall++;
> > -
> > - /* extract the FID array and its count in two steps */
> > /* fall through */
> > - case 1:
> > + case 1: /* extract the FID array and its count in two steps */
> > _debug("extract FID count");
> > ret = afs_extract_data(call, true);
> > if (ret < 0)
This case above would not change, but this case below would:
diff --git a/fs/afs/fsclient.c b/fs/afs/fsclient.c
index 114f281f3687..d9dc1bdfa695 100644
--- a/fs/afs/fsclient.c
+++ b/fs/afs/fsclient.c
@@ -341,8 +341,7 @@ static int afs_deliver_fs_fetch_data(struct afs_call *call)
}
/* Fall through */
- /* extract the returned data length */
- case 1:
+ case 1: /* extract the returned data length */
_debug("extract data length");
ret = afs_extract_data(call, true);
if (ret < 0)
Here the script would not convert the /* Fall through */
because the next non-blank line does not start with
case or default
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] afs: Move comments after /* fallthrough */
2019-08-15 13:40 ` David Howells
2019-08-15 23:46 ` Joe Perches
@ 2019-08-16 10:36 ` David Howells
2019-08-16 11:07 ` Joe Perches
2019-08-19 21:03 ` David Howells
1 sibling, 2 replies; 9+ messages in thread
From: David Howells @ 2019-08-16 10:36 UTC (permalink / raw)
To: Joe Perches
Cc: dhowells, Nathan Chancellor, Nick Desaulniers,
Nathan Huckleberry, linux-afs, linux-kernel
Joe Perches <joe@perches.com> wrote:
> Here the script would not convert the /* Fall through */
> because the next non-blank line does not start with
> case or default
Convert the "/* Fall through */" to what?
You said "for a script to appropriately convert case statement blocks with /*
fallthrough */ comments to a macro". Can you give an example of what the code
would look like with this macro emplaced?
David
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] afs: Move comments after /* fallthrough */
2019-08-16 10:36 ` David Howells
@ 2019-08-16 11:07 ` Joe Perches
2019-08-19 21:03 ` David Howells
1 sibling, 0 replies; 9+ messages in thread
From: Joe Perches @ 2019-08-16 11:07 UTC (permalink / raw)
To: David Howells
Cc: Nathan Chancellor, Nick Desaulniers, Nathan Huckleberry,
linux-afs, linux-kernel
On Fri, 2019-08-16 at 11:36 +0100, David Howells wrote:
> Joe Perches <joe@perches.com> wrote:
>
> > Here the script would not convert the /* Fall through */
> > because the next non-blank line does not start with
> > case or default
>
> Convert the "/* Fall through */" to what?
>
> You said "for a script to appropriately convert case statement blocks with /*
> fallthrough */ comments to a macro". Can you give an example of what the code
> would look like with this macro emplaced?
Sure.
The basic idea is to use a macro for __attribute__((__fallthrough__))
like:
#define fallthrough __attribute__((__fallthrough__))
though some would prefer __fallthrough to be more similar to
other attribute macros. I prefer fallthrough; to be more
similar to break;
So it would end up like (just an example, won't apply)
diff --git a/fs/afs/cmservice.c b/fs/afs/cmservice.c
index b86195e4dc6c..d962332008aa 100644
--- a/fs/afs/cmservice.c
+++ b/fs/afs/cmservice.c
@@ -284,7 +284,7 @@ static int afs_deliver_cb_callback(struct afs_call *call)
call->unmarshall++;
/* extract the FID array and its count in two steps */
- /* fall through */
+ fallthrough;
case 1:
_debug("extract FID count");
ret = afs_extract_data(call, true);
@@ -303,8 +303,7 @@ static int afs_deliver_cb_callback(struct afs_call *call)
return -ENOMEM;
afs_extract_to_buf(call, call->count * 3 * 4);
call->unmarshall++;
-
- /* Fall through */
+ fallthrough;
case 2:
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] afs: Move comments after /* fallthrough */
2019-08-16 10:36 ` David Howells
2019-08-16 11:07 ` Joe Perches
@ 2019-08-19 21:03 ` David Howells
2019-08-19 21:50 ` Joe Perches
1 sibling, 1 reply; 9+ messages in thread
From: David Howells @ 2019-08-19 21:03 UTC (permalink / raw)
To: Joe Perches
Cc: dhowells, Nathan Chancellor, Nick Desaulniers,
Nathan Huckleberry, linux-afs, linux-kernel
Joe Perches <joe@perches.com> wrote:
> /* extract the FID array and its count in two steps */
> - /* fall through */
> + fallthrough;
> case 1:
Okay, that doesn't look too bad. I thought you might be going to combine it
with the case inside a macro in some way.
David
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] afs: Move comments after /* fallthrough */
2019-08-19 21:03 ` David Howells
@ 2019-08-19 21:50 ` Joe Perches
0 siblings, 0 replies; 9+ messages in thread
From: Joe Perches @ 2019-08-19 21:50 UTC (permalink / raw)
To: David Howells
Cc: Nathan Chancellor, Nick Desaulniers, Nathan Huckleberry,
linux-afs, linux-kernel
On Mon, 2019-08-19 at 22:03 +0100, David Howells wrote:
> Joe Perches <joe@perches.com> wrote:
>
> > /* extract the FID array and its count in two steps */
> > - /* fall through */
> > + fallthrough;
> > case 1:
>
> Okay, that doesn't look too bad. I thought you might be going to combine it
> with the case inside a macro in some way.
Does that mean you will apply this?
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-08-19 21:50 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-15 2:36 [PATCH] afs: Move comments after /* fallthrough */ Joe Perches
2019-08-15 13:40 ` David Howells
2019-08-15 23:46 ` Joe Perches
2019-08-16 10:36 ` David Howells
2019-08-16 11:07 ` Joe Perches
2019-08-19 21:03 ` David Howells
2019-08-19 21:50 ` Joe Perches
2019-08-15 22:34 ` Nick Desaulniers
2019-08-15 22:55 ` Joe Perches
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 NNTP newsgroup(s).