diff mbox

selinux: Fix bool initialization/comparison

Message ID 1507383097088-1068794175-19-diffsplit-thomas@m3y3r.de (mailing list archive)
State New, archived
Headers show

Commit Message

Thomas Meyer Oct. 7, 2017, 2:02 p.m. UTC
Bool initializations should use true and false. Bool tests don't need
comparisons.

Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
---

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Casey Schaufler Oct. 10, 2017, 11:17 p.m. UTC | #1
On 10/7/2017 7:02 AM, Thomas Meyer wrote:
> Bool initializations should use true and false. Bool tests don't need
> comparisons.
>
> Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
> ---
>
> diff -u -p a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
> --- a/security/selinux/ss/mls.c
> +++ b/security/selinux/ss/mls.c
> @@ -550,7 +550,7 @@ int mls_compute_sid(struct context *scon
>  
>  		/* Fallthrough */
>  	case AVTAB_CHANGE:
> -		if ((tclass == policydb.process_class) || (sock == true))
> +		if ((tclass == policydb.process_class) || (sock))

bool tests don't need parentheses, either.

	+	if ((tclass == policydb.process_class) || sock)

>  			/* Use the process MLS attributes. */
>  			return mls_context_cpy(newcontext, scontext);
>  		else
> diff -u -p a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -1656,7 +1656,7 @@ static int security_compute_sid(u32 ssid
>  	} else if (cladatum && cladatum->default_role == DEFAULT_TARGET) {
>  		newcontext.role = tcontext->role;
>  	} else {
> -		if ((tclass == policydb.process_class) || (sock == true))
> +		if ((tclass == policydb.process_class) || (sock))

Likewise

>  			newcontext.role = scontext->role;
>  		else
>  			newcontext.role = OBJECT_R_VAL;
> @@ -1668,7 +1668,7 @@ static int security_compute_sid(u32 ssid
>  	} else if (cladatum && cladatum->default_type == DEFAULT_TARGET) {
>  		newcontext.type = tcontext->type;
>  	} else {
> -		if ((tclass == policydb.process_class) || (sock == true)) {
> +		if ((tclass == policydb.process_class) || (sock)) {

On more time.

>  			/* Use the type of process. */
>  			newcontext.type = scontext->type;
>  		} else {
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Casey Schaufler Oct. 10, 2017, 11:19 p.m. UTC | #2
On 10/7/2017 7:02 AM, Thomas Meyer wrote:
> Bool initializations should use true and false. Bool tests don't need
> comparisons.
>
> Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
> ---
>
> diff -u -p a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
> --- a/security/selinux/ss/mls.c
> +++ b/security/selinux/ss/mls.c
> @@ -550,7 +550,7 @@ int mls_compute_sid(struct context *scon
>  
>  		/* Fallthrough */
>  	case AVTAB_CHANGE:
> -		if ((tclass == policydb.process_class) || (sock == true))
> +		if ((tclass == policydb.process_class) || (sock))

How about

	+	if ((tclass == policydb.process_class) || sock)

>  			/* Use the process MLS attributes. */
>  			return mls_context_cpy(newcontext, scontext);
>  		else
> diff -u -p a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -1656,7 +1656,7 @@ static int security_compute_sid(u32 ssid
>  	} else if (cladatum && cladatum->default_role == DEFAULT_TARGET) {
>  		newcontext.role = tcontext->role;
>  	} else {
> -		if ((tclass == policydb.process_class) || (sock == true))
> +		if ((tclass == policydb.process_class) || (sock))

Excessive ()

>  			newcontext.role = scontext->role;
>  		else
>  			newcontext.role = OBJECT_R_VAL;
> @@ -1668,7 +1668,7 @@ static int security_compute_sid(u32 ssid
>  	} else if (cladatum && cladatum->default_type == DEFAULT_TARGET) {
>  		newcontext.type = tcontext->type;
>  	} else {
> -		if ((tclass == policydb.process_class) || (sock == true)) {
> +		if ((tclass == policydb.process_class) || (sock)) {

Excessive ()

>  			/* Use the type of process. */
>  			newcontext.type = scontext->type;
>  		} else {
>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Moore Oct. 13, 2017, 10:40 p.m. UTC | #3
On Tue, Oct 10, 2017 at 7:19 PM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 10/7/2017 7:02 AM, Thomas Meyer wrote:
>> Bool initializations should use true and false. Bool tests don't need
>> comparisons.
>>
>> Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
>> ---

Hi Thomas,

Thank you for submitting this patch, but it really doesn't add any
value to the SELinux code so I'm not going to merge it at this time.
If you would like to contribute to SELinux there are a number of
issues open on our GitHub page that you could look into resolving,
those patches would be very welcome.

* https://github.com/SELinuxProject

-Paul

>> diff -u -p a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
>> --- a/security/selinux/ss/mls.c
>> +++ b/security/selinux/ss/mls.c
>> @@ -550,7 +550,7 @@ int mls_compute_sid(struct context *scon
>>
>>               /* Fallthrough */
>>       case AVTAB_CHANGE:
>> -             if ((tclass == policydb.process_class) || (sock == true))
>> +             if ((tclass == policydb.process_class) || (sock))
>
> How about
>
>         +       if ((tclass == policydb.process_class) || sock)
>
>>                       /* Use the process MLS attributes. */
>>                       return mls_context_cpy(newcontext, scontext);
>>               else
>> diff -u -p a/security/selinux/ss/services.c b/security/selinux/ss/services.c
>> --- a/security/selinux/ss/services.c
>> +++ b/security/selinux/ss/services.c
>> @@ -1656,7 +1656,7 @@ static int security_compute_sid(u32 ssid
>>       } else if (cladatum && cladatum->default_role == DEFAULT_TARGET) {
>>               newcontext.role = tcontext->role;
>>       } else {
>> -             if ((tclass == policydb.process_class) || (sock == true))
>> +             if ((tclass == policydb.process_class) || (sock))
>
> Excessive ()
>
>>                       newcontext.role = scontext->role;
>>               else
>>                       newcontext.role = OBJECT_R_VAL;
>> @@ -1668,7 +1668,7 @@ static int security_compute_sid(u32 ssid
>>       } else if (cladatum && cladatum->default_type == DEFAULT_TARGET) {
>>               newcontext.type = tcontext->type;
>>       } else {
>> -             if ((tclass == policydb.process_class) || (sock == true)) {
>> +             if ((tclass == policydb.process_class) || (sock)) {
>
> Excessive ()
>
>>                       /* Use the type of process. */
>>                       newcontext.type = scontext->type;
>>               } else {
>>
>
diff mbox

Patch

diff -u -p a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
--- a/security/selinux/ss/mls.c
+++ b/security/selinux/ss/mls.c
@@ -550,7 +550,7 @@  int mls_compute_sid(struct context *scon
 
 		/* Fallthrough */
 	case AVTAB_CHANGE:
-		if ((tclass == policydb.process_class) || (sock == true))
+		if ((tclass == policydb.process_class) || (sock))
 			/* Use the process MLS attributes. */
 			return mls_context_cpy(newcontext, scontext);
 		else
diff -u -p a/security/selinux/ss/services.c b/security/selinux/ss/services.c
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -1656,7 +1656,7 @@  static int security_compute_sid(u32 ssid
 	} else if (cladatum && cladatum->default_role == DEFAULT_TARGET) {
 		newcontext.role = tcontext->role;
 	} else {
-		if ((tclass == policydb.process_class) || (sock == true))
+		if ((tclass == policydb.process_class) || (sock))
 			newcontext.role = scontext->role;
 		else
 			newcontext.role = OBJECT_R_VAL;
@@ -1668,7 +1668,7 @@  static int security_compute_sid(u32 ssid
 	} else if (cladatum && cladatum->default_type == DEFAULT_TARGET) {
 		newcontext.type = tcontext->type;
 	} else {
-		if ((tclass == policydb.process_class) || (sock == true)) {
+		if ((tclass == policydb.process_class) || (sock)) {
 			/* Use the type of process. */
 			newcontext.type = scontext->type;
 		} else {