tidy tests, cppcheck, valgrind, iwyu (#75)

* 71 add cfg to IpcResponse * 71 fix test memory leaks * 71 use macros for logs for better error discoverability * 71 cppcheck suppressions for tests * 71 iwyu * 71 iwyu * 71 add --yaml client option * 71 remove --yaml client option * 71 remove --yaml client option * 71 remove --yaml client option
alex-courtis · Mar 20, 2023 · 0f5d9d7 · 0f5d9d7
1 parent 60c9bbc
commit 0f5d9d7
Show file tree

Hide file tree

Showing 21 changed files with 126 additions and 174 deletions.
diff --git a/.cppcheck.supp b/.cppcheck.supp
@@ -0,0 +1,2 @@
+incorrectStringBooleanError:tst/tst-*.c
+unusedFunction:tst/wrap-*.c
diff --git a/way-displays.supp → .vg.supp b/way-displays.supp → .vg.supp
diff --git a/GNUmakefile b/GNUmakefile
@@ -68,7 +68,7 @@ iwyu: clean $(SRC_O) $(TST_O)
 IWYU = /usr/bin/include-what-you-use -Xiwyu --no_fwd_decls -Xiwyu --no_comments -Xiwyu --verbose=2
 
 cppcheck: $(SRC_C) $(SRC_CXX) $(INC_H) $(EXAMPLE_C) $(TST_H) $(TST_C)
-	cppcheck $(^) --enable=warning,unusedFunction,performance,portability $(CPPFLAGS)
+	cppcheck $(^) --enable=warning,unusedFunction,performance,portability --suppressions-list=.cppcheck.supp $(CPPFLAGS)
 
 test:
 	$(MAKE) -f tst/GNUmakefile tst-all

diff --git a/examples/example_client.c b/examples/example_client.c
@@ -104,7 +104,7 @@ CFG:\n\
 	execute(CFG_DEL, request);
 }
 
-void usage(void) {
+void example_usage(void) {
 	fprintf(stderr, "Usage: example_client <GET | CFG_WRITE | CFG_SET | CFG_DEL>\n");
 	exit(1);
 }
@@ -114,7 +114,7 @@ main(int argc, char **argv) {
 	log_set_threshold(DEBUG, true);
 
 	if (argc != 2) {
-		usage();
+		example_usage();
 	}
 
 	void (*fn)(void);
@@ -127,7 +127,7 @@ main(int argc, char **argv) {
 	} else if (strcmp(argv[1], ipc_request_op_name(CFG_DEL)) == 0) {
 		fn = cfg_del;
 	} else {
-		usage();
+		example_usage();
 	}
 
 	if (!getenv("WAYLAND_DISPLAY")) {

diff --git a/inc/ipc.h b/inc/ipc.h
@@ -29,7 +29,7 @@ struct IpcResponse {
 	int rc;
 	int fd;
 	bool messages;
-	bool status;
+	bool state;
 };
 
 int ipc_request_send(struct IpcRequest *request);

diff --git a/inc/mode.h b/inc/mode.h
@@ -25,9 +25,9 @@ struct ModesResRefresh {
 	struct SList *modes;
 };
 
-struct Mode *mode_preferred(struct Head *head);
+struct Mode *mode_preferred(struct SList *modes, struct SList *modes_failed);
 
-struct Mode *mode_max_preferred(struct Head *head);
+struct Mode *mode_max_preferred(struct SList *modes, struct SList *modes_failed);
 
 int32_t mhz_to_hz(int32_t mhz);
 

diff --git a/src/head.c b/src/head.c
@@ -201,9 +201,9 @@ struct Mode *head_find_mode(struct Head *head) {
 	// always preferred
 	if (!mode) {
 		if (head_is_max_preferred_refresh(head)) {
-			mode = mode_max_preferred(head);
+			mode = mode_max_preferred(head->modes, head->modes_failed);
 		} else {
-			mode = mode_preferred(head);
+			mode = mode_preferred(head->modes, head->modes_failed);
 		}
 		if (!mode && !head->warned_no_preferred) {
 			head->warned_no_preferred = true;

diff --git a/src/marshalling.cpp b/src/marshalling.cpp
@@ -498,7 +498,7 @@ char *marshal_ipc_response(struct IpcResponse *response) {
 
 		e << YAML::Key << "DONE" << YAML::Value << response->done;
 
-		if (response->status) {
+		if (response->state) {
 			if (cfg) {
 				e << YAML::Key << "CFG" << YAML::BeginMap;		// CFG
 				e << *cfg;

diff --git a/src/mode.c b/src/mode.c
@@ -8,38 +8,36 @@
 #include "head.h"
 #include "list.h"
 
-struct Mode *mode_preferred(struct Head *head) {
-	if (!head)
-		return NULL;
-
+struct Mode *mode_preferred(struct SList *modes, struct SList *modes_failed) {
 	struct Mode *mode = NULL;
-	for (struct SList *i = head->modes; i; i = i->nex) {
+
+	for (struct SList *i = modes; i; i = i->nex) {
 		if (!i->val)
 			continue;
 		mode = i->val;
 
-		if (mode->preferred && !slist_find_equal(head->modes_failed, NULL, mode)) {
+		if (mode->preferred && !slist_find_equal(modes_failed, NULL, mode)) {
 			return mode;
 		}
 	}
 
 	return NULL;
 }
 
-struct Mode *mode_max_preferred(struct Head *head) {
-	struct Mode *preferred = mode_preferred(head);
+struct Mode *mode_max_preferred(struct SList *modes, struct SList *modes_failed) {
+	struct Mode *preferred = mode_preferred(modes, modes_failed);
 
 	if (!preferred)
 		return NULL;
 
 	struct Mode *mode = NULL, *max = NULL;
 
-	for (struct SList *i = head->modes; i; i = i->nex) {
+	for (struct SList *i = modes; i; i = i->nex) {
 		if (!i->val)
 			continue;
 		mode = i->val;
 
-		if (slist_find_equal(head->modes_failed, NULL, mode)) {
+		if (slist_find_equal(modes_failed, NULL, mode)) {
 			continue;
 		}
 

diff --git a/src/server.c b/src/server.c
@@ -85,11 +85,11 @@ void handle_ipc_request(int fd_sock) {
 	ipc_response->fd = ipc_request->fd;
 	ipc_response->done = true;
 	ipc_response->messages = true;
-	ipc_response->status = true;
+	ipc_response->state = true;
 
 	if (ipc_request->bad) {
 		ipc_response->rc = IPC_RC_BAD_REQUEST;
-		ipc_response->status = false;
+		ipc_response->state = false;
 		goto send;
 	}
 

diff --git a/tst/expects.h b/tst/expects.h
@@ -10,10 +10,45 @@
 
 #include "log.h"
 
-void expect_log_(enum LogThreshold threshold, const char *__restrict __format, const void *arg1, const void *arg2, const void *arg3, const void *arg4, ...);
-void expect_log_info(const char *__restrict __format, const void *arg1, const void *arg2, const void *arg3, const void *arg4, ...);
-void expect_log_warn(const char *__restrict __format, const void *arg1, const void *arg2, const void *arg3, const void *arg4, ...);
-void expect_log_error(const char *__restrict __format, const void *arg1, const void *arg2, const void *arg3, const void *arg4, ...);
-void expect_log_error_nocap(const char *__restrict __format, const void *arg1, const void *arg2, const void *arg3, const void *arg4, ...);
+#define expect_log(fn, f, a1, a2, a3, a4) \
+	if (f) \
+		expect_string(fn, __format, f); \
+	else \
+		expect_any(fn, __format); \
+	if (a1) \
+		expect_string(fn, arg1, a1); \
+	else \
+		expect_any(fn, arg1); \
+	if (a2) \
+		expect_string(fn, arg2, a2); \
+	else \
+		expect_any(fn, arg2); \
+	if (a3) \
+		expect_string(fn, arg3, a3); \
+	else \
+		expect_any(fn, arg3); \
+	if (a4) \
+		expect_string(fn, arg4, a4); \
+	else \
+		expect_any(fn, arg4);
+
+#define expect_log_debug(f, a1, a2, a3, a4) \
+	expect_log(__wrap_log_debug, f, a1, a2, a3, a4)
+
+#define expect_log_info(f, a1, a2, a3, a4) \
+	expect_log(__wrap_log_info, f, a1, a2, a3, a4)
+
+#define expect_log_warn(f, a1, a2, a3, a4) \
+	expect_log(__wrap_log_warn, f, a1, a2, a3, a4)
+
+#define expect_log_error(f, a1, a2, a3, a4) \
+	expect_log(__wrap_log_error, f, a1, a2, a3, a4)
+
+#define expect_log_error_nocap(f, a1, a2, a3, a4) \
+	expect_log(__wrap_log_error_nocap, f, a1, a2, a3, a4)
+
+#define expect_log_(t, f, a1, a2, a3, a4) \
+	expect_value(__wrap_log_, threshold, t); \
+	expect_log(__wrap_log_, f, a1, a2, a3, a4)
 
 #endif // EXPECTS_H
diff --git a/tst/marshalling/cfg-all.yaml b/tst/marshalling/cfg-all.yaml
@@ -25,3 +25,4 @@ DISABLED:
   - EIGHT
   - nine
 LOG_THRESHOLD: ERROR
+
diff --git a/tst/marshalling/ipc-request-cfg-set.yaml b/tst/marshalling/ipc-request-cfg-set.yaml
@@ -27,3 +27,4 @@ CFG:
     - EIGHT
     - nine
   LOG_THRESHOLD: ERROR
+
diff --git a/tst/marshalling/ipc-request-get.yaml b/tst/marshalling/ipc-request-get.yaml
@@ -1 +1,2 @@
 OP: GET
+
diff --git a/tst/marshalling/ipc-response-ok.yaml b/tst/marshalling/ipc-response-ok.yaml
@@ -67,3 +67,4 @@ MESSAGES:
   WARNING: war
   ERROR: err
 RC: 2
+
diff --git a/tst/tst-cfg.c b/tst/tst-cfg.c
@@ -255,15 +255,15 @@ void validate_fix__row(void **state) {
 void validate_fix__scale(void **state) {
 	struct State *s = *state;
 
-	char FMT[] = "\nIgnoring non-positive SCALE %s %.3f";
+	char *fmt = "\nIgnoring non-positive SCALE %s %.3f";
 
 	slist_append(&s->from->user_scales, cfg_user_scale_init("ok", 1));
 
 	slist_append(&s->from->user_scales, cfg_user_scale_init("neg", -1));
-	expect_log_warn(FMT, "neg", NULL, NULL, NULL);
+	expect_log_warn(fmt, "neg", NULL, NULL, NULL);
 
 	slist_append(&s->from->user_scales, cfg_user_scale_init("zero", 0));
-	expect_log_warn(FMT, "zero", NULL, NULL, NULL);
+	expect_log_warn(fmt, "zero", NULL, NULL, NULL);
 
 	validate_fix(s->from);
 
@@ -304,27 +304,27 @@ void validate_fix__mode(void **state) {
 void validate_warn__(void **state) {
 	struct State *s = *state;
 
-	char FMT[] = "\n%s '%s' is less than 4 characters, which may result in some unwanted matches.";
+	char *fmt = "\n%s '%s' is less than 4 characters, which may result in some unwanted matches.";
 
 	slist_append(&s->expected->user_scales, cfg_user_scale_init("sss", 1));
 	slist_append(&s->expected->user_scales, cfg_user_scale_init("ssssssss", 2));
-	expect_log_warn(FMT, "SCALE", "sss", NULL, NULL);
+	expect_log_warn(fmt, "SCALE", "sss", NULL, NULL);
 
 	slist_append(&s->expected->user_modes, cfg_user_mode_init("mmm", false, 1, 1, 1, false));
 	slist_append(&s->expected->user_modes, cfg_user_mode_init("mmmmmmmm", false, 1, 1, 1, false));
-	expect_log_warn(FMT, "MODE", "mmm", NULL, NULL);
+	expect_log_warn(fmt, "MODE", "mmm", NULL, NULL);
 
 	slist_append(&s->expected->order_name_desc, strdup("ooo"));
 	slist_append(&s->expected->order_name_desc, strdup("oooooooooo"));
-	expect_log_warn(FMT, "ORDER", "ooo", NULL, NULL);
+	expect_log_warn(fmt, "ORDER", "ooo", NULL, NULL);
 
 	slist_append(&s->expected->max_preferred_refresh_name_desc, strdup("ppp"));
 	slist_append(&s->expected->max_preferred_refresh_name_desc, strdup("pppppppppp"));
-	expect_log_warn(FMT, "MAX_PREFERRED_REFRESH", "ppp", NULL, NULL);
+	expect_log_warn(fmt, "MAX_PREFERRED_REFRESH", "ppp", NULL, NULL);
 
 	slist_append(&s->expected->disabled_name_desc, strdup("ddd"));
 	slist_append(&s->expected->disabled_name_desc, strdup("dddddddddd"));
-	expect_log_warn(FMT, "DISABLED", "ddd", NULL, NULL);
+	expect_log_warn(fmt, "DISABLED", "ddd", NULL, NULL);
 
 	validate_warn(s->expected);
 }

diff --git a/tst/tst-cli.c b/tst/tst-cli.c
@@ -197,7 +197,8 @@ void parse_element__mode_set_max(void **state) {
 
 	cfg_free(actual);
 
-	slist_free(&expected.user_scales);
+	slist_free(&expected.user_modes);
+	cfg_user_mode_free(expectedUserMode);
 }
 
 void parse_element__mode_set_res(void **state) {
@@ -219,7 +220,8 @@ void parse_element__mode_set_res(void **state) {
 
 	cfg_free(actual);
 
-	slist_free(&expected.user_scales);
+	slist_free(&expected.user_modes);
+	cfg_user_mode_free(expectedUserMode);
 }
 
 void parse_element__mode_set_res_refresh(void **state) {
@@ -242,7 +244,8 @@ void parse_element__mode_set_res_refresh(void **state) {
 
 	cfg_free(actual);
 
-	slist_free(&expected.user_scales);
+	slist_free(&expected.user_modes);
+	cfg_user_mode_free(expectedUserMode);
 }
 
 void parse_element__mode_del_ok(void **state) {
@@ -262,7 +265,8 @@ void parse_element__mode_del_ok(void **state) {
 
 	cfg_free(actual);
 
-	slist_free(&expected.user_scales);
+	slist_free(&expected.user_modes);
+	cfg_user_mode_free(expectedUserMode);
 }
 
 void parse_element__disabled_ok(void **state) {
@@ -316,6 +320,8 @@ void parse_write__ok(void **state) {
 
 	assert_non_null(request);
 	assert_int_equal(request->op, CFG_WRITE);
+
+	ipc_request_free(request);
 }
 
 void parse_set__mode_nargs(void **state) {
@@ -403,6 +409,8 @@ void parse_set__ok(void **state) {
 
 	assert_non_null(request);
 	assert_int_equal(request->op, CFG_SET);
+
+	ipc_request_free(request);
 }
 
 void parse_del__mode_nargs(void **state) {
@@ -455,6 +463,8 @@ void parse_del__ok(void **state) {
 
 	assert_non_null(request);
 	assert_int_equal(request->op, CFG_DEL);
+
+	ipc_request_free(request);
 }
 
 void parse_log_threshold__invalid(void **state) {
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		incorrectStringBooleanError:tst/tst-*.c
		unusedFunction:tst/wrap-*.c
Original file line number	Diff line number	Diff line change
Expand Up		@@ -25,3 +25,4 @@ DISABLED:
		- EIGHT
		- nine
		LOG_THRESHOLD: ERROR
Original file line number	Diff line number	Diff line change
Expand Up		@@ -27,3 +27,4 @@ CFG:
		- EIGHT
		- nine
		LOG_THRESHOLD: ERROR
Original file line number	Diff line number	Diff line change
Expand Up		@@ -67,3 +67,4 @@ MESSAGES:
		WARNING: war
		ERROR: err
		RC: 2