ustrtoull callers cleanup: add warnings/errors when ustrotull fails

Many callers of ustrtoull were not checking for errors. Since there are legacy SWUs that might depend on them, many of these place should not start erroring out immediately, but we must start somewhere: - add many warnings when ustrtoull failed, without impacting the final result - for the ones that are the most user facing (command line arguments), make it a hard error Signed-off-by: Dominique Martinet <[email protected]> Acked-by: Stefano Babic <[email protected]>
sbabic · May 30, 2024 · 9a74fc3 · 9a74fc3
1 parent 994d87c
commit 9a74fc3
Show file tree

Hide file tree

Showing 7 changed files with 38 additions and 5 deletions.
diff --git a/corelib/server_utils.c b/corelib/server_utils.c
@@ -4,6 +4,7 @@
  *
  * SPDX-License-Identifier:     GPL-2.0-only
  */
+#include <errno.h>
 #include <unistd.h>
 #include <stdbool.h>
 #include <stdlib.h>
@@ -24,8 +25,11 @@ int channel_settings(void *elem, void *data)
 		&chan->retries);
 
 	GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "max-download-speed", tmp);
-	if (strlen(tmp))
+	if (strlen(tmp)) {
 		chan->max_download_speed = (unsigned int)ustrtoull(tmp, NULL, 10);
+		if (errno)
+			WARN("max-download-speed setting %s: ustrtoull failed", tmp);
+	}
 
 	GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "retrywait", tmp);
 	if (strlen(tmp))

diff --git a/handlers/copy_handler.c b/handlers/copy_handler.c
@@ -204,9 +204,10 @@ static int recurse_directory(const char *fpath, const struct stat *sb,
 static int copy_image_file(struct img_type *img, void *data)
 {
 	int ret = 0;
+	char *tmp;
 	struct dict_list *proplist;
 	struct dict_list_elem *entry;
-	size_t size;
+	size_t size = 0;
 	struct script_handler_data *script_data;
 	bool recursive, createdest;
 
@@ -249,7 +250,12 @@ static int copy_image_file(struct img_type *img, void *data)
 	/*
 	 * Detect the size if not set in sw-descriptiont
 	 */
-	size = dict_get_value(&img->properties, "size") ? ustrtoull(dict_get_value(&img->properties, "size"), NULL, 0) : 0;
+	tmp = dict_get_value(&img->properties, "size");
+	if (tmp) {
+		size = ustrtoull(tmp, NULL, 0);
+		if (errno)
+			WARN("size property %s: ustrtoull failed", tmp);
+	}
 
 	/*
 	 * No chain set, fallback to rawcopy

diff --git a/handlers/delta_handler.c b/handlers/delta_handler.c
@@ -326,10 +326,13 @@ static int delta_retrieve_attributes(struct img_type *img, struct hnd_priv *priv
 	char *srcsize;
 	srcsize = dict_get_value(&img->properties, "source-size");
 	if (srcsize) {
-		if (!strcmp(srcsize, "detect"))
+		if (!strcmp(srcsize, "detect")) {
 			priv->detectsrcsize = true;
-		else
+		} else {
 			priv->srcsize = ustrtoull(srcsize, NULL, 10);
+			if (errno)
+				WARN("source-size %s: ustrotull failed", srcsize);
+		}
 	}
 
 	char *zckloglevel = dict_get_value(&img->properties, "zckloglevel");

diff --git a/handlers/diskpart_handler.c b/handlers/diskpart_handler.c
@@ -1232,11 +1232,15 @@ static int diskpart(struct img_type *img,
 					switch (i) {
 					case PART_SIZE:
 						part->size = ustrtoull(equal, NULL, 10);
+						if (errno)
+							WARN("partition size %s: ustrtoull failed", equal);
 						if (!size_delimiter_match(equal))
 							part->explicit_size = 1;
 						break;
 					case PART_START:
 						part->start = ustrtoull(equal, NULL, 10);
+						if (errno)
+							WARN("partition size %s: ustrtoull failed", equal);
 						break;
 					case PART_TYPE:
 						strncpy(part->type, equal, sizeof(part->type));

diff --git a/suricatta/server_general.c b/suricatta/server_general.c
@@ -663,6 +663,11 @@ static server_op_res_t server_start(const char *fname, int argc, char *argv[])
 		case 'n':
 			channel_data_defaults.max_download_speed =
 				(unsigned int)ustrtoull(optarg, NULL, 10);
+			if (errno) {
+				ERROR("max-download-speed %s: ustrtoull failed",
+				      optarg);
+				return SERVER_EINIT;
+			}
 			break;
 
 		/* Ignore the --server option which is already parsed by the caller. */

diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c
@@ -871,6 +871,9 @@ static void get_action_id_from_env(int *action_id)
 	char *action_str = swupdate_vars_get("action_id", NULL);
 	if (action_str) {
 		int tmp = ustrtoull(action_str, NULL, 10);
+		if (errno)
+			WARN("action_id %s: ustrtoull failed",
+			     action_str);
 		/*
 		 * action_id = 0 is invalid, then check it
 		 */
@@ -1910,6 +1913,11 @@ static server_op_res_t server_start(const char *fname, int argc, char *argv[])
 		case 'n':
 			channel_data_defaults.max_download_speed =
 				(unsigned int)ustrtoull(optarg, NULL, 10);
+			if (errno) {
+				ERROR("max-download-speed %s: ustrtoull failed",
+				      optarg);
+				return SERVER_EINIT;
+			}
 			break;
 		/* Ignore not recognized options, they can be already parsed by the caller */
 		case '?':

diff --git a/suricatta/server_lua.c b/suricatta/server_lua.c
@@ -591,6 +591,9 @@ static void channel_set_options(lua_State *L, channel_data_t *channel_data)
 	get_from_table(L, "max_download_speed", max_download_speed);
 	if (max_download_speed) {
 		channel_data->max_download_speed = (unsigned int)ustrtoull(max_download_speed, NULL, 10);
+		if (errno)
+			WARN("max-download-speed %s: ustrtoull failed",
+			     max_download_speed);
 		free(max_download_speed);
 	}
 	lua_getfield(L, -1, "proxy");