modpost: refactor error handling and clarify error/fatal difference

We have 3 log functions. fatal() is special because it lets modpost bail out immediately. The difference between warn() and error() is the only prefix parts ("WARNING:" vs "ERROR:"). In my understanding, the expected handling of error() is to propagate the return code of the function to the exit code of modpost, as check_exports() etc. already does. This is a good manner in general because we should display as many error messages as possible in a single run of modpost. What is annoying about fatal() is that it kills modpost at the first error. People would need to run Kbuild again and again until they fix all errors. But, unfortunately, people tend to do: "This case should not be allowed. Let's replace warn() with fatal()." One of the reasons is probably it is tedious to manually hoist the error code to the main() function. This commit refactors error() so any single call for it automatically makes modpost return the error code. I also added comments in modpost.h for warn(), error(), and fatal(). Please use fatal() only when you have a strong reason to do so. For example: - Memory shortage (i.e. malloc() etc. has failed) - The ELF file is broken, and there is no point to continue parsing - Something really odd has happened For general coding errors, please use error().

Masahiro Yamada committed 5 years ago

Commit 0fd3fba1 parent bc72d722 files · +27 −29Browse files

Showing the changes to scripts (2 of 2 files). Show all

Unified · Split

2 files

scripts/mod/modpost.c +14 −29
scripts/mod/modpost.h +13 −0

scripts/mod/modpost.c+14−29

 /* If set to 1, only warn (instead of error) about missing ns imports */
 static int allow_missing_ns_imports;
+static bool error_occurred;
 enum export {
 	export_plain,      export_unused,     export_gpl,
 	export_unused_gpl, export_gpl_future, export_unknown
 	if (loglevel == LOG_FATAL)
 		exit(1);
+	if (loglevel == LOG_ERROR)
+		error_occurred = true;
 }
 static inline bool strends(const char *str, const char *postfix)
 	}
 }
-static int check_exports(struct module *mod)
+static void check_exports(struct module *mod)
 {
 	struct symbol *s, *exp;
-	int err = 0;
 	for (s = mod->unres; s; s = s->next) {
 		const char *basename;
 		exp = find_symbol(s->name);
 		if (!exp || exp->module == mod) {
-			if (have_vmlinux && !s->weak) {
+			if (have_vmlinux && !s->weak)
 				modpost_log(warn_unresolved ? LOG_WARN : LOG_ERROR,
 					    "\"%s\" [%s.ko] undefined!\n",
 					    s->name, mod->name);
-				if (!warn_unresolved)
-					err = 1;
-			}
 			continue;
 		}
 		basename = strrchr(mod->name, '/');
 			modpost_log(allow_missing_ns_imports ? LOG_WARN : LOG_ERROR,
 				    "module %s uses symbol %s from namespace %s, but does not import it.\n",
 				    basename, exp->name, exp->namespace);
-			if (!allow_missing_ns_imports)
-				err = 1;
 			add_namespace(&mod->missing_namespaces, exp->namespace);
 		}
 			check_for_gpl_usage(exp->export, basename, exp->name);
 		check_for_unused(exp->export, basename, exp->name);
 	}
-	return err;
 }
-static int check_modname_len(struct module *mod)
+static void check_modname_len(struct module *mod)
 {
 	const char *mod_name;
 		mod_name = mod->name;
 	else
 		mod_name++;
-	if (strlen(mod_name) >= MODULE_NAME_LEN) {
+	if (strlen(mod_name) >= MODULE_NAME_LEN)
 		error("module name is too long [%s.ko]\n", mod->name);
-		return 1;
-	}
-	return 0;
 }
 /**
 /**
  * Record CRCs for unresolved symbols
  **/
-static int add_versions(struct buffer *b, struct module *mod)
+static void add_versions(struct buffer *b, struct module *mod)
 {
 	struct symbol *s, *exp;
-	int err = 0;
 	for (s = mod->unres; s; s = s->next) {
 		exp = find_symbol(s->name);
 	}
 	if (!modversions)
-		return err;
+		return;
 	buf_printf(b, "\n");
 	buf_printf(b, "static const struct modversion_info ____versions[]\n");
 		if (strlen(s->name) >= MODULE_NAME_LEN) {
 			error("too long symbol \"%s\" [%s.ko]\n",
 			      s->name, mod->name);
-			err = 1;
 			break;
 		}
 		buf_printf(b, "\t{ %#8x, \"%s\" },\n",
 	}
 	buf_printf(b, "};\n");
-	return err;
 }
 static void add_depends(struct buffer *b, struct module *mod)
 	char *missing_namespace_deps = NULL;
 	char *dump_write = NULL, *files_source = NULL;
 	int opt;
-	int err;
 	int n;
 	struct dump_list *dump_read_start = NULL;
 	struct dump_list **dump_read_iter = &dump_read_start;
 	if (!have_vmlinux)
 		warn("Symbol info of vmlinux is missing. Unresolved symbol check will be entirely skipped.\n");
-	err = 0;
 	for (mod = modules; mod; mod = mod->next) {
 		char fname[PATH_MAX];
 		buf.pos = 0;
-		err |= check_modname_len(mod);
+		check_modname_len(mod);
-		err |= check_exports(mod);
+		check_exports(mod);
 		add_header(&buf, mod);
 		add_intree_flag(&buf, !external_module);
 		add_retpoline(&buf);
 		add_staging_flag(&buf, mod->name);
-		err |= add_versions(&buf, mod);
+		add_versions(&buf, mod);
 		add_depends(&buf, mod);
 		add_moddevtable(&buf, mod);
 		add_srcversion(&buf, mod);
 	free(buf.p);
-	return err;
+	return error_occurred ? 1 : 0;
 }

scripts/mod/modpost.h+13−0

@@ -201,6 +201,19 @@ enum loglevel { · 200 unmodified lines
201		201
202	void modpost_log(enum loglevel loglevel, const char *fmt, ...);	202	void modpost_log(enum loglevel loglevel, const char *fmt, ...);
203		203
		204	/*
		205	* warn - show the given message, then let modpost continue running, still
		206	* allowing modpost to exit successfully. This should be used when
		207	* we still allow to generate vmlinux and modules.
		208	*
		209	* error - show the given message, then let modpost continue running, but fail
		210	* in the end. This should be used when we should stop building vmlinux
		211	* or modules, but we can continue running modpost to catch as many
		212	* issues as possible.
		213	*
		214	* fatal - show the given message, and bail out immediately. This should be
		215	* used when there is no point to continue running modpost.
		216	*/
204	#define warn(fmt, args...) modpost_log(LOG_WARN, fmt, ##args)	217	#define warn(fmt, args...) modpost_log(LOG_WARN, fmt, ##args)
205	#define error(fmt, args...) modpost_log(LOG_ERROR, fmt, ##args)	218	#define error(fmt, args...) modpost_log(LOG_ERROR, fmt, ##args)
206	#define fatal(fmt, args...) modpost_log(LOG_FATAL, fmt, ##args)	219	#define fatal(fmt, args...) modpost_log(LOG_FATAL, fmt, ##args)

modpost: refactor error handling and clarify error/fatal difference

Keyboard shortcuts

General

Issues and pull requests

Issues, pull requests and Inbox

Repository code