* [PATCH] libselinux: refactor selinux_check_securetty_context
@ 2025-07-31 16:21 Rahul Sandhu
2025-08-01 17:31 ` Stephen Smalley
0 siblings, 1 reply; 6+ 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] 6+ messages in thread
* Re: [PATCH] libselinux: refactor selinux_check_securetty_context
2025-07-31 16:21 Rahul Sandhu
@ 2025-08-01 17:31 ` Stephen Smalley
0 siblings, 0 replies; 6+ 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] 6+ messages in thread
* Re: [PATCH] libselinux: refactor selinux_check_securetty_context
[not found] <CAEjxPJ60msaQXmD3kWOPoLb-=Fx1WK2TdGObpg Vq40-yJEfTw@mail.gmail.com>
@ 2025-08-01 17:42 ` Rahul Sandhu
2025-08-01 17:57 ` Stephen Smalley
0 siblings, 1 reply; 6+ messages in thread
From: Rahul Sandhu @ 2025-08-01 17:42 UTC (permalink / raw)
To: stephen.smalley.work; +Cc: nvraxn, selinux
> No need for { } for single-statement bodies.
Thanks, I wasn't sure about the correct style here as I couldn't find a
style guide. Is there one I could read, and if there is could we link
to it in the CONTRIBUTING guide[1]?
[1] https://github.com/SELinuxProject/selinux/blob/main/CONTRIBUTING.md
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libselinux: refactor selinux_check_securetty_context
2025-08-01 17:42 ` [PATCH] libselinux: refactor selinux_check_securetty_context Rahul Sandhu
@ 2025-08-01 17:57 ` Stephen Smalley
0 siblings, 0 replies; 6+ messages in thread
From: Stephen Smalley @ 2025-08-01 17:57 UTC (permalink / raw)
To: Rahul Sandhu; +Cc: selinux
On Fri, Aug 1, 2025 at 1:42 PM Rahul Sandhu <nvraxn@gmail.com> wrote:
>
> > No need for { } for single-statement bodies.
>
> Thanks, I wasn't sure about the correct style here as I couldn't find a
> style guide. Is there one I could read, and if there is could we link
> to it in the CONTRIBUTING guide[1]?
>
> [1] https://github.com/SELinuxProject/selinux/blob/main/CONTRIBUTING.md
Unfortunately not. It would be good to specify one at some point and
start applying it. The selinux-testsuite has a tools/check-syntax
script for checking (and optionally fixing) coding style; we could
either duplicate that for the userspace or come up with something
else. There is a scripts/Lindent in the selinux userspace tree from
Linux but it hasn't really been used or enforced by most.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libselinux: refactor selinux_check_securetty_context
[not found] <CAEjxPJ5OXCS1nkJzukkp61J3a4fmGeOLftTiDHKjQmPvDw yKQ@mail.gmail.com>
@ 2025-08-01 18:02 ` Rahul Sandhu
2025-08-01 18:28 ` Stephen Smalley
0 siblings, 1 reply; 6+ messages in thread
From: Rahul Sandhu @ 2025-08-01 18:02 UTC (permalink / raw)
To: stephen.smalley.work; +Cc: nvraxn, selinux
> Unfortunately not. It would be good to specify one at some point and
> start applying it.
Ah okay, and yea sounds reasonable.
> The selinux-testsuite has a tools/check-syntax script for checking
> (and optionally fixing) coding style; we could either duplicate that
> for the userspace or come up with something else.
Maybe we could consider clang-format? My understanding is that Linux
has also adopted it, and our code style doesn't seem too far off, so
that could probably be tweaked without too much work. Also has the
advantage of a lot of tooling and editors supporting/integrating with
it.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libselinux: refactor selinux_check_securetty_context
2025-08-01 18:02 ` Rahul Sandhu
@ 2025-08-01 18:28 ` Stephen Smalley
0 siblings, 0 replies; 6+ messages in thread
From: Stephen Smalley @ 2025-08-01 18:28 UTC (permalink / raw)
To: Rahul Sandhu; +Cc: selinux
On Fri, Aug 1, 2025 at 2:02 PM Rahul Sandhu <nvraxn@gmail.com> wrote:
>
> > Unfortunately not. It would be good to specify one at some point and
> > start applying it.
>
> Ah okay, and yea sounds reasonable.
>
> > The selinux-testsuite has a tools/check-syntax script for checking
> > (and optionally fixing) coding style; we could either duplicate that
> > for the userspace or come up with something else.
>
> Maybe we could consider clang-format? My understanding is that Linux
> has also adopted it, and our code style doesn't seem too far off, so
> that could probably be tweaked without too much work. Also has the
> advantage of a lot of tooling and editors supporting/integrating with
> it.
I would be fine with that, and with using the Linux kernel's config
for it. Other SELinux userspace maintainers are free to speak up if
they disagree.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-08-01 18:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CAEjxPJ60msaQXmD3kWOPoLb-=Fx1WK2TdGObpg Vq40-yJEfTw@mail.gmail.com>
2025-08-01 17:42 ` [PATCH] libselinux: refactor selinux_check_securetty_context Rahul Sandhu
2025-08-01 17:57 ` Stephen Smalley
[not found] <CAEjxPJ5OXCS1nkJzukkp61J3a4fmGeOLftTiDHKjQmPvDw yKQ@mail.gmail.com>
2025-08-01 18:02 ` Rahul Sandhu
2025-08-01 18:28 ` Stephen Smalley
2025-07-31 16:21 Rahul Sandhu
2025-08-01 17:31 ` 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).