selinux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] libselinux: refactor selinux_check_securetty_context
@ 2025-07-31 16:21 Rahul Sandhu
  2025-07-31 16:23 ` [PATCH v2] " Rahul Sandhu
  2025-08-01 17:31 ` [PATCH] " Stephen Smalley
  0 siblings, 2 replies; 4+ messages in thread
From: Rahul Sandhu @ 2025-07-31 16:21 UTC (permalink / raw)
  To: selinux; +Cc: Rahul Sandhu

Invert the checks for fp and con, and initalise char *end as NULL.

Signed-off-by: Rahul Sandhu <nvraxn@gmail.com>
---
 .../src/selinux_check_securetty_context.c     | 80 +++++++++++--------
 1 file changed, 46 insertions(+), 34 deletions(-)

diff --git a/libselinux/src/selinux_check_securetty_context.c b/libselinux/src/selinux_check_securetty_context.c
index 7609752e..f04b66e0 100644
--- a/libselinux/src/selinux_check_securetty_context.c
+++ b/libselinux/src/selinux_check_securetty_context.c
@@ -1,3 +1,4 @@
+#include <fcntl.h>
 #include <unistd.h>
 #include <stdlib.h>
 #include <string.h>
@@ -8,45 +9,56 @@
 
 int selinux_check_securetty_context(const char * tty_context)
 {
+	context_t con = context_new(tty_context);
+	if (!con) {
+		return -1;
+	}
+
+	FILE *fp = fopen(selinux_securetty_types_path(), "re");
+	if (!fp) {
+		context_free(con);
+		return -1;
+	}
+
+	const char *type = context_type_get(con);
+
 	char *line = NULL;
-	char *start, *end = NULL;
+	char *start = NULL;
+	char *end = NULL;
 	size_t line_len = 0;
-	ssize_t len;
 	int found = -1;
-	FILE *fp;
-	fp = fopen(selinux_securetty_types_path(), "re");
-	if (fp) {
-		context_t con = context_new(tty_context);
-		if (con) {
-			const char *type = context_type_get(con);
-			while ((len = getline(&line, &line_len, fp)) != -1) {
-
-				if (line[len - 1] == '\n')
-					line[len - 1] = 0;
-
-				/* Skip leading whitespace. */
-				start = line;
-				while (*start && isspace((unsigned char)*start))
-					start++;
-				if (!(*start))
-					continue;
-
-				end = start;
-				while (*end && !isspace((unsigned char)*end))
-					end++;
-				if (*end)
-					*end++ = 0;
-				if (!strcmp(type, start)) {
-					found = 0;
-					break;
-				}
-			}
-			free(line);
-			context_free(con);
+	ssize_t len;
+	while ((len = getline(&line, &line_len, fp)) != -1) {
+
+		if (line[len - 1] == '\n') {
+			line[len - 1] = 0;
+		}
+
+		/* Skip leading whitespace. */
+		start = line;
+		while (*start && isspace((unsigned char)*start)) {
+			start++;
+		}
+		if (!(*start)) {
+			continue;
+		}
+
+		end = start;
+		while (*end && !isspace((unsigned char)*end)) {
+			end++;
+		}
+		if (*end) {
+			*end++ = 0;
+		}
+		if (!strcmp(type, start)) {
+			found = 0;
+			break;
 		}
-		fclose(fp);
 	}
 
+	free(line);
+	context_free(con);
+	fclose(fp);
+
 	return found;
 }
-
-- 
2.50.1


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

* [PATCH v2] libselinux: refactor selinux_check_securetty_context
  2025-07-31 16:21 [PATCH] libselinux: refactor selinux_check_securetty_context Rahul Sandhu
@ 2025-07-31 16:23 ` Rahul Sandhu
  2025-08-01 17:31 ` [PATCH] " Stephen Smalley
  1 sibling, 0 replies; 4+ messages in thread
From: Rahul Sandhu @ 2025-07-31 16:23 UTC (permalink / raw)
  To: nvraxn; +Cc: selinux

Invert the checks for fp and con, and initalise char *end as NULL.

Signed-off-by: Rahul Sandhu <nvraxn@gmail.com>
---
 .../src/selinux_check_securetty_context.c     | 79 +++++++++++--------
 1 file changed, 45 insertions(+), 34 deletions(-)

v2: oops, didn't mean to include fcntl.h

diff --git a/libselinux/src/selinux_check_securetty_context.c b/libselinux/src/selinux_check_securetty_context.c
index 7609752e..6d039f8f 100644
--- a/libselinux/src/selinux_check_securetty_context.c
+++ b/libselinux/src/selinux_check_securetty_context.c
@@ -8,45 +8,56 @@
 
 int selinux_check_securetty_context(const char * tty_context)
 {
+	context_t con = context_new(tty_context);
+	if (!con) {
+		return -1;
+	}
+
+	FILE *fp = fopen(selinux_securetty_types_path(), "re");
+	if (!fp) {
+		context_free(con);
+		return -1;
+	}
+
+	const char *type = context_type_get(con);
+
 	char *line = NULL;
-	char *start, *end = NULL;
+	char *start = NULL;
+	char *end = NULL;
 	size_t line_len = 0;
-	ssize_t len;
 	int found = -1;
-	FILE *fp;
-	fp = fopen(selinux_securetty_types_path(), "re");
-	if (fp) {
-		context_t con = context_new(tty_context);
-		if (con) {
-			const char *type = context_type_get(con);
-			while ((len = getline(&line, &line_len, fp)) != -1) {
-
-				if (line[len - 1] == '\n')
-					line[len - 1] = 0;
-
-				/* Skip leading whitespace. */
-				start = line;
-				while (*start && isspace((unsigned char)*start))
-					start++;
-				if (!(*start))
-					continue;
-
-				end = start;
-				while (*end && !isspace((unsigned char)*end))
-					end++;
-				if (*end)
-					*end++ = 0;
-				if (!strcmp(type, start)) {
-					found = 0;
-					break;
-				}
-			}
-			free(line);
-			context_free(con);
+	ssize_t len;
+	while ((len = getline(&line, &line_len, fp)) != -1) {
+
+		if (line[len - 1] == '\n') {
+			line[len - 1] = 0;
+		}
+
+		/* Skip leading whitespace. */
+		start = line;
+		while (*start && isspace((unsigned char)*start)) {
+			start++;
+		}
+		if (!(*start)) {
+			continue;
+		}
+
+		end = start;
+		while (*end && !isspace((unsigned char)*end)) {
+			end++;
+		}
+		if (*end) {
+			*end++ = 0;
+		}
+		if (!strcmp(type, start)) {
+			found = 0;
+			break;
 		}
-		fclose(fp);
 	}
 
+	free(line);
+	context_free(con);
+	fclose(fp);
+
 	return found;
 }
-
-- 
2.50.1


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

* Re: [PATCH] libselinux: refactor selinux_check_securetty_context
  2025-07-31 16:21 [PATCH] libselinux: refactor selinux_check_securetty_context Rahul Sandhu
  2025-07-31 16:23 ` [PATCH v2] " Rahul Sandhu
@ 2025-08-01 17:31 ` Stephen Smalley
  2025-08-01 17:45   ` [PATCH v3] " Rahul Sandhu
  1 sibling, 1 reply; 4+ messages in thread
From: Stephen Smalley @ 2025-08-01 17:31 UTC (permalink / raw)
  To: Rahul Sandhu; +Cc: selinux

On Thu, Jul 31, 2025 at 12:22 PM Rahul Sandhu <nvraxn@gmail.com> wrote:
>
> Invert the checks for fp and con, and initalise char *end as NULL.

spelling (initalise) and *end was already initialized, also see below.

>
> Signed-off-by: Rahul Sandhu <nvraxn@gmail.com>
> ---
>  .../src/selinux_check_securetty_context.c     | 80 +++++++++++--------
>  1 file changed, 46 insertions(+), 34 deletions(-)
>
> diff --git a/libselinux/src/selinux_check_securetty_context.c b/libselinux/src/selinux_check_securetty_context.c
> index 7609752e..f04b66e0 100644
> --- a/libselinux/src/selinux_check_securetty_context.c
> +++ b/libselinux/src/selinux_check_securetty_context.c
> @@ -1,3 +1,4 @@
> +#include <fcntl.h>
>  #include <unistd.h>
>  #include <stdlib.h>
>  #include <string.h>
> @@ -8,45 +9,56 @@
>
>  int selinux_check_securetty_context(const char * tty_context)
>  {
> +       context_t con = context_new(tty_context);

This means we allocate con before we even know if we will ever reach
code that will need it.

> +       if (!con) {

No need for { } for single-statement bodies.

> +               return -1;
> +       }
> +
> +       FILE *fp = fopen(selinux_securetty_types_path(), "re");
> +       if (!fp) {
> +               context_free(con);
> +               return -1;
> +       }
> +
> +       const char *type = context_type_get(con);
> +
>         char *line = NULL;
> -       char *start, *end = NULL;
> +       char *start = NULL;

start is only set and used within the loop body so no need to
initialize it here.

> +       char *end = NULL;
>         size_t line_len = 0;
> -       ssize_t len;
>         int found = -1;
> -       FILE *fp;
> -       fp = fopen(selinux_securetty_types_path(), "re");
> -       if (fp) {
> -               context_t con = context_new(tty_context);
> -               if (con) {
> -                       const char *type = context_type_get(con);
> -                       while ((len = getline(&line, &line_len, fp)) != -1) {
> -
> -                               if (line[len - 1] == '\n')
> -                                       line[len - 1] = 0;
> -
> -                               /* Skip leading whitespace. */
> -                               start = line;
> -                               while (*start && isspace((unsigned char)*start))
> -                                       start++;
> -                               if (!(*start))
> -                                       continue;
> -
> -                               end = start;
> -                               while (*end && !isspace((unsigned char)*end))
> -                                       end++;
> -                               if (*end)
> -                                       *end++ = 0;
> -                               if (!strcmp(type, start)) {
> -                                       found = 0;
> -                                       break;
> -                               }
> -                       }
> -                       free(line);
> -                       context_free(con);
> +       ssize_t len;
> +       while ((len = getline(&line, &line_len, fp)) != -1) {
> +
> +               if (line[len - 1] == '\n') {
> +                       line[len - 1] = 0;
> +               }
> +
> +               /* Skip leading whitespace. */
> +               start = line;
> +               while (*start && isspace((unsigned char)*start)) {
> +                       start++;
> +               }
> +               if (!(*start)) {
> +                       continue;
> +               }
> +
> +               end = start;
> +               while (*end && !isspace((unsigned char)*end)) {
> +                       end++;
> +               }
> +               if (*end) {
> +                       *end++ = 0;
> +               }
> +               if (!strcmp(type, start)) {
> +                       found = 0;
> +                       break;
>                 }
> -               fclose(fp);
>         }
>
> +       free(line);
> +       context_free(con);
> +       fclose(fp);
> +
>         return found;
>  }
> -
> --
> 2.50.1
>
>

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

* [PATCH v3] libselinux: refactor selinux_check_securetty_context
  2025-08-01 17:31 ` [PATCH] " Stephen Smalley
@ 2025-08-01 17:45   ` Rahul Sandhu
  0 siblings, 0 replies; 4+ messages in thread
From: Rahul Sandhu @ 2025-08-01 17:45 UTC (permalink / raw)
  To: stephen.smalley.work; +Cc: nvraxn, selinux

Invert the checks for fp and con.

Signed-off-by: Rahul Sandhu <nvraxn@gmail.com>
---
 .../src/selinux_check_securetty_context.c     | 69 ++++++++++---------
 1 file changed, 36 insertions(+), 33 deletions(-)

diff --git a/libselinux/src/selinux_check_securetty_context.c b/libselinux/src/selinux_check_securetty_context.c
index 7609752e..bf61f53d 100644
--- a/libselinux/src/selinux_check_securetty_context.c
+++ b/libselinux/src/selinux_check_securetty_context.c
@@ -8,45 +8,48 @@
 
 int selinux_check_securetty_context(const char * tty_context)
 {
+	FILE *fp = fopen(selinux_securetty_types_path(), "re");
+	if (!fp)
+		return -1;
+
+	context_t con = context_new(tty_context);
+	if (!con) {
+		fclose(fp);
+		return -1;
+	}
+
+	const char *type = context_type_get(con);
+
 	char *line = NULL;
 	char *start, *end = NULL;
 	size_t line_len = 0;
-	ssize_t len;
 	int found = -1;
-	FILE *fp;
-	fp = fopen(selinux_securetty_types_path(), "re");
-	if (fp) {
-		context_t con = context_new(tty_context);
-		if (con) {
-			const char *type = context_type_get(con);
-			while ((len = getline(&line, &line_len, fp)) != -1) {
-
-				if (line[len - 1] == '\n')
-					line[len - 1] = 0;
-
-				/* Skip leading whitespace. */
-				start = line;
-				while (*start && isspace((unsigned char)*start))
-					start++;
-				if (!(*start))
-					continue;
-
-				end = start;
-				while (*end && !isspace((unsigned char)*end))
-					end++;
-				if (*end)
-					*end++ = 0;
-				if (!strcmp(type, start)) {
-					found = 0;
-					break;
-				}
-			}
-			free(line);
-			context_free(con);
+	ssize_t len;
+	while ((len = getline(&line, &line_len, fp)) != -1) {
+		if (line[len - 1] == '\n')
+			line[len - 1] = 0;
+
+		/* Skip leading whitespace. */
+		start = line;
+		while (*start && isspace((unsigned char)*start))
+			start++;
+		if (!(*start))
+			continue;
+
+		end = start;
+		while (*end && !isspace((unsigned char)*end))
+			end++;
+		if (*end)
+			*end++ = 0;
+		if (!strcmp(type, start)) {
+			found = 0;
+			break;
 		}
-		fclose(fp);
 	}
 
+	free(line);
+	context_free(con);
+	fclose(fp);
+
 	return found;
 }
-
-- 
2.50.1


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

end of thread, other threads:[~2025-08-01 17:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-31 16:21 [PATCH] libselinux: refactor selinux_check_securetty_context Rahul Sandhu
2025-07-31 16:23 ` [PATCH v2] " Rahul Sandhu
2025-08-01 17:31 ` [PATCH] " Stephen Smalley
2025-08-01 17:45   ` [PATCH v3] " Rahul Sandhu

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