| Message ID | 20241111-apparmor-fix-label-declaration-warning-v1-1-adb64ab6482b@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | apparmor: Add empty statement between label and declaration in profile_transition(() | expand |
On 11/11/24 06:49, Nathan Chancellor wrote: > Clang 18 and newer warns (or errors with CONFIG_WERROR=y): > > security/apparmor/domain.c:695:3: error: label followed by a declaration is a C23 extension [-Werror,-Wc23-extensions] > 695 | struct aa_profile *new_profile = NULL; > | ^ > > With Clang 17 and older, this is just an unconditional hard error: > > security/apparmor/domain.c:695:3: error: expected expression > 695 | struct aa_profile *new_profile = NULL; > | ^ > security/apparmor/domain.c:697:3: error: use of undeclared identifier 'new_profile' > 697 | new_profile = aa_new_learning_profile(profile, false, name, > | ^ > security/apparmor/domain.c:699:8: error: use of undeclared identifier 'new_profile' > 699 | if (!new_profile) { > | ^ > security/apparmor/domain.c:704:11: error: use of undeclared identifier 'new_profile' > 704 | new = &new_profile->label; > | ^ > > Add a semicolon directly after the label to create an empty statement, > which keeps the original intent of the code while clearing up the > warning/error on all clang versions. > > Fixes: ee650b3820f3 ("apparmor: properly handle cx/px lookup failure for complain") > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202411101808.AI8YG6cs-lkp@intel.com/ > Signed-off-by: Nathan Chancellor <nathan@kernel.org> thanks for the patch, but I went with an alternate version, that I did last Sunday (sorry I guess I forgot to push the tree). Since I hadn't pushed the tree I did consider replacing my patch with it but in the end decided to not go with the C99 variable declaration, moving the var to the top of the outer block (what my Sunday patch did). The reason being that while I don't think the style guideline forbid them, I end up getting patches for them anyways, as some compiler flag sets will warn about them. > --- > security/apparmor/domain.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c > index 602d7a1bb44823a9b81e34d270b03c5f3aff3a34..eb0f222aa29442686b0a6751001c879f5b366c59 100644 > --- a/security/apparmor/domain.c > +++ b/security/apparmor/domain.c > @@ -691,7 +691,7 @@ static struct aa_label *profile_transition(const struct cred *subj_cred, > error = -EACCES; > } > } else if (COMPLAIN_MODE(profile)) { > -create_learning_profile: > +create_learning_profile:; > /* no exec permission - learning mode */ > struct aa_profile *new_profile = NULL; > > > --- > base-commit: 8c4f7960ae8a7a03a43f814e4af471b8e6ea3391 > change-id: 20241111-apparmor-fix-label-declaration-warning-fcd24ce2d447 > > Best regards,
On Fri, Nov 15, 2024 at 06:17:11PM -0800, John Johansen wrote: > thanks for the patch, but I went with an alternate version, that I did last Sunday > (sorry I guess I forgot to push the tree). Since I hadn't pushed the tree I did > consider replacing my patch with it but in the end decided to not go with > the C99 variable declaration, moving the var to the top of the outer block (what > my Sunday patch did). Thanks for the information, I considered doing the same thing but went with the minimal fix just for ease of acceptance. Appreciate you getting this resolved now. > The reason being that while I don't think the style guideline forbid them, I end > up getting patches for them anyways, as some compiler flag sets will warn about > them. For what it's worth, commit b5ec6fd286df ("kbuild: Drop -Wdeclaration-after-statement") dropped the option that warns about that, so you should not have to worry about that anymore. Cheers, Nathan
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 602d7a1bb44823a9b81e34d270b03c5f3aff3a34..eb0f222aa29442686b0a6751001c879f5b366c59 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -691,7 +691,7 @@ static struct aa_label *profile_transition(const struct cred *subj_cred, error = -EACCES; } } else if (COMPLAIN_MODE(profile)) { -create_learning_profile: +create_learning_profile:; /* no exec permission - learning mode */ struct aa_profile *new_profile = NULL;
Clang 18 and newer warns (or errors with CONFIG_WERROR=y): security/apparmor/domain.c:695:3: error: label followed by a declaration is a C23 extension [-Werror,-Wc23-extensions] 695 | struct aa_profile *new_profile = NULL; | ^ With Clang 17 and older, this is just an unconditional hard error: security/apparmor/domain.c:695:3: error: expected expression 695 | struct aa_profile *new_profile = NULL; | ^ security/apparmor/domain.c:697:3: error: use of undeclared identifier 'new_profile' 697 | new_profile = aa_new_learning_profile(profile, false, name, | ^ security/apparmor/domain.c:699:8: error: use of undeclared identifier 'new_profile' 699 | if (!new_profile) { | ^ security/apparmor/domain.c:704:11: error: use of undeclared identifier 'new_profile' 704 | new = &new_profile->label; | ^ Add a semicolon directly after the label to create an empty statement, which keeps the original intent of the code while clearing up the warning/error on all clang versions. Fixes: ee650b3820f3 ("apparmor: properly handle cx/px lookup failure for complain") Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202411101808.AI8YG6cs-lkp@intel.com/ Signed-off-by: Nathan Chancellor <nathan@kernel.org> --- security/apparmor/domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- base-commit: 8c4f7960ae8a7a03a43f814e4af471b8e6ea3391 change-id: 20241111-apparmor-fix-label-declaration-warning-fcd24ce2d447 Best regards,