| Message ID | 20191022020228.14117-1-marcos.souza.org@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | btrfs-progs: Setting implicit-fallthrough by default | expand |
On Mon, Oct 21, 2019 at 11:02:26PM -0300, Marcos Paulo de Souza wrote: > From: Marcos Paulo de Souza <mpdesouza@suse.com> > > While compiling btrfs-progs using clang I found an issue using > __attribute__(fallthrough), which does not seems to work in clang. > > To solve this issue, the code was changed to use /* fallthrough */, which is the > same notation adopted by linux kernel. I'd suggest to follow what kernel does, IIRC there's some whole-tree cleanup of all the fall through statements with a unified conversion to the right(tm) annotation. > Once these places were changed, -Wimplicit-fallthrough was set in Makefile, to > avoid further implicit-fallthrough cases being added in the future. That would be good to add by default, but I'm a bit worried about the differences between compilers and that we'd have to switch the annotations again once the attribute support lands. Maybe that's not a big deal.
On 22.10.19 г. 5:02 ч., Marcos Paulo de Souza wrote: > From: Marcos Paulo de Souza <mpdesouza@suse.com> > > While compiling btrfs-progs using clang I found an issue using > __attribute__(fallthrough), which does not seems to work in clang. > > To solve this issue, the code was changed to use /* fallthrough */, which is the > same notation adopted by linux kernel. > > Once these places were changed, -Wimplicit-fallthrough was set in Makefile, to > avoid further implicit-fallthrough cases being added in the future. > > Marcos Paulo de Souza (2): > btrfs-progs: utils: Replace __attribute__(fallthrough) > btrfs-progs: Makefile: Add -Wimplicit-fallthrough > > Makefile | 1 + > common/utils.c | 12 ++++++------ > 2 files changed, 7 insertions(+), 6 deletions(-) > Overall the patch looks good, it just changes the fallthrough to the least common denominator which seems to be a simple comment. In clang 10 the currently used attribute method is also going to be supported. But we'll get most value if we just enable it now, so Reviewed-by: Nikolay Borisov <nborisov@suse.com>
On Tue, Oct 22, 2019 at 03:45:38PM +0300, Nikolay Borisov wrote: > > > On 22.10.19 г. 5:02 ч., Marcos Paulo de Souza wrote: > > From: Marcos Paulo de Souza <mpdesouza@suse.com> > > > > While compiling btrfs-progs using clang I found an issue using > > __attribute__(fallthrough), which does not seems to work in clang. > > > > To solve this issue, the code was changed to use /* fallthrough */, which is the > > same notation adopted by linux kernel. > > > > Once these places were changed, -Wimplicit-fallthrough was set in Makefile, to > > avoid further implicit-fallthrough cases being added in the future. > > > > Marcos Paulo de Souza (2): > > btrfs-progs: utils: Replace __attribute__(fallthrough) > > btrfs-progs: Makefile: Add -Wimplicit-fallthrough > > > > Makefile | 1 + > > common/utils.c | 12 ++++++------ > > 2 files changed, 7 insertions(+), 6 deletions(-) > > > > Overall the patch looks good, it just changes the fallthrough to the > least common denominator which seems to be a simple comment. In clang 10 > the currently used attribute method is also going to be supported. > > But we'll get most value if we just enable it now, so > > Reviewed-by: Nikolay Borisov <nborisov@suse.com> Agreed, I've added the note to the first patch. 1-2 in devel. Thanks.
From: Marcos Paulo de Souza <mpdesouza@suse.com> While compiling btrfs-progs using clang I found an issue using __attribute__(fallthrough), which does not seems to work in clang. To solve this issue, the code was changed to use /* fallthrough */, which is the same notation adopted by linux kernel. Once these places were changed, -Wimplicit-fallthrough was set in Makefile, to avoid further implicit-fallthrough cases being added in the future. Marcos Paulo de Souza (2): btrfs-progs: utils: Replace __attribute__(fallthrough) btrfs-progs: Makefile: Add -Wimplicit-fallthrough Makefile | 1 + common/utils.c | 12 ++++++------ 2 files changed, 7 insertions(+), 6 deletions(-)