* [PATCH] selinux: remove useless assignments
@ 2019-03-25 8:11 Ondrej Mosnacek
2019-03-25 14:31 ` Paul Moore
2019-03-25 15:45 ` Casey Schaufler
0 siblings, 2 replies; 4+ messages in thread
From: Ondrej Mosnacek @ 2019-03-25 8:11 UTC (permalink / raw)
To: selinux, Paul Moore; +Cc: Stephen Smalley, Hariprasad Kelam
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 */
+ else
rc = context_struct_to_string(policydb, context, scontext,
scontext_len);
- }
out_unlock:
read_unlock(&state->ss->policy_rwlock);
out:
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] selinux: remove useless assignments
2019-03-25 8:11 [PATCH] selinux: remove useless assignments Ondrej Mosnacek
@ 2019-03-25 14:31 ` Paul Moore
2019-03-25 15:00 ` Ondrej Mosnacek
2019-03-25 15:45 ` Casey Schaufler
1 sibling, 1 reply; 4+ messages in thread
From: Paul Moore @ 2019-03-25 14:31 UTC (permalink / raw)
To: Ondrej Mosnacek, Hariprasad Kelam; +Cc: selinux, Stephen Smalley
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:
--
paul moore
www.paul-moore.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] selinux: remove useless assignments
2019-03-25 14:31 ` Paul Moore
@ 2019-03-25 15:00 ` Ondrej Mosnacek
0 siblings, 0 replies; 4+ messages in thread
From: Ondrej Mosnacek @ 2019-03-25 15:00 UTC (permalink / raw)
To: Paul Moore; +Cc: Hariprasad Kelam, selinux, Stephen Smalley
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] selinux: remove useless assignments
2019-03-25 8:11 [PATCH] selinux: remove useless assignments Ondrej Mosnacek
2019-03-25 14:31 ` Paul Moore
@ 2019-03-25 15:45 ` Casey Schaufler
1 sibling, 0 replies; 4+ messages in thread
From: Casey Schaufler @ 2019-03-25 15:45 UTC (permalink / raw)
To: Ondrej Mosnacek, selinux, Paul Moore; +Cc: Stephen Smalley, Hariprasad Kelam
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:
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-03-25 15:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-25 8:11 [PATCH] selinux: remove useless assignments Ondrej Mosnacek
2019-03-25 14:31 ` Paul Moore
2019-03-25 15:00 ` Ondrej Mosnacek
2019-03-25 15:45 ` Casey Schaufler
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).