selinux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] libselinux: refactor selinux_getenforcemode
@ 2025-07-27 15:32 Rahul Sandhu
  2025-07-29 14:33 ` Stephen Smalley
  0 siblings, 1 reply; 6+ messages in thread
From: Rahul Sandhu @ 2025-07-27 15:32 UTC (permalink / raw)
  To: selinux; +Cc: Rahul Sandhu

Invert the check for cfg being a nullptr and early return.

Signed-off-by: Rahul Sandhu <nvraxn@gmail.com>
---
 libselinux/src/selinux_config.c | 75 ++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 38 deletions(-)

diff --git a/libselinux/src/selinux_config.c b/libselinux/src/selinux_config.c
index 75db14ba..a2335fa9 100644
--- a/libselinux/src/selinux_config.c
+++ b/libselinux/src/selinux_config.c
@@ -88,47 +88,46 @@ static const uint16_t file_path_suffixes_idx[NEL] = {
 
 int selinux_getenforcemode(int *enforce)
 {
-	int ret = -1;
 	FILE *cfg = fopen(SELINUXCONFIG, "re");
-	if (cfg) {
-		char *buf;
-		char *tag;
-		int len = sizeof(SELINUXTAG) - 1;
-		buf = malloc(selinux_page_size);
-		if (!buf) {
-			fclose(cfg);
-			return -1;
-		}
-		while (fgets_unlocked(buf, selinux_page_size, cfg)) {
-			if (strncmp(buf, SELINUXTAG, len))
-				continue;
-			tag = buf+len;
-			while (isspace((unsigned char)*tag))
-				tag++;
-			if (!strncasecmp
-			    (tag, "enforcing", sizeof("enforcing") - 1)) {
-				*enforce = 1;
-				ret = 0;
-				break;
-			} else
-			    if (!strncasecmp
-				(tag, "permissive",
-				 sizeof("permissive") - 1)) {
-				*enforce = 0;
-				ret = 0;
-				break;
-			} else
-			    if (!strncasecmp
-				(tag, "disabled",
-				 sizeof("disabled") - 1)) {
-				*enforce = -1;
-				ret = 0;
-				break;
-			}
-		}
+	if (!cfg)
+		return -1;
+
+	char *buf = malloc(selinux_page_size);
+	if (!buf) {
 		fclose(cfg);
-		free(buf);
+		return -1;
 	}
+
+	int ret = -1;
+	static const int len = sizeof(SELINUXTAG) - 1;
+	while (fgets_unlocked(buf, selinux_page_size, cfg)) {
+		if (strncmp(buf, SELINUXTAG, len))
+			continue;
+
+		char *tag = buf + len;
+		while (isspace((unsigned char)*tag))
+			tag++;
+
+		if (!strncasecmp(tag, "enforcing", sizeof("enforcing") - 1)) {
+			*enforce = 1;
+			ret = 0;
+			break;
+		} else if (!strncasecmp(tag, "permissive",
+					sizeof("permissive") - 1)) {
+			*enforce = 0;
+			ret = 0;
+			break;
+		} else if (!strncasecmp(tag, "disabled",
+					sizeof("disabled") - 1)) {
+			*enforce = -1;
+			ret = 0;
+			break;
+		}
+	}
+
+	fclose(cfg);
+	free(buf);
+
 	return ret;
 }
 
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] libselinux: refactor selinux_getenforcemode
  2025-07-27 15:32 [PATCH] libselinux: refactor selinux_getenforcemode Rahul Sandhu
@ 2025-07-29 14:33 ` Stephen Smalley
  2025-07-29 17:07   ` Christian Göttsche
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Smalley @ 2025-07-29 14:33 UTC (permalink / raw)
  To: Rahul Sandhu; +Cc: selinux

On Sun, Jul 27, 2025 at 11:33 AM Rahul Sandhu <nvraxn@gmail.com> wrote:
>
> Invert the check for cfg being a nullptr and early return.
>
> Signed-off-by: Rahul Sandhu <nvraxn@gmail.com>

Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>

> ---
>  libselinux/src/selinux_config.c | 75 ++++++++++++++++-----------------
>  1 file changed, 37 insertions(+), 38 deletions(-)
>
> diff --git a/libselinux/src/selinux_config.c b/libselinux/src/selinux_config.c
> index 75db14ba..a2335fa9 100644
> --- a/libselinux/src/selinux_config.c
> +++ b/libselinux/src/selinux_config.c
> @@ -88,47 +88,46 @@ static const uint16_t file_path_suffixes_idx[NEL] = {
>
>  int selinux_getenforcemode(int *enforce)
>  {
> -       int ret = -1;
>         FILE *cfg = fopen(SELINUXCONFIG, "re");
> -       if (cfg) {
> -               char *buf;
> -               char *tag;
> -               int len = sizeof(SELINUXTAG) - 1;
> -               buf = malloc(selinux_page_size);
> -               if (!buf) {
> -                       fclose(cfg);
> -                       return -1;
> -               }
> -               while (fgets_unlocked(buf, selinux_page_size, cfg)) {
> -                       if (strncmp(buf, SELINUXTAG, len))
> -                               continue;
> -                       tag = buf+len;
> -                       while (isspace((unsigned char)*tag))
> -                               tag++;
> -                       if (!strncasecmp
> -                           (tag, "enforcing", sizeof("enforcing") - 1)) {
> -                               *enforce = 1;
> -                               ret = 0;
> -                               break;
> -                       } else
> -                           if (!strncasecmp
> -                               (tag, "permissive",
> -                                sizeof("permissive") - 1)) {
> -                               *enforce = 0;
> -                               ret = 0;
> -                               break;
> -                       } else
> -                           if (!strncasecmp
> -                               (tag, "disabled",
> -                                sizeof("disabled") - 1)) {
> -                               *enforce = -1;
> -                               ret = 0;
> -                               break;
> -                       }
> -               }
> +       if (!cfg)
> +               return -1;
> +
> +       char *buf = malloc(selinux_page_size);
> +       if (!buf) {
>                 fclose(cfg);
> -               free(buf);
> +               return -1;
>         }
> +
> +       int ret = -1;
> +       static const int len = sizeof(SELINUXTAG) - 1;
> +       while (fgets_unlocked(buf, selinux_page_size, cfg)) {
> +               if (strncmp(buf, SELINUXTAG, len))
> +                       continue;
> +
> +               char *tag = buf + len;
> +               while (isspace((unsigned char)*tag))
> +                       tag++;
> +
> +               if (!strncasecmp(tag, "enforcing", sizeof("enforcing") - 1)) {
> +                       *enforce = 1;
> +                       ret = 0;
> +                       break;
> +               } else if (!strncasecmp(tag, "permissive",
> +                                       sizeof("permissive") - 1)) {
> +                       *enforce = 0;
> +                       ret = 0;
> +                       break;
> +               } else if (!strncasecmp(tag, "disabled",
> +                                       sizeof("disabled") - 1)) {
> +                       *enforce = -1;
> +                       ret = 0;
> +                       break;
> +               }
> +       }
> +
> +       fclose(cfg);
> +       free(buf);
> +
>         return ret;
>  }
>
> --
> 2.50.1
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] libselinux: refactor selinux_getenforcemode
  2025-07-29 14:33 ` Stephen Smalley
@ 2025-07-29 17:07   ` Christian Göttsche
  2025-07-30  7:47     ` [PATCH v2] " Rahul Sandhu
  0 siblings, 1 reply; 6+ messages in thread
From: Christian Göttsche @ 2025-07-29 17:07 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: Rahul Sandhu, selinux

Jul 29, 2025 16:34:02 Stephen Smalley <stephen.smalley.work@gmail.com>:

> On Sun, Jul 27, 2025 at 11:33 AM Rahul Sandhu <nvraxn@gmail.com> wrote:
>>
>> Invert the check for cfg being a nullptr and early return.
>>
>> Signed-off-by: Rahul Sandhu <nvraxn@gmail.com>
>
> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
>
>> ---
>> libselinux/src/selinux_config.c | 75 ++++++++++++++++-----------------
>> 1 file changed, 37 insertions(+), 38 deletions(-)
>>
>> diff --git a/libselinux/src/selinux_config.c b/libselinux/src/selinux_config.c
>> index 75db14ba..a2335fa9 100644
>> --- a/libselinux/src/selinux_config.c
>> +++ b/libselinux/src/selinux_config.c
>> @@ -88,47 +88,46 @@ static const uint16_t file_path_suffixes_idx[NEL] = {
>>
>> int selinux_getenforcemode(int *enforce)
>> {
>> -       int ret = -1;
>>         FILE *cfg = fopen(SELINUXCONFIG, "re");
>> -       if (cfg) {
>> -               char *buf;
>> -               char *tag;
>> -               int len = sizeof(SELINUXTAG) - 1;
>> -               buf = malloc(selinux_page_size);
>> -               if (!buf) {
>> -                       fclose(cfg);
>> -                       return -1;
>> -               }
>> -               while (fgets_unlocked(buf, selinux_page_size, cfg)) {
>> -                       if (strncmp(buf, SELINUXTAG, len))
>> -                               continue;
>> -                       tag = buf+len;
>> -                       while (isspace((unsigned char)*tag))
>> -                               tag++;
>> -                       if (!strncasecmp
>> -                           (tag, "enforcing", sizeof("enforcing") - 1)) {
>> -                               *enforce = 1;
>> -                               ret = 0;
>> -                               break;
>> -                       } else
>> -                           if (!strncasecmp
>> -                               (tag, "permissive",
>> -                                sizeof("permissive") - 1)) {
>> -                               *enforce = 0;
>> -                               ret = 0;
>> -                               break;
>> -                       } else
>> -                           if (!strncasecmp
>> -                               (tag, "disabled",
>> -                                sizeof("disabled") - 1)) {
>> -                               *enforce = -1;
>> -                               ret = 0;
>> -                               break;
>> -                       }
>> -               }
>> +       if (!cfg)
>> +               return -1;
>> +
>> +       char *buf = malloc(selinux_page_size);
>> +       if (!buf) {
>>                 fclose(cfg);
>> -               free(buf);
>> +               return -1;
>>         }
>> +
>> +       int ret = -1;
>> +       static const int len = sizeof(SELINUXTAG) - 1;

Does this constant need to be static?

>> +       while (fgets_unlocked(buf, selinux_page_size, cfg)) {
>> +               if (strncmp(buf, SELINUXTAG, len))
>> +                       continue;
>> +
>> +               char *tag = buf + len;
>> +               while (isspace((unsigned char)*tag))
>> +                       tag++;
>> +
>> +               if (!strncasecmp(tag, "enforcing", sizeof("enforcing") - 1)) {
>> +                       *enforce = 1;
>> +                       ret = 0;
>> +                       break;
>> +               } else if (!strncasecmp(tag, "permissive",
>> +                                       sizeof("permissive") - 1)) {
>> +                       *enforce = 0;
>> +                       ret = 0;
>> +                       break;
>> +               } else if (!strncasecmp(tag, "disabled",
>> +                                       sizeof("disabled") - 1)) {
>> +                       *enforce = -1;
>> +                       ret = 0;
>> +                       break;
>> +               }
>> +       }
>> +
>> +       fclose(cfg);
>> +       free(buf);
>> +
>>         return ret;
>> }
>>
>> --
>> 2.50.1
>>
>>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] libselinux: refactor selinux_getenforcemode
  2025-07-29 17:07   ` Christian Göttsche
@ 2025-07-30  7:47     ` Rahul Sandhu
  2025-07-30 12:59       ` Stephen Smalley
  0 siblings, 1 reply; 6+ messages in thread
From: Rahul Sandhu @ 2025-07-30  7:47 UTC (permalink / raw)
  To: cgzones; +Cc: nvraxn, selinux, stephen.smalley.work

Invert the check for cfg being a nullptr and early return.

Signed-off-by: Rahul Sandhu <nvraxn@gmail.com>
---
 libselinux/src/selinux_config.c | 75 ++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 38 deletions(-)

v2: Don't mark len static.

diff --git a/libselinux/src/selinux_config.c b/libselinux/src/selinux_config.c
index 75db14ba..e1bc1b79 100644
--- a/libselinux/src/selinux_config.c
+++ b/libselinux/src/selinux_config.c
@@ -88,47 +88,46 @@ static const uint16_t file_path_suffixes_idx[NEL] = {
 
 int selinux_getenforcemode(int *enforce)
 {
-	int ret = -1;
 	FILE *cfg = fopen(SELINUXCONFIG, "re");
-	if (cfg) {
-		char *buf;
-		char *tag;
-		int len = sizeof(SELINUXTAG) - 1;
-		buf = malloc(selinux_page_size);
-		if (!buf) {
-			fclose(cfg);
-			return -1;
-		}
-		while (fgets_unlocked(buf, selinux_page_size, cfg)) {
-			if (strncmp(buf, SELINUXTAG, len))
-				continue;
-			tag = buf+len;
-			while (isspace((unsigned char)*tag))
-				tag++;
-			if (!strncasecmp
-			    (tag, "enforcing", sizeof("enforcing") - 1)) {
-				*enforce = 1;
-				ret = 0;
-				break;
-			} else
-			    if (!strncasecmp
-				(tag, "permissive",
-				 sizeof("permissive") - 1)) {
-				*enforce = 0;
-				ret = 0;
-				break;
-			} else
-			    if (!strncasecmp
-				(tag, "disabled",
-				 sizeof("disabled") - 1)) {
-				*enforce = -1;
-				ret = 0;
-				break;
-			}
-		}
+	if (!cfg)
+		return -1;
+
+	char *buf = malloc(selinux_page_size);
+	if (!buf) {
 		fclose(cfg);
-		free(buf);
+		return -1;
 	}
+
+	int ret = -1;
+	const int len = sizeof(SELINUXTAG) - 1;
+	while (fgets_unlocked(buf, selinux_page_size, cfg)) {
+		if (strncmp(buf, SELINUXTAG, len))
+			continue;
+
+		char *tag = buf + len;
+		while (isspace((unsigned char)*tag))
+			tag++;
+
+		if (!strncasecmp(tag, "enforcing", sizeof("enforcing") - 1)) {
+			*enforce = 1;
+			ret = 0;
+			break;
+		} else if (!strncasecmp(tag, "permissive",
+					sizeof("permissive") - 1)) {
+			*enforce = 0;
+			ret = 0;
+			break;
+		} else if (!strncasecmp(tag, "disabled",
+					sizeof("disabled") - 1)) {
+			*enforce = -1;
+			ret = 0;
+			break;
+		}
+	}
+
+	fclose(cfg);
+	free(buf);
+
 	return ret;
 }
 
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] libselinux: refactor selinux_getenforcemode
  2025-07-30  7:47     ` [PATCH v2] " Rahul Sandhu
@ 2025-07-30 12:59       ` Stephen Smalley
  2025-07-31 14:17         ` Stephen Smalley
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Smalley @ 2025-07-30 12:59 UTC (permalink / raw)
  To: Rahul Sandhu; +Cc: cgzones, selinux

On Wed, Jul 30, 2025 at 3:47 AM Rahul Sandhu <nvraxn@gmail.com> wrote:
>
> Invert the check for cfg being a nullptr and early return.
>
> Signed-off-by: Rahul Sandhu <nvraxn@gmail.com>

Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>

> ---
>  libselinux/src/selinux_config.c | 75 ++++++++++++++++-----------------
>  1 file changed, 37 insertions(+), 38 deletions(-)
>
> v2: Don't mark len static.
>
> diff --git a/libselinux/src/selinux_config.c b/libselinux/src/selinux_config.c
> index 75db14ba..e1bc1b79 100644
> --- a/libselinux/src/selinux_config.c
> +++ b/libselinux/src/selinux_config.c
> @@ -88,47 +88,46 @@ static const uint16_t file_path_suffixes_idx[NEL] = {
>
>  int selinux_getenforcemode(int *enforce)
>  {
> -       int ret = -1;
>         FILE *cfg = fopen(SELINUXCONFIG, "re");
> -       if (cfg) {
> -               char *buf;
> -               char *tag;
> -               int len = sizeof(SELINUXTAG) - 1;
> -               buf = malloc(selinux_page_size);
> -               if (!buf) {
> -                       fclose(cfg);
> -                       return -1;
> -               }
> -               while (fgets_unlocked(buf, selinux_page_size, cfg)) {
> -                       if (strncmp(buf, SELINUXTAG, len))
> -                               continue;
> -                       tag = buf+len;
> -                       while (isspace((unsigned char)*tag))
> -                               tag++;
> -                       if (!strncasecmp
> -                           (tag, "enforcing", sizeof("enforcing") - 1)) {
> -                               *enforce = 1;
> -                               ret = 0;
> -                               break;
> -                       } else
> -                           if (!strncasecmp
> -                               (tag, "permissive",
> -                                sizeof("permissive") - 1)) {
> -                               *enforce = 0;
> -                               ret = 0;
> -                               break;
> -                       } else
> -                           if (!strncasecmp
> -                               (tag, "disabled",
> -                                sizeof("disabled") - 1)) {
> -                               *enforce = -1;
> -                               ret = 0;
> -                               break;
> -                       }
> -               }
> +       if (!cfg)
> +               return -1;
> +
> +       char *buf = malloc(selinux_page_size);
> +       if (!buf) {
>                 fclose(cfg);
> -               free(buf);
> +               return -1;
>         }
> +
> +       int ret = -1;
> +       const int len = sizeof(SELINUXTAG) - 1;
> +       while (fgets_unlocked(buf, selinux_page_size, cfg)) {
> +               if (strncmp(buf, SELINUXTAG, len))
> +                       continue;
> +
> +               char *tag = buf + len;
> +               while (isspace((unsigned char)*tag))
> +                       tag++;
> +
> +               if (!strncasecmp(tag, "enforcing", sizeof("enforcing") - 1)) {
> +                       *enforce = 1;
> +                       ret = 0;
> +                       break;
> +               } else if (!strncasecmp(tag, "permissive",
> +                                       sizeof("permissive") - 1)) {
> +                       *enforce = 0;
> +                       ret = 0;
> +                       break;
> +               } else if (!strncasecmp(tag, "disabled",
> +                                       sizeof("disabled") - 1)) {
> +                       *enforce = -1;
> +                       ret = 0;
> +                       break;
> +               }
> +       }
> +
> +       fclose(cfg);
> +       free(buf);
> +
>         return ret;
>  }
>
> --
> 2.50.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] libselinux: refactor selinux_getenforcemode
  2025-07-30 12:59       ` Stephen Smalley
@ 2025-07-31 14:17         ` Stephen Smalley
  0 siblings, 0 replies; 6+ messages in thread
From: Stephen Smalley @ 2025-07-31 14:17 UTC (permalink / raw)
  To: Rahul Sandhu; +Cc: cgzones, selinux

On Wed, Jul 30, 2025 at 8:59 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Wed, Jul 30, 2025 at 3:47 AM Rahul Sandhu <nvraxn@gmail.com> wrote:
> >
> > Invert the check for cfg being a nullptr and early return.
> >
> > Signed-off-by: Rahul Sandhu <nvraxn@gmail.com>
>
> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>

Thanks, applied.

>
> > ---
> >  libselinux/src/selinux_config.c | 75 ++++++++++++++++-----------------
> >  1 file changed, 37 insertions(+), 38 deletions(-)
> >
> > v2: Don't mark len static.
> >
> > diff --git a/libselinux/src/selinux_config.c b/libselinux/src/selinux_config.c
> > index 75db14ba..e1bc1b79 100644
> > --- a/libselinux/src/selinux_config.c
> > +++ b/libselinux/src/selinux_config.c
> > @@ -88,47 +88,46 @@ static const uint16_t file_path_suffixes_idx[NEL] = {
> >
> >  int selinux_getenforcemode(int *enforce)
> >  {
> > -       int ret = -1;
> >         FILE *cfg = fopen(SELINUXCONFIG, "re");
> > -       if (cfg) {
> > -               char *buf;
> > -               char *tag;
> > -               int len = sizeof(SELINUXTAG) - 1;
> > -               buf = malloc(selinux_page_size);
> > -               if (!buf) {
> > -                       fclose(cfg);
> > -                       return -1;
> > -               }
> > -               while (fgets_unlocked(buf, selinux_page_size, cfg)) {
> > -                       if (strncmp(buf, SELINUXTAG, len))
> > -                               continue;
> > -                       tag = buf+len;
> > -                       while (isspace((unsigned char)*tag))
> > -                               tag++;
> > -                       if (!strncasecmp
> > -                           (tag, "enforcing", sizeof("enforcing") - 1)) {
> > -                               *enforce = 1;
> > -                               ret = 0;
> > -                               break;
> > -                       } else
> > -                           if (!strncasecmp
> > -                               (tag, "permissive",
> > -                                sizeof("permissive") - 1)) {
> > -                               *enforce = 0;
> > -                               ret = 0;
> > -                               break;
> > -                       } else
> > -                           if (!strncasecmp
> > -                               (tag, "disabled",
> > -                                sizeof("disabled") - 1)) {
> > -                               *enforce = -1;
> > -                               ret = 0;
> > -                               break;
> > -                       }
> > -               }
> > +       if (!cfg)
> > +               return -1;
> > +
> > +       char *buf = malloc(selinux_page_size);
> > +       if (!buf) {
> >                 fclose(cfg);
> > -               free(buf);
> > +               return -1;
> >         }
> > +
> > +       int ret = -1;
> > +       const int len = sizeof(SELINUXTAG) - 1;
> > +       while (fgets_unlocked(buf, selinux_page_size, cfg)) {
> > +               if (strncmp(buf, SELINUXTAG, len))
> > +                       continue;
> > +
> > +               char *tag = buf + len;
> > +               while (isspace((unsigned char)*tag))
> > +                       tag++;
> > +
> > +               if (!strncasecmp(tag, "enforcing", sizeof("enforcing") - 1)) {
> > +                       *enforce = 1;
> > +                       ret = 0;
> > +                       break;
> > +               } else if (!strncasecmp(tag, "permissive",
> > +                                       sizeof("permissive") - 1)) {
> > +                       *enforce = 0;
> > +                       ret = 0;
> > +                       break;
> > +               } else if (!strncasecmp(tag, "disabled",
> > +                                       sizeof("disabled") - 1)) {
> > +                       *enforce = -1;
> > +                       ret = 0;
> > +                       break;
> > +               }
> > +       }
> > +
> > +       fclose(cfg);
> > +       free(buf);
> > +
> >         return ret;
> >  }
> >
> > --
> > 2.50.1
> >

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-07-31 14:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-27 15:32 [PATCH] libselinux: refactor selinux_getenforcemode Rahul Sandhu
2025-07-29 14:33 ` Stephen Smalley
2025-07-29 17:07   ` Christian Göttsche
2025-07-30  7:47     ` [PATCH v2] " Rahul Sandhu
2025-07-30 12:59       ` Stephen Smalley
2025-07-31 14:17         ` Stephen Smalley

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).