| Message ID | 20190325081115.19874-1-omosnace@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | selinux: remove useless assignments | expand |
On Mon, Mar 25, 2019 at 4:11 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > The code incorrectly assigned directly to the variables instead of the > values they point to. Since the values are already set to NULL/0 at the > beginning of the function, we can simply remove these useless > assignments. > > Reported-by: Hariprasad Kelam <hariprasad.kelam@gmail.com> > Fixes: fede148324c3 ("selinux: log invalid contexts in AVCs") > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > --- > security/selinux/ss/services.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c > index ec62918521b1..b18a8d7c1b5e 100644 > --- a/security/selinux/ss/services.c > +++ b/security/selinux/ss/services.c > @@ -1318,14 +1318,11 @@ static int security_sid_to_context_core(struct selinux_state *state, > rc = -EINVAL; > goto out_unlock; > } > - if (only_invalid && !context->len) { > - scontext = NULL; > - scontext_len = 0; > - rc = 0; > - } else { > + if (only_invalid && !context->len) > + rc = 0; /* *scontext/*scontext_len are already set to NULL/0 */ The compiler doesn't like that you've used "/*" inside a comment. I'm surprised you didn't see this when compiling the code ... you did compile this before sending it to the list, right? Anyway, the patch looks fine to me otherwise so I removed the comment (it was a arguably verbose anyway) and merged into selinux/next. > + else > rc = context_struct_to_string(policydb, context, scontext, > scontext_len); > - } > out_unlock: > read_unlock(&state->ss->policy_rwlock); > out:
On Mon, Mar 25, 2019 at 3:31 PM Paul Moore <paul@paul-moore.com> wrote: > On Mon, Mar 25, 2019 at 4:11 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > The code incorrectly assigned directly to the variables instead of the > > values they point to. Since the values are already set to NULL/0 at the > > beginning of the function, we can simply remove these useless > > assignments. > > > > Reported-by: Hariprasad Kelam <hariprasad.kelam@gmail.com> > > Fixes: fede148324c3 ("selinux: log invalid contexts in AVCs") > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > > --- > > security/selinux/ss/services.c | 9 +++------ > > 1 file changed, 3 insertions(+), 6 deletions(-) > > > > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c > > index ec62918521b1..b18a8d7c1b5e 100644 > > --- a/security/selinux/ss/services.c > > +++ b/security/selinux/ss/services.c > > @@ -1318,14 +1318,11 @@ static int security_sid_to_context_core(struct selinux_state *state, > > rc = -EINVAL; > > goto out_unlock; > > } > > - if (only_invalid && !context->len) { > > - scontext = NULL; > > - scontext_len = 0; > > - rc = 0; > > - } else { > > + if (only_invalid && !context->len) > > + rc = 0; /* *scontext/*scontext_len are already set to NULL/0 */ > > The compiler doesn't like that you've used "/*" inside a comment. I'm > surprised you didn't see this when compiling the code ... you did > compile this before sending it to the list, right? Hm, could be that I had to rebuild (almost) the whole tree for some reason (git checkout across branches, probably) and the warning got lost in the sea of build output (no -Werror here, so the build did not fail). > > Anyway, the patch looks fine to me otherwise so I removed the comment > (it was a arguably verbose anyway) and merged into selinux/next. Fine by me, thanks! > > > + else > > rc = context_struct_to_string(policydb, context, scontext, > > scontext_len); > > - } > > out_unlock: > > read_unlock(&state->ss->policy_rwlock); > > out: > > -- > paul moore > www.paul-moore.com -- Ondrej Mosnacek <omosnace at redhat dot com> Software Engineer, Security Technologies Red Hat, Inc.
On 3/25/2019 1:11 AM, Ondrej Mosnacek wrote: > The code incorrectly assigned directly to the variables instead of the > values they point to. Since the values are already set to NULL/0 at the > beginning of the function, we can simply remove these useless > assignments. > > Reported-by: Hariprasad Kelam <hariprasad.kelam@gmail.com> > Fixes: fede148324c3 ("selinux: log invalid contexts in AVCs") > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > --- > security/selinux/ss/services.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c > index ec62918521b1..b18a8d7c1b5e 100644 > --- a/security/selinux/ss/services.c > +++ b/security/selinux/ss/services.c > @@ -1318,14 +1318,11 @@ static int security_sid_to_context_core(struct selinux_state *state, > rc = -EINVAL; > goto out_unlock; > } > - if (only_invalid && !context->len) { > - scontext = NULL; > - scontext_len = 0; > - rc = 0; > - } else { > + if (only_invalid && !context->len) > + rc = 0; /* *scontext/*scontext_len are already set to NULL/0 */ I know that modern compilers won't be fooled, but please don't nest comments. > + else > rc = context_struct_to_string(policydb, context, scontext, > scontext_len); > - } > out_unlock: > read_unlock(&state->ss->policy_rwlock); > out:
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index ec62918521b1..b18a8d7c1b5e 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -1318,14 +1318,11 @@ static int security_sid_to_context_core(struct selinux_state *state, rc = -EINVAL; goto out_unlock; } - if (only_invalid && !context->len) { - scontext = NULL; - scontext_len = 0; - rc = 0; - } else { + if (only_invalid && !context->len) + rc = 0; /* *scontext/*scontext_len are already set to NULL/0 */ + else rc = context_struct_to_string(policydb, context, scontext, scontext_len); - } out_unlock: read_unlock(&state->ss->policy_rwlock); out:
The code incorrectly assigned directly to the variables instead of the values they point to. Since the values are already set to NULL/0 at the beginning of the function, we can simply remove these useless assignments. Reported-by: Hariprasad Kelam <hariprasad.kelam@gmail.com> Fixes: fede148324c3 ("selinux: log invalid contexts in AVCs") Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> --- security/selinux/ss/services.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)