[PATCH 01/13] checkpolicy: pass CFLAGS at link stage

[PATCH 01/13] checkpolicy: pass CFLAGS at link stage

[Christian Göttsche]

Pass CFLAGS when invoking CC at link time, it might contain optimization
or sanitizer flags required for linking.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/Makefile      | 4 ++--
 checkpolicy/test/Makefile | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/checkpolicy/Makefile b/checkpolicy/Makefile
index 0d282ef9..be63c018 100644
--- a/checkpolicy/Makefile
+++ b/checkpolicy/Makefile
@@ -30,10 +30,10 @@ all:  $(TARGETS)
 	$(MAKE) -C test
 
 checkpolicy: $(CHECKPOLOBJS) $(LIBSEPOLA)
-	$(CC) -o $@ $^ $(LDFLAGS) $(LDLIBS_LIBSEPOLA)
+	$(CC) $(CFLAGS) -o $@ $^ $(LDFLAGS) $(LDLIBS_LIBSEPOLA)
 
 checkmodule: $(CHECKMODOBJS) $(LIBSEPOLA)
-	$(CC) -o $@ $^ $(LDFLAGS) $(LDLIBS_LIBSEPOLA)
+	$(CC) $(CFLAGS) -o $@ $^ $(LDFLAGS) $(LDLIBS_LIBSEPOLA)
 
 %.o: %.c 
 	$(CC) $(CFLAGS) -o $@ -c $<
diff --git a/checkpolicy/test/Makefile b/checkpolicy/test/Makefile
index 89e7557c..e2a332b5 100644
--- a/checkpolicy/test/Makefile
+++ b/checkpolicy/test/Makefile
@@ -13,10 +13,10 @@ endif
 all: dispol dismod
 
 dispol: dispol.o $(LIBSEPOLA)
-	$(CC) $(LDFLAGS) -o $@ $^ $(LDLIBS_LIBSEPOLA)
+	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ $(LDLIBS_LIBSEPOLA)
 
 dismod: dismod.o $(LIBSEPOLA)
-	$(CC) $(LDFLAGS) -o $@ $^ $(LDLIBS_LIBSEPOLA)
+	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ $(LDLIBS_LIBSEPOLA)
 
 clean:
 	-rm -f dispol dismod *.o 
-- 
2.32.0

[PATCH 02/13] checkpolicy: drop -pipe compile option

[Christian Göttsche]

The compiler option -pipe does not affect the generated code; it affects
whether the compiler uses temporary files or pipes. As the benefit might
vary from system to system usually its up to the packager or build
framework to set it.
Also these are the only places where the flag is used.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/Makefile      | 2 +-
 checkpolicy/test/Makefile | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/checkpolicy/Makefile b/checkpolicy/Makefile
index be63c018..f9e1fc7c 100644
--- a/checkpolicy/Makefile
+++ b/checkpolicy/Makefile
@@ -10,7 +10,7 @@ TARGETS = checkpolicy checkmodule
 LEX = flex
 YACC = bison -y
 
-CFLAGS ?= -g -Wall -Werror -Wshadow -O2 -pipe -fno-strict-aliasing
+CFLAGS ?= -g -Wall -Werror -Wshadow -O2 -fno-strict-aliasing
 
 # If no specific libsepol.a is specified, fall back on LDFLAGS search path
 # Otherwise, as $(LIBSEPOLA) already appears in the dependencies, there
diff --git a/checkpolicy/test/Makefile b/checkpolicy/test/Makefile
index e2a332b5..8e5d16b3 100644
--- a/checkpolicy/test/Makefile
+++ b/checkpolicy/test/Makefile
@@ -1,7 +1,7 @@
 #
 # Makefile for building the dispol program
 #
-CFLAGS ?= -g -Wall -W -Werror -O2 -pipe
+CFLAGS ?= -g -Wall -W -Werror -O2
 
 # If no specific libsepol.a is specified, fall back on LDFLAGS search path
 # Otherwise, as $(LIBSEPOLA) already appears in the dependencies, there
-- 
2.32.0

[PATCH 03/13] checkpolicy: simplify assignment

[Christian Göttsche]

checkpolicy.c:504:20: style: The statement 'if (policyvers!=n) policyvers=n' is logically equivalent to 'policyvers=n'. [duplicateConditionalAssign]
    if (policyvers != n)
                   ^
checkpolicy.c:505:17: note: Assignment 'policyvers=n'
     policyvers = n;
                ^
checkpolicy.c:504:20: note: Condition 'policyvers!=n' is redundant
    if (policyvers != n)
                   ^

Found by Cppcheck

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/checkpolicy.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/checkpolicy/checkpolicy.c b/checkpolicy/checkpolicy.c
index acf1eac4..9627275f 100644
--- a/checkpolicy/checkpolicy.c
+++ b/checkpolicy/checkpolicy.c
@@ -501,8 +501,7 @@ int main(int argc, char **argv)
 					usage(argv[0]);
 					exit(1);
 				}
-				if (policyvers != n)
-					policyvers = n;
+				policyvers = n;
 				break;
 			}
 		case 'E':
-- 
2.32.0

[PATCH 04/13] checkpolicy: drop dead condition

[Christian Göttsche]

The variable `id` is guaranteed to be non-NULL due to the preceding
while condition.

    policy_define.c:1171:7: style: Condition '!id' is always false [knownConditionTrueFalse]
      if (!id) {
          ^
    policy_define.c:1170:13: note: Assuming that condition 'id=queue_remove(id_queue)' is not redundant
     while ((id = queue_remove(id_queue))) {
                ^
    policy_define.c:1171:7: note: Condition '!id' is always false
      if (!id) {
          ^

Found by Cppcheck.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/policy_define.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index 16234f31..7eff747a 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -1168,11 +1168,6 @@ int expand_attrib(void)
 
 	ebitmap_init(&attrs);
 	while ((id = queue_remove(id_queue))) {
-		if (!id) {
-			yyerror("No attribute name for expandattribute statement?");
-			goto exit;
-		}
-
 		if (!is_id_in_scope(SYM_TYPES, id)) {
 			yyerror2("attribute %s is not within scope", id);
 			goto exit;
-- 
2.32.0

[PATCH 05/13] checkpolicy: use correct format specifier for unsigned

[Christian Göttsche]

    test/dispol.c:288:4: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
       snprintf(buf, sizeof(buf), "unknown (%d)", i);
       ^
    test/dismod.c:830:4: warning: %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
       snprintf(buf, sizeof(buf), "unknown (%d)", i);
       ^

Found by Cppcheck.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/test/dismod.c | 2 +-
 checkpolicy/test/dispol.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/checkpolicy/test/dismod.c b/checkpolicy/test/dismod.c
index 3408e9b6..fadbc8d1 100644
--- a/checkpolicy/test/dismod.c
+++ b/checkpolicy/test/dismod.c
@@ -827,7 +827,7 @@ static void display_policycaps(policydb_t * p, FILE * fp)
 	ebitmap_for_each_positive_bit(&p->policycaps, node, i) {
 		capname = sepol_polcap_getname(i);
 		if (capname == NULL) {
-			snprintf(buf, sizeof(buf), "unknown (%d)", i);
+			snprintf(buf, sizeof(buf), "unknown (%u)", i);
 			capname = buf;
 		}
 		fprintf(fp, "\t%s\n", capname);
diff --git a/checkpolicy/test/dispol.c b/checkpolicy/test/dispol.c
index 8785b725..37f71842 100644
--- a/checkpolicy/test/dispol.c
+++ b/checkpolicy/test/dispol.c
@@ -285,7 +285,7 @@ static void display_policycaps(policydb_t * p, FILE * fp)
 	ebitmap_for_each_positive_bit(&p->policycaps, node, i) {
 		capname = sepol_polcap_getname(i);
 		if (capname == NULL) {
-			snprintf(buf, sizeof(buf), "unknown (%d)", i);
+			snprintf(buf, sizeof(buf), "unknown (%u)", i);
 			capname = buf;
 		}
 		fprintf(fp, "\t%s\n", capname);
-- 
2.32.0

[PATCH 06/13] checkpolicy: follow declaration-after-statement

[Christian Göttsche]

Follow the project style of no declaration after statement.

Found by the GCC warning -Wdeclaration-after-statement.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/checkmodule.c   | 6 ++++--
 checkpolicy/policy_define.c | 3 ++-
 checkpolicy/test/dismod.c   | 2 +-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/checkpolicy/checkmodule.c b/checkpolicy/checkmodule.c
index 40d0ec99..316b2898 100644
--- a/checkpolicy/checkmodule.c
+++ b/checkpolicy/checkmodule.c
@@ -288,14 +288,16 @@ int main(int argc, char **argv)
 	}
 
 	if (policy_type != POLICY_BASE && outfile) {
+		char *out_name;
+		char *separator;
 		char *mod_name = modpolicydb.name;
 		char *out_path = strdup(outfile);
 		if (out_path == NULL) {
 			fprintf(stderr, "%s:  out of memory\n", argv[0]);
 			exit(1);
 		}
-		char *out_name = basename(out_path);
-		char *separator = strrchr(out_name, '.');
+		out_name = basename(out_path);
+		separator = strrchr(out_name, '.');
 		if (separator) {
 			*separator = '\0';
 		}
diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index 7eff747a..22218c07 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -1904,8 +1904,9 @@ int avrule_read_ioctls(struct av_ioctl_range_list **rangehead)
 {
 	char *id;
 	struct av_ioctl_range_list *rnew, *r = NULL;
-	*rangehead = NULL;
 	uint8_t omit = 0;
+	
+	*rangehead = NULL;	
 
 	/* read in all the ioctl commands */
 	while ((id = queue_remove(id_queue))) {
diff --git a/checkpolicy/test/dismod.c b/checkpolicy/test/dismod.c
index fadbc8d1..b1b96115 100644
--- a/checkpolicy/test/dismod.c
+++ b/checkpolicy/test/dismod.c
@@ -697,8 +697,8 @@ int display_avblock(int field, policydb_t * policy,
 {
 	avrule_block_t *block = policydb.global;
 	while (block != NULL) {
-		fprintf(out_fp, "--- begin avrule block ---\n");
 		avrule_decl_t *decl = block->branch_list;
+		fprintf(out_fp, "--- begin avrule block ---\n");
 		while (decl != NULL) {
 			if (display_avdecl(decl, field, policy, out_fp)) {
 				return -1;
-- 
2.32.0

[PATCH 07/13] checkpolicy: remove dead assignments

[Christian Göttsche]

The variable `cladatum` is otherwise always assigned before used, so
these two assignments without a follow up usages are not needed.

Found by clang-analyzer.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/checkpolicy.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/checkpolicy/checkpolicy.c b/checkpolicy/checkpolicy.c
index 9627275f..c88ca542 100644
--- a/checkpolicy/checkpolicy.c
+++ b/checkpolicy/checkpolicy.c
@@ -1176,8 +1176,6 @@ int main(int argc, char **argv)
 					printf("\nNo such class.\n");
 					break;
 				}
-				cladatum =
-				    policydb.class_val_to_struct[tclass - 1];
 			} else {
 				ans[strlen(ans) - 1] = 0;
 				cladatum =
@@ -1229,8 +1227,6 @@ int main(int argc, char **argv)
 					printf("\nNo such class.\n");
 					break;
 				}
-				cladatum =
-				    policydb.class_val_to_struct[tclass - 1];
 			} else {
 				ans[strlen(ans) - 1] = 0;
 				cladatum =
-- 
2.32.0

[PATCH 08/13] checkpolicy: check before potential NULL dereference

[Christian Göttsche]

    policy_define.c: In function ‘define_te_avtab_extended_perms’:
    policy_define.c:1946:17: error: potential null pointer dereference [-Werror=null-dereference]
     1946 |         r->omit = omit;
          |                 ^

In the case of `r` being NULL, avrule_read_ioctls() would return
with its parameter `rangehead` being a pointer to NULL, which is
considered a failure in its caller `avrule_ioctl_ranges`.
So it is not necessary to alter the return value.

Found by GCC 11 with LTO enabled.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/policy_define.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index 22218c07..370ff8e3 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -1943,7 +1943,9 @@ int avrule_read_ioctls(struct av_ioctl_range_list **rangehead)
 		}
 	}
 	r = *rangehead;
-	r->omit = omit;
+	if (r) {
+		r->omit = omit;
+	}
 	return 0;
 error:
 	yyerror("out of memory");
-- 
2.32.0

[PATCH 09/13] checkpolicy: avoid potential use of uninitialized variable

[Christian Göttsche]

    checkpolicy.c: In function ‘main’:
    checkpolicy.c:1000:25: error: ‘tsid’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     1000 |                         printf("if_sid %d default_msg_sid %d\n", ssid, tsid);
          |                         ^

    checkpolicy.c: In function ‘main’:
    checkpolicy.c:971:25: error: ‘tsid’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
      971 |                         printf("fs_sid %d default_file_sid %d\n", ssid, tsid);
          |                         ^

Found by GCC 11 with LTO enabled.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/checkpolicy.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/checkpolicy/checkpolicy.c b/checkpolicy/checkpolicy.c
index c88ca542..04f35eda 100644
--- a/checkpolicy/checkpolicy.c
+++ b/checkpolicy/checkpolicy.c
@@ -967,8 +967,12 @@ int main(int argc, char **argv)
 			printf("fs kdevname?  ");
 			FGETS(ans, sizeof(ans), stdin);
 			ans[strlen(ans) - 1] = 0;
-			sepol_fs_sid(ans, &ssid, &tsid);
-			printf("fs_sid %d default_file_sid %d\n", ssid, tsid);
+			ret = sepol_fs_sid(ans, &ssid, &tsid);
+			if (ret) {
+				printf("unknown fs kdevname\n");
+			} else {
+				printf("fs_sid %d default_file_sid %d\n", ssid, tsid);
+			}
 			break;
 		case '9':
 			printf("protocol?  ");
@@ -996,8 +1000,12 @@ int main(int argc, char **argv)
 			printf("netif name?  ");
 			FGETS(ans, sizeof(ans), stdin);
 			ans[strlen(ans) - 1] = 0;
-			sepol_netif_sid(ans, &ssid, &tsid);
-			printf("if_sid %d default_msg_sid %d\n", ssid, tsid);
+			ret = sepol_netif_sid(ans, &ssid, &tsid);
+			if (ret) {
+				printf("unknown name\n");
+			} else {
+				printf("if_sid %d default_msg_sid %d\n", ssid, tsid);
+			}
 			break;
 		case 'b':{
 				char *p;
-- 
2.32.0

[PATCH 10/13] checkpolicy: drop redundant cast to the same type

[Christian Göttsche]

Found by clang-tidy.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/policy_define.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index 370ff8e3..462e3994 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -1796,7 +1796,7 @@ int define_bool_tunable(int is_tunable)
 		return -1;
 	}
 
-	datum->state = (int)(bool_value[0] == 'T') ? 1 : 0;
+	datum->state = (bool_value[0] == 'T') ? 1 : 0;
 	free(bool_value);
 	return 0;
       cleanup:
-- 
2.32.0

[PATCH 11/13] checkpolicy: parse_util drop unused declaration

[Christian Göttsche]

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/parse_util.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/checkpolicy/parse_util.c b/checkpolicy/parse_util.c
index f2809b48..1795e93c 100644
--- a/checkpolicy/parse_util.c
+++ b/checkpolicy/parse_util.c
@@ -28,7 +28,6 @@ extern int yyparse(void);
 extern void yyrestart(FILE *);
 extern queue_t id_queue;
 extern unsigned int policydb_errors;
-extern unsigned long policydb_lineno;
 extern policydb_t *policydbp;
 extern int mlspol;
 extern void set_source_file(const char *name);
-- 
2.32.0

[PATCH 12/13] checkpolicy/test: mark file local functions static

[Christian Göttsche]

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/test/dismod.c | 36 ++++++++++++++++++------------------
 checkpolicy/test/dispol.c | 22 +++++++++++-----------
 2 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/checkpolicy/test/dismod.c b/checkpolicy/test/dismod.c
index b1b96115..90c29318 100644
--- a/checkpolicy/test/dismod.c
+++ b/checkpolicy/test/dismod.c
@@ -111,7 +111,7 @@ static void display_id(policydb_t * p, FILE * fp, uint32_t symbol_type,
 	}
 }
 
-int display_type_set(type_set_t * set, uint32_t flags, policydb_t * policy,
+static int display_type_set(type_set_t * set, uint32_t flags, policydb_t * policy,
 		     FILE * fp)
 {
 	unsigned int i, num_types;
@@ -175,7 +175,7 @@ int display_type_set(type_set_t * set, uint32_t flags, policydb_t * policy,
 	return 0;
 }
 
-int display_mod_role_set(role_set_t * roles, policydb_t * p, FILE * fp)
+static int display_mod_role_set(role_set_t * roles, policydb_t * p, FILE * fp)
 {
 	unsigned int i, num = 0;
 
@@ -210,7 +210,7 @@ int display_mod_role_set(role_set_t * roles, policydb_t * p, FILE * fp)
 
 }
 
-int display_avrule(avrule_t * avrule, policydb_t * policy,
+static int display_avrule(avrule_t * avrule, policydb_t * policy,
 		   FILE * fp)
 {
 	class_perm_node_t *cur;
@@ -313,7 +313,7 @@ int display_avrule(avrule_t * avrule, policydb_t * policy,
 	return 0;
 }
 
-int display_type_callback(hashtab_key_t key, hashtab_datum_t datum, void *data)
+static int display_type_callback(hashtab_key_t key, hashtab_datum_t datum, void *data)
 {
 	type_datum_t *type;
 	FILE *fp;
@@ -355,14 +355,14 @@ int display_type_callback(hashtab_key_t key, hashtab_datum_t datum, void *data)
 	return 0;
 }
 
-int display_types(policydb_t * p, FILE * fp)
+static int display_types(policydb_t * p, FILE * fp)
 {
 	if (hashtab_map(p->p_types.table, display_type_callback, fp))
 		return -1;
 	return 0;
 }
 
-int display_users(policydb_t * p, FILE * fp)
+static int display_users(policydb_t * p, FILE * fp)
 {
 	unsigned int i, j;
 	ebitmap_t *bitmap;
@@ -381,7 +381,7 @@ int display_users(policydb_t * p, FILE * fp)
 	return 0;
 }
 
-int display_bools(policydb_t * p, FILE * fp)
+static int display_bools(policydb_t * p, FILE * fp)
 {
 	unsigned int i;
 
@@ -392,7 +392,7 @@ int display_bools(policydb_t * p, FILE * fp)
 	return 0;
 }
 
-void display_expr(policydb_t * p, cond_expr_t * exp, FILE * fp)
+static void display_expr(policydb_t * p, cond_expr_t * exp, FILE * fp)
 {
 
 	cond_expr_t *cur;
@@ -427,14 +427,14 @@ void display_expr(policydb_t * p, cond_expr_t * exp, FILE * fp)
 	}
 }
 
-void display_policycon(FILE * fp)
+static void display_policycon(FILE * fp)
 {
 	/* There was an attempt to implement this at one time.  Look through
 	 * git history to find it. */
 	fprintf(fp, "Sorry, not implemented\n");
 }
 
-void display_initial_sids(policydb_t * p, FILE * fp)
+static void display_initial_sids(policydb_t * p, FILE * fp)
 {
 	ocontext_t *cur;
 	char *user, *role, *type;
@@ -459,7 +459,7 @@ void display_initial_sids(policydb_t * p, FILE * fp)
 #endif
 }
 
-void display_class_set(ebitmap_t *classes, policydb_t *p, FILE *fp)
+static void display_class_set(ebitmap_t *classes, policydb_t *p, FILE *fp)
 {
 	unsigned int i, num = 0;
 
@@ -482,7 +482,7 @@ void display_class_set(ebitmap_t *classes, policydb_t *p, FILE *fp)
 		fprintf(fp, " }");
 }
 
-void display_role_trans(role_trans_rule_t * tr, policydb_t * p, FILE * fp)
+static void display_role_trans(role_trans_rule_t * tr, policydb_t * p, FILE * fp)
 {
 	for (; tr; tr = tr->next) {
 		fprintf(fp, "role transition ");
@@ -495,7 +495,7 @@ void display_role_trans(role_trans_rule_t * tr, policydb_t * p, FILE * fp)
 	}
 }
 
-void display_role_allow(role_allow_rule_t * ra, policydb_t * p, FILE * fp)
+static void display_role_allow(role_allow_rule_t * ra, policydb_t * p, FILE * fp)
 {
 	for (; ra; ra = ra->next) {
 		fprintf(fp, "role allow ");
@@ -517,7 +517,7 @@ static void display_filename_trans(filename_trans_rule_t * tr, policydb_t * p, F
 	}
 }
 
-int role_display_callback(hashtab_key_t key __attribute__((unused)),
+static int role_display_callback(hashtab_key_t key __attribute__((unused)),
 			  hashtab_datum_t datum, void *data)
 {
 	role_datum_t *role;
@@ -611,7 +611,7 @@ int change_bool(char *name, int state, policydb_t * p, FILE * fp)
 }
 #endif
 
-int display_avdecl(avrule_decl_t * decl, int field,
+static int display_avdecl(avrule_decl_t * decl, int field,
 		   policydb_t * policy, FILE * out_fp)
 {
 	fprintf(out_fp, "decl %u:%s\n", decl->decl_id,
@@ -692,7 +692,7 @@ int display_avdecl(avrule_decl_t * decl, int field,
 	return 0;		/* should never get here */
 }
 
-int display_avblock(int field, policydb_t * policy,
+static int display_avblock(int field, policydb_t * policy,
 		    FILE * out_fp)
 {
 	avrule_block_t *block = policydb.global;
@@ -710,7 +710,7 @@ int display_avblock(int field, policydb_t * policy,
 	return 0;
 }
 
-int display_handle_unknown(policydb_t * p, FILE * out_fp)
+static int display_handle_unknown(policydb_t * p, FILE * out_fp)
 {
 	if (p->handle_unknown == ALLOW_UNKNOWN)
 		fprintf(out_fp, "Allow unknown classes and perms\n");
@@ -834,7 +834,7 @@ static void display_policycaps(policydb_t * p, FILE * fp)
 	}
 }
 
-int menu(void)
+static int menu(void)
 {
 	printf("\nSelect a command:\n");
 	printf("1)  display unconditional AVTAB\n");
diff --git a/checkpolicy/test/dispol.c b/checkpolicy/test/dispol.c
index 37f71842..8ddefb04 100644
--- a/checkpolicy/test/dispol.c
+++ b/checkpolicy/test/dispol.c
@@ -42,7 +42,7 @@ static __attribute__((__noreturn__)) void usage(const char *progname)
 	exit(1);
 }
 
-int render_access_mask(uint32_t mask, avtab_key_t * key, policydb_t * p,
+static int render_access_mask(uint32_t mask, avtab_key_t * key, policydb_t * p,
 		       FILE * fp)
 {
 	char *perm;
@@ -54,13 +54,13 @@ int render_access_mask(uint32_t mask, avtab_key_t * key, policydb_t * p,
 	return 0;
 }
 
-int render_type(uint32_t type, policydb_t * p, FILE * fp)
+static int render_type(uint32_t type, policydb_t * p, FILE * fp)
 {
 	fprintf(fp, "%s", p->p_type_val_to_name[type - 1]);
 	return 0;
 }
 
-int render_key(avtab_key_t * key, policydb_t * p, FILE * fp)
+static int render_key(avtab_key_t * key, policydb_t * p, FILE * fp)
 {
 	char *stype, *ttype, *tclass;
 	stype = p->p_type_val_to_name[key->source_type - 1];
@@ -84,7 +84,7 @@ int render_key(avtab_key_t * key, policydb_t * p, FILE * fp)
 #define RENDER_DISABLED		0x0004
 #define RENDER_CONDITIONAL	(RENDER_ENABLED|RENDER_DISABLED)
 
-int render_av_rule(avtab_key_t * key, avtab_datum_t * datum, uint32_t what,
+static int render_av_rule(avtab_key_t * key, avtab_datum_t * datum, uint32_t what,
 		   policydb_t * p, FILE * fp)
 {
 	if (!(what & RENDER_UNCONDITIONAL)) {
@@ -163,7 +163,7 @@ int render_av_rule(avtab_key_t * key, avtab_datum_t * datum, uint32_t what,
 	return 0;
 }
 
-int display_avtab(avtab_t * a, uint32_t what, policydb_t * p, FILE * fp)
+static int display_avtab(avtab_t * a, uint32_t what, policydb_t * p, FILE * fp)
 {
 	unsigned int i;
 	avtab_ptr_t cur;
@@ -178,7 +178,7 @@ int display_avtab(avtab_t * a, uint32_t what, policydb_t * p, FILE * fp)
 	return 0;
 }
 
-int display_bools(policydb_t * p, FILE * fp)
+static int display_bools(policydb_t * p, FILE * fp)
 {
 	unsigned int i;
 
@@ -189,7 +189,7 @@ int display_bools(policydb_t * p, FILE * fp)
 	return 0;
 }
 
-void display_expr(policydb_t * p, cond_expr_t * exp, FILE * fp)
+static void display_expr(policydb_t * p, cond_expr_t * exp, FILE * fp)
 {
 
 	cond_expr_t *cur;
@@ -224,7 +224,7 @@ void display_expr(policydb_t * p, cond_expr_t * exp, FILE * fp)
 	}
 }
 
-int display_cond_expressions(policydb_t * p, FILE * fp)
+static int display_cond_expressions(policydb_t * p, FILE * fp)
 {
 	cond_node_t *cur;
 	cond_av_list_t *av_cur;
@@ -249,7 +249,7 @@ int display_cond_expressions(policydb_t * p, FILE * fp)
 	return 0;
 }
 
-int display_handle_unknown(policydb_t * p, FILE * out_fp)
+static int display_handle_unknown(policydb_t * p, FILE * out_fp)
 {
 	if (p->handle_unknown == ALLOW_UNKNOWN)
 		fprintf(out_fp, "Allow unknown classes and permissions\n");
@@ -260,7 +260,7 @@ int display_handle_unknown(policydb_t * p, FILE * out_fp)
 	return 0;
 }
 
-int change_bool(char *name, int state, policydb_t * p, FILE * fp)
+static int change_bool(char *name, int state, policydb_t * p, FILE * fp)
 {
 	cond_bool_datum_t *bool;
 
@@ -368,7 +368,7 @@ static void display_filename_trans(policydb_t *p, FILE *fp)
 	hashtab_map(p->filename_trans, filenametr_display, &args);
 }
 
-int menu(void)
+static int menu(void)
 {
 	printf("\nSelect a command:\n");
 	printf("1)  display unconditional AVTAB\n");
-- 
2.32.0

[PATCH 13/13] checkpolicy: mark read-only parameters in policy define const

[Christian Göttsche]

Make it more obvious which parameters are read-only and not being
modified and allow callers to pass const pointers.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/policy_define.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
index 462e3994..a8aa615e 100644
--- a/checkpolicy/policy_define.c
+++ b/checkpolicy/policy_define.c
@@ -77,7 +77,7 @@ extern int yyerror(const char *msg);
 #define ERRORMSG_LEN 255
 static char errormsg[ERRORMSG_LEN + 1] = {0};
 
-static int id_has_dot(char *id);
+static int id_has_dot(const char *id);
 static int parse_security_context(context_struct_t *c);
 
 /* initialize all of the state variables for the scanner/parser */
@@ -141,7 +141,7 @@ int insert_id(const char *id, int push)
 
 /* If the identifier has a dot within it and that its first character
    is not a dot then return 1, else return 0. */
-static int id_has_dot(char *id)
+static int id_has_dot(const char *id)
 {
 	if (strchr(id, '.') >= id + 1) {
 		return 1;
@@ -2172,7 +2172,7 @@ void avrule_xperm_setrangebits(uint16_t low, uint16_t high,
 	}
 }
 
-int avrule_xperms_used(av_extended_perms_t *xperms)
+int avrule_xperms_used(const av_extended_perms_t *xperms)
 {
 	unsigned int i;
 
@@ -2347,7 +2347,7 @@ unsigned int xperms_for_each_bit(unsigned int *bit, av_extended_perms_t *xperms)
 	return 0;
 }
 
-int avrule_cpy(avrule_t *dest, avrule_t *src)
+int avrule_cpy(avrule_t *dest, const avrule_t *src)
 {
 	class_perm_node_t *src_perms;
 	class_perm_node_t *dest_perms, *dest_tail;
@@ -2395,7 +2395,7 @@ int avrule_cpy(avrule_t *dest, avrule_t *src)
 	return 0;
 }
 
-int define_te_avtab_ioctl(avrule_t *avrule_template)
+int define_te_avtab_ioctl(const avrule_t *avrule_template)
 {
 	avrule_t *avrule;
 	struct av_ioctl_range_list *rangelist;
@@ -3444,9 +3444,10 @@ bad:
 	return -1;
 }
 
-static constraint_expr_t *constraint_expr_clone(constraint_expr_t * expr)
+static constraint_expr_t *constraint_expr_clone(const constraint_expr_t * expr)
 {
-	constraint_expr_t *h = NULL, *l = NULL, *e, *newe;
+	constraint_expr_t *h = NULL, *l = NULL, *newe;
+	const constraint_expr_t *e;
 	for (e = expr; e; e = e->next) {
 		newe = malloc(sizeof(*newe));
 		if (!newe)
-- 
2.32.0

Re: [PATCH 06/13] checkpolicy: follow declaration-after-statement

[Nicolas Iooss]

On Tue, Jul 6, 2021 at 7:54 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Follow the project style of no declaration after statement.
>
> Found by the GCC warning -Wdeclaration-after-statement.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  checkpolicy/checkmodule.c   | 6 ++++--
>  checkpolicy/policy_define.c | 3 ++-
>  checkpolicy/test/dismod.c   | 2 +-
>  3 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/checkpolicy/checkmodule.c b/checkpolicy/checkmodule.c
> index 40d0ec99..316b2898 100644
> --- a/checkpolicy/checkmodule.c
> +++ b/checkpolicy/checkmodule.c
> @@ -288,14 +288,16 @@ int main(int argc, char **argv)
>         }
>
>         if (policy_type != POLICY_BASE && outfile) {
> +               char *out_name;
> +               char *separator;
>                 char *mod_name = modpolicydb.name;
>                 char *out_path = strdup(outfile);
>                 if (out_path == NULL) {
>                         fprintf(stderr, "%s:  out of memory\n", argv[0]);
>                         exit(1);
>                 }
> -               char *out_name = basename(out_path);
> -               char *separator = strrchr(out_name, '.');
> +               out_name = basename(out_path);
> +               separator = strrchr(out_name, '.');
>                 if (separator) {
>                         *separator = '\0';
>                 }
> diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
> index 7eff747a..22218c07 100644
> --- a/checkpolicy/policy_define.c
> +++ b/checkpolicy/policy_define.c
> @@ -1904,8 +1904,9 @@ int avrule_read_ioctls(struct av_ioctl_range_list **rangehead)
>  {
>         char *id;
>         struct av_ioctl_range_list *rnew, *r = NULL;
> -       *rangehead = NULL;
>         uint8_t omit = 0;
> +
> +       *rangehead = NULL;

Hello,
All the patches of this series look good to me. But here, you
introduced trailing tabs, which is reported by "git am" when applying
the patch:

        .git/rebase-apply/patch:40: trailing whitespace.

        .git/rebase-apply/patch:41: trailing whitespace.
                *rangehead = NULL;
        warning: 2 lines add whitespace errors.

Anyway I can remove these tabs when applying this patch.

Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>

If nobody else has comments, I will apply them tomorrow.
Thanks!
Nicolas

Re: [PATCH 06/13] checkpolicy: follow declaration-after-statement

[Nicolas Iooss]

On Mon, Jul 12, 2021 at 9:13 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Tue, Jul 6, 2021 at 7:54 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > Follow the project style of no declaration after statement.
> >
> > Found by the GCC warning -Wdeclaration-after-statement.
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > ---
> >  checkpolicy/checkmodule.c   | 6 ++++--
> >  checkpolicy/policy_define.c | 3 ++-
> >  checkpolicy/test/dismod.c   | 2 +-
> >  3 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/checkpolicy/checkmodule.c b/checkpolicy/checkmodule.c
> > index 40d0ec99..316b2898 100644
> > --- a/checkpolicy/checkmodule.c
> > +++ b/checkpolicy/checkmodule.c
> > @@ -288,14 +288,16 @@ int main(int argc, char **argv)
> >         }
> >
> >         if (policy_type != POLICY_BASE && outfile) {
> > +               char *out_name;
> > +               char *separator;
> >                 char *mod_name = modpolicydb.name;
> >                 char *out_path = strdup(outfile);
> >                 if (out_path == NULL) {
> >                         fprintf(stderr, "%s:  out of memory\n", argv[0]);
> >                         exit(1);
> >                 }
> > -               char *out_name = basename(out_path);
> > -               char *separator = strrchr(out_name, '.');
> > +               out_name = basename(out_path);
> > +               separator = strrchr(out_name, '.');
> >                 if (separator) {
> >                         *separator = '\0';
> >                 }
> > diff --git a/checkpolicy/policy_define.c b/checkpolicy/policy_define.c
> > index 7eff747a..22218c07 100644
> > --- a/checkpolicy/policy_define.c
> > +++ b/checkpolicy/policy_define.c
> > @@ -1904,8 +1904,9 @@ int avrule_read_ioctls(struct av_ioctl_range_list **rangehead)
> >  {
> >         char *id;
> >         struct av_ioctl_range_list *rnew, *r = NULL;
> > -       *rangehead = NULL;
> >         uint8_t omit = 0;
> > +
> > +       *rangehead = NULL;
>
> Hello,
> All the patches of this series look good to me. But here, you
> introduced trailing tabs, which is reported by "git am" when applying
> the patch:
>
>         .git/rebase-apply/patch:40: trailing whitespace.
>
>         .git/rebase-apply/patch:41: trailing whitespace.
>                 *rangehead = NULL;
>         warning: 2 lines add whitespace errors.
>
> Anyway I can remove these tabs when applying this patch.
>
> Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>
> If nobody else has comments, I will apply them tomorrow.
> Thanks!
> Nicolas

Merged with the trailing whitespaces removed.
Thanks!
Nicolas