diff mbox series

[15/15] checkpolicy: misc policy_define.c cleanup

Message ID 20240122135507.63506-15-cgzones@googlemail.com (mailing list archive)
State Accepted
Commit 3dc1116992f2
Delegated to: Petr Lautrbach
Headers show
Series [01/15] checkpolicy: add libfuzz based fuzzer | expand

Commit Message

Christian Göttsche Jan. 22, 2024, 1:55 p.m. UTC 
Sync function parameter names.

Drop superfluous return value.

  The function avrule_merge_ioctls() has no failure conditions and
  always returns 0.

Drop duplicate include.

Use native type for ranges.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/policy_define.c | 27 ++++++++++++---------------
 checkpolicy/policy_define.h |  2 +-
 2 files changed, 13 insertions(+), 16 deletions(-)

Comments

James Carter Feb. 13, 2024, 8:39 p.m. UTC | #1 
On Mon, Jan 22, 2024 at 9:02 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Sync function parameter names.
>
> Drop superfluous return value.
>
>   The function avrule_merge_ioctls() has no failure conditions and
>   always returns 0.
>
> Drop duplicate include.
>
> Use native type for ranges.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Acked-by: James Carter <jwcart2@gmail.com>

> ---
>  checkpolicy/policy_define.c | 27 ++++++++++++---------------
>  checkpolicy/policy_define.h |  2 +-
>  2 files changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
> index cd49cae3..79d67a78 100644
> --- a/checkpolicy/policy_define.c
> +++ b/checkpolicy/policy_define.c
> @@ -44,7 +44,6 @@
>  #define IPPROTO_SCTP 132
>  #endif
>  #include <arpa/inet.h>
> -#include <stdlib.h>
>  #include <limits.h>
>  #include <inttypes.h>
>  #include <ctype.h>
> @@ -1096,7 +1095,7 @@ int define_level(void)
>
>         while ((id = queue_remove(id_queue))) {
>                 cat_datum_t *cdatum;
> -               int range_start, range_end, i;
> +               uint32_t range_start, range_end, i;
>
>                 if (id_has_dot(id)) {
>                         char *id_start = id;
> @@ -1932,7 +1931,7 @@ error:
>         return -1;
>  }
>
> -static int avrule_merge_ioctls(struct av_ioctl_range_list **rangehead)
> +static void avrule_merge_ioctls(struct av_ioctl_range_list **rangehead)
>  {
>         struct av_ioctl_range_list *r, *tmp;
>         r = *rangehead;
> @@ -1949,7 +1948,6 @@ static int avrule_merge_ioctls(struct av_ioctl_range_list **rangehead)
>                 }
>                 r = r->next;
>         }
> -       return 0;
>  }
>
>  static int avrule_read_ioctls(struct av_ioctl_range_list **rangehead)
> @@ -2070,8 +2068,7 @@ static int avrule_ioctl_ranges(struct av_ioctl_range_list **rangelist)
>         /* sort and merge the input ioctls */
>         if (avrule_sort_ioctls(&rangehead))
>                 return -1;
> -       if (avrule_merge_ioctls(&rangehead))
> -               return -1;
> +       avrule_merge_ioctls(&rangehead);
>         /* flip ranges if these are omitted */
>         if (omit) {
>                 if (avrule_omit_ioctls(&rangehead))
> @@ -3854,7 +3851,7 @@ uintptr_t define_cexpr(uint32_t expr_type, uintptr_t arg1, uintptr_t arg2)
>         return 0;
>  }
>
> -int define_conditional(cond_expr_t * expr, avrule_t * t, avrule_t * f)
> +int define_conditional(cond_expr_t * expr, avrule_t * t_list, avrule_t * f_list)
>  {
>         cond_expr_t *e;
>         int depth, booleans, tunables;
> @@ -3866,15 +3863,15 @@ int define_conditional(cond_expr_t * expr, avrule_t * t, avrule_t * f)
>                 yyerror("illegal conditional expression");
>                 return -1;
>         }
> -       if (!t) {
> -               if (!f) {
> +       if (!t_list) {
> +               if (!f_list) {
>                         /* empty is fine, destroy expression and return */
>                         cond_expr_destroy(expr);
>                         return 0;
>                 }
>                 /* Invert */
> -               t = f;
> -               f = 0;
> +               t_list = f_list;
> +               f_list = NULL;
>                 expr = define_cond_expr(COND_NOT, expr, 0);
>                 if (!expr) {
>                         yyerror("unable to invert conditional expression");
> @@ -3940,8 +3937,8 @@ int define_conditional(cond_expr_t * expr, avrule_t * t, avrule_t * f)
>         /*  use tmp conditional node to partially build new node */
>         memset(&cn, 0, sizeof(cn));
>         cn.expr = expr;
> -       cn.avtrue_list = t;
> -       cn.avfalse_list = f;
> +       cn.avtrue_list = t_list;
> +       cn.avfalse_list = f_list;
>
>         /* normalize/precompute expression */
>         if (cond_normalize_expr(policydbp, &cn) < 0) {
> @@ -4117,7 +4114,7 @@ static int set_user_roles(role_set_t * set, char *id)
>  static int parse_categories(char *id, level_datum_t * levdatum, ebitmap_t * cats)
>  {
>         cat_datum_t *cdatum;
> -       int range_start, range_end, i;
> +       uint32_t range_start, range_end, i;
>
>         if (id_has_dot(id)) {
>                 char *id_start = id;
> @@ -5527,7 +5524,7 @@ static int define_genfs_context_helper(char *fstype, int has_type)
>         class_datum_t *cladatum;
>         char *type = NULL;
>         const char *sclass;
> -       int len, len2;
> +       size_t len, len2;
>
>         if (policydbp->target_platform != SEPOL_TARGET_SELINUX) {
>                 yyerror("genfs not supported for target");
> diff --git a/checkpolicy/policy_define.h b/checkpolicy/policy_define.h
> index 075b048d..bcbfe4f3 100644
> --- a/checkpolicy/policy_define.h
> +++ b/checkpolicy/policy_define.h
> @@ -13,7 +13,7 @@
>  #define FALSE 0
>
>  avrule_t *define_cond_compute_type(int which);
> -avrule_t *define_cond_pol_list(avrule_t *avlist, avrule_t *stmt);
> +avrule_t *define_cond_pol_list(avrule_t *avlist, avrule_t *sl);
>  avrule_t *define_cond_te_avtab(int which);
>  avrule_t *define_cond_filename_trans(void);
>  cond_expr_t *define_cond_expr(uint32_t expr_type, void *arg1, void* arg2);
> --
> 2.43.0
>
>
James Carter March 4, 2024, 7:20 p.m. UTC | #2 
On Tue, Feb 13, 2024 at 3:39 PM James Carter <jwcart2@gmail.com> wrote:
>
> On Mon, Jan 22, 2024 at 9:02 AM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > Sync function parameter names.
> >
> > Drop superfluous return value.
> >
> >   The function avrule_merge_ioctls() has no failure conditions and
> >   always returns 0.
> >
> > Drop duplicate include.
> >
> > Use native type for ranges.
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> Acked-by: James Carter <jwcart2@gmail.com>
>
Merged.
Thanks,
Jim

> > ---
> >  checkpolicy/policy_define.c | 27 ++++++++++++---------------
> >  checkpolicy/policy_define.h |  2 +-
> >  2 files changed, 13 insertions(+), 16 deletions(-)
> >
> > diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
> > index cd49cae3..79d67a78 100644
> > --- a/checkpolicy/policy_define.c
> > +++ b/checkpolicy/policy_define.c
> > @@ -44,7 +44,6 @@
> >  #define IPPROTO_SCTP 132
> >  #endif
> >  #include <arpa/inet.h>
> > -#include <stdlib.h>
> >  #include <limits.h>
> >  #include <inttypes.h>
> >  #include <ctype.h>
> > @@ -1096,7 +1095,7 @@ int define_level(void)
> >
> >         while ((id = queue_remove(id_queue))) {
> >                 cat_datum_t *cdatum;
> > -               int range_start, range_end, i;
> > +               uint32_t range_start, range_end, i;
> >
> >                 if (id_has_dot(id)) {
> >                         char *id_start = id;
> > @@ -1932,7 +1931,7 @@ error:
> >         return -1;
> >  }
> >
> > -static int avrule_merge_ioctls(struct av_ioctl_range_list **rangehead)
> > +static void avrule_merge_ioctls(struct av_ioctl_range_list **rangehead)
> >  {
> >         struct av_ioctl_range_list *r, *tmp;
> >         r = *rangehead;
> > @@ -1949,7 +1948,6 @@ static int avrule_merge_ioctls(struct av_ioctl_range_list **rangehead)
> >                 }
> >                 r = r->next;
> >         }
> > -       return 0;
> >  }
> >
> >  static int avrule_read_ioctls(struct av_ioctl_range_list **rangehead)
> > @@ -2070,8 +2068,7 @@ static int avrule_ioctl_ranges(struct av_ioctl_range_list **rangelist)
> >         /* sort and merge the input ioctls */
> >         if (avrule_sort_ioctls(&rangehead))
> >                 return -1;
> > -       if (avrule_merge_ioctls(&rangehead))
> > -               return -1;
> > +       avrule_merge_ioctls(&rangehead);
> >         /* flip ranges if these are omitted */
> >         if (omit) {
> >                 if (avrule_omit_ioctls(&rangehead))
> > @@ -3854,7 +3851,7 @@ uintptr_t define_cexpr(uint32_t expr_type, uintptr_t arg1, uintptr_t arg2)
> >         return 0;
> >  }
> >
> > -int define_conditional(cond_expr_t * expr, avrule_t * t, avrule_t * f)
> > +int define_conditional(cond_expr_t * expr, avrule_t * t_list, avrule_t * f_list)
> >  {
> >         cond_expr_t *e;
> >         int depth, booleans, tunables;
> > @@ -3866,15 +3863,15 @@ int define_conditional(cond_expr_t * expr, avrule_t * t, avrule_t * f)
> >                 yyerror("illegal conditional expression");
> >                 return -1;
> >         }
> > -       if (!t) {
> > -               if (!f) {
> > +       if (!t_list) {
> > +               if (!f_list) {
> >                         /* empty is fine, destroy expression and return */
> >                         cond_expr_destroy(expr);
> >                         return 0;
> >                 }
> >                 /* Invert */
> > -               t = f;
> > -               f = 0;
> > +               t_list = f_list;
> > +               f_list = NULL;
> >                 expr = define_cond_expr(COND_NOT, expr, 0);
> >                 if (!expr) {
> >                         yyerror("unable to invert conditional expression");
> > @@ -3940,8 +3937,8 @@ int define_conditional(cond_expr_t * expr, avrule_t * t, avrule_t * f)
> >         /*  use tmp conditional node to partially build new node */
> >         memset(&cn, 0, sizeof(cn));
> >         cn.expr = expr;
> > -       cn.avtrue_list = t;
> > -       cn.avfalse_list = f;
> > +       cn.avtrue_list = t_list;
> > +       cn.avfalse_list = f_list;
> >
> >         /* normalize/precompute expression */
> >         if (cond_normalize_expr(policydbp, &cn) < 0) {
> > @@ -4117,7 +4114,7 @@ static int set_user_roles(role_set_t * set, char *id)
> >  static int parse_categories(char *id, level_datum_t * levdatum, ebitmap_t * cats)
> >  {
> >         cat_datum_t *cdatum;
> > -       int range_start, range_end, i;
> > +       uint32_t range_start, range_end, i;
> >
> >         if (id_has_dot(id)) {
> >                 char *id_start = id;
> > @@ -5527,7 +5524,7 @@ static int define_genfs_context_helper(char *fstype, int has_type)
> >         class_datum_t *cladatum;
> >         char *type = NULL;
> >         const char *sclass;
> > -       int len, len2;
> > +       size_t len, len2;
> >
> >         if (policydbp->target_platform != SEPOL_TARGET_SELINUX) {
> >                 yyerror("genfs not supported for target");
> > diff --git a/checkpolicy/policy_define.h b/checkpolicy/policy_define.h
> > index 075b048d..bcbfe4f3 100644
> > --- a/checkpolicy/policy_define.h
> > +++ b/checkpolicy/policy_define.h
> > @@ -13,7 +13,7 @@
> >  #define FALSE 0
> >
> >  avrule_t *define_cond_compute_type(int which);
> > -avrule_t *define_cond_pol_list(avrule_t *avlist, avrule_t *stmt);
> > +avrule_t *define_cond_pol_list(avrule_t *avlist, avrule_t *sl);
> >  avrule_t *define_cond_te_avtab(int which);
> >  avrule_t *define_cond_filename_trans(void);
> >  cond_expr_t *define_cond_expr(uint32_t expr_type, void *arg1, void* arg2);
> > --
> > 2.43.0
> >
> >
diff mbox series

Patch

diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index cd49cae3..79d67a78 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -44,7 +44,6 @@ 
 #define IPPROTO_SCTP 132
 #endif
 #include <arpa/inet.h>
-#include <stdlib.h>
 #include <limits.h>
 #include <inttypes.h>
 #include <ctype.h>
@@ -1096,7 +1095,7 @@  int define_level(void)
 
 	while ((id = queue_remove(id_queue))) {
 		cat_datum_t *cdatum;
-		int range_start, range_end, i;
+		uint32_t range_start, range_end, i;
 
 		if (id_has_dot(id)) {
 			char *id_start = id;
@@ -1932,7 +1931,7 @@  error:
 	return -1;
 }
 
-static int avrule_merge_ioctls(struct av_ioctl_range_list **rangehead)
+static void avrule_merge_ioctls(struct av_ioctl_range_list **rangehead)
 {
 	struct av_ioctl_range_list *r, *tmp;
 	r = *rangehead;
@@ -1949,7 +1948,6 @@  static int avrule_merge_ioctls(struct av_ioctl_range_list **rangehead)
 		}
 		r = r->next;
 	}
-	return 0;
 }
 
 static int avrule_read_ioctls(struct av_ioctl_range_list **rangehead)
@@ -2070,8 +2068,7 @@  static int avrule_ioctl_ranges(struct av_ioctl_range_list **rangelist)
 	/* sort and merge the input ioctls */
 	if (avrule_sort_ioctls(&rangehead))
 		return -1;
-	if (avrule_merge_ioctls(&rangehead))
-		return -1;
+	avrule_merge_ioctls(&rangehead);
 	/* flip ranges if these are omitted */
 	if (omit) {
 		if (avrule_omit_ioctls(&rangehead))
@@ -3854,7 +3851,7 @@  uintptr_t define_cexpr(uint32_t expr_type, uintptr_t arg1, uintptr_t arg2)
 	return 0;
 }
 
-int define_conditional(cond_expr_t * expr, avrule_t * t, avrule_t * f)
+int define_conditional(cond_expr_t * expr, avrule_t * t_list, avrule_t * f_list)
 {
 	cond_expr_t *e;
 	int depth, booleans, tunables;
@@ -3866,15 +3863,15 @@  int define_conditional(cond_expr_t * expr, avrule_t * t, avrule_t * f)
 		yyerror("illegal conditional expression");
 		return -1;
 	}
-	if (!t) {
-		if (!f) {
+	if (!t_list) {
+		if (!f_list) {
 			/* empty is fine, destroy expression and return */
 			cond_expr_destroy(expr);
 			return 0;
 		}
 		/* Invert */
-		t = f;
-		f = 0;
+		t_list = f_list;
+		f_list = NULL;
 		expr = define_cond_expr(COND_NOT, expr, 0);
 		if (!expr) {
 			yyerror("unable to invert conditional expression");
@@ -3940,8 +3937,8 @@  int define_conditional(cond_expr_t * expr, avrule_t * t, avrule_t * f)
 	/*  use tmp conditional node to partially build new node */
 	memset(&cn, 0, sizeof(cn));
 	cn.expr = expr;
-	cn.avtrue_list = t;
-	cn.avfalse_list = f;
+	cn.avtrue_list = t_list;
+	cn.avfalse_list = f_list;
 
 	/* normalize/precompute expression */
 	if (cond_normalize_expr(policydbp, &cn) < 0) {
@@ -4117,7 +4114,7 @@  static int set_user_roles(role_set_t * set, char *id)
 static int parse_categories(char *id, level_datum_t * levdatum, ebitmap_t * cats)
 {
 	cat_datum_t *cdatum;
-	int range_start, range_end, i;
+	uint32_t range_start, range_end, i;
 
 	if (id_has_dot(id)) {
 		char *id_start = id;
@@ -5527,7 +5524,7 @@  static int define_genfs_context_helper(char *fstype, int has_type)
 	class_datum_t *cladatum;
 	char *type = NULL;
 	const char *sclass;
-	int len, len2;
+	size_t len, len2;
 
 	if (policydbp->target_platform != SEPOL_TARGET_SELINUX) {
 		yyerror("genfs not supported for target");
diff --git a/checkpolicy/policy_define.h b/checkpolicy/policy_define.h
index 075b048d..bcbfe4f3 100644
--- a/checkpolicy/policy_define.h
+++ b/checkpolicy/policy_define.h
@@ -13,7 +13,7 @@ 
 #define FALSE 0
 
 avrule_t *define_cond_compute_type(int which);
-avrule_t *define_cond_pol_list(avrule_t *avlist, avrule_t *stmt);
+avrule_t *define_cond_pol_list(avrule_t *avlist, avrule_t *sl);
 avrule_t *define_cond_te_avtab(int which);
 avrule_t *define_cond_filename_trans(void);
 cond_expr_t *define_cond_expr(uint32_t expr_type, void *arg1, void* arg2);