src: various refactorings

Makefile.am

@@ -182,6 +182,7 @@ EXTRA_DIST = COPYING COPYING.libcrun README.md NEWS SECURITY.md rpm/crun.spec au
 	src/libcrun/linux.h src/libcrun/utils.h src/libcrun/error.h src/libcrun/criu.h \
 	src/libcrun/scheduler.h src/libcrun/mempolicy.h src/libcrun/status.h src/libcrun/terminal.h \
 	src/libcrun/mount_flags.h src/libcrun/intelrdt.h src/libcrun/ring_buffer.h src/libcrun/string_map.h \
+	src/libcrun/json_gen_utils.h \
 	src/libcrun/net_device.h src/libcrun/spec.h \
 	src/libcrun/syscalls.h \
 	crun.1.md crun.1 libcrun.lds \

src/create.c

@@ -74,7 +74,7 @@ parse_opt (int key, char *arg, struct argp_state *state)
       break;
 
     case OPTION_PRESERVE_FDS:
-      crun_context.preserve_fds = parse_int_or_fail (argp_mandatory_argument (arg, state), "preserve-fds");
+      crun_context.preserve_fds = parse_id_or_fail (argp_mandatory_argument (arg, state), NULL, "preserve-fds");
       break;
 
     case OPTION_NO_SUBREAPER:

src/crun.c

@@ -389,20 +389,29 @@ argp_mandatory_argument (char *arg, struct argp_state *state)
   return state->argv[state->next++];
 }
 
+/* Parse a numeric value from STR, aborting on an out-of-range or malformed
+   value.  KIND names the field for error messages.  If ENDPTR is not NULL it
+   is set past the parsed number (so the caller can continue parsing, e.g. a
+   "UID:GID" pair); otherwise the whole string must be a valid number.  */
 int
-parse_int_or_fail (const char *str, const char *kind)
+parse_id_or_fail (const char *str, char **endptr, const char *kind)
 {
-  char *endptr = NULL;
+  char *end = NULL;
   long long l;
 
   errno = 0;
-  l = strtoll (str, &endptr, 10);
-  if (errno != 0)
-    libcrun_fail_with_error (errno, "invalid value for `%s`", kind);
-  if (endptr != NULL && *endptr != '\0')
-    libcrun_fail_with_error (EINVAL, "invalid value for `%s`", kind);
+  l = strtoll (str, &end, 10);
+  if (end == str)
+    libcrun_fail_with_error (0, "invalid %s specified", kind);
+  if (errno == ERANGE)
+    libcrun_fail_with_error (0, "invalid %s specified", kind);
   if (l < INT_MIN || l > INT_MAX)
-    libcrun_fail_with_error (ERANGE, "invalid value for `%s`", kind);
+    libcrun_fail_with_error (0, "invalid %s specified", kind);
+
+  if (endptr)
+    *endptr = end;
+  else if (*end != '\0')
+    libcrun_fail_with_error (0, "invalid %s specified", kind);
 
   return (int) l;
 }

src/crun.h

@@ -37,7 +37,7 @@ struct crun_global_arguments
 };
 
 char *argp_mandatory_argument (char *arg, struct argp_state *state);
-int parse_int_or_fail (const char *str, const char *kind);
+int parse_id_or_fail (const char *str, char **endptr, const char *kind);
 int init_libcrun_context (libcrun_context_t *con, const char *id, struct crun_global_arguments *glob,
                           libcrun_error_t *err);
 void crun_assert_n_args (int n, int min, int max);

src/exec.c

@@ -88,26 +88,25 @@ static struct argp_option options[]
 
 static char args_doc[] = "exec CONTAINER cmd";
 
+static void
+append_to_string_array (char ***arr, size_t *size, const char *arg)
+{
+  *arr = xrealloc (*arr, (*size + 2) * sizeof (**arr));
+  (*arr)[*size + 1] = NULL;
+  (*arr)[*size] = xstrdup (arg);
+  (*size)++;
+}
+
 static void
 append_env (const char *arg)
 {
-  exec_options.env = realloc (exec_options.env, (exec_options.env_size + 2) * sizeof (*exec_options.env));
-  if (exec_options.env == NULL)
-    error (EXIT_FAILURE, errno, "cannot allocate memory");
-  exec_options.env[exec_options.env_size + 1] = NULL;
-  exec_options.env[exec_options.env_size] = xstrdup (arg);
-  exec_options.env_size++;
+  append_to_string_array (&exec_options.env, &exec_options.env_size, arg);
 }
 
 static void
 append_cap (const char *arg)
 {
-  exec_options.cap = realloc (exec_options.cap, (exec_options.cap_size + 2) * sizeof (*exec_options.cap));
-  if (exec_options.cap == NULL)
-    error (EXIT_FAILURE, errno, "cannot allocate memory");
-  exec_options.cap[exec_options.cap_size + 1] = NULL;
-  exec_options.cap[exec_options.cap_size] = xstrdup (arg);
-  exec_options.cap_size++;
+  append_to_string_array (&exec_options.cap, &exec_options.cap_size, arg);
 }
 
 static char **
@@ -152,7 +151,7 @@ parse_opt (int key, char *arg, struct argp_state *state)
       break;
 
     case OPTION_PRESERVE_FDS:
-      exec_options.preserve_fds = parse_int_or_fail (argp_mandatory_argument (arg, state), "preserve-fds");
+      exec_options.preserve_fds = parse_id_or_fail (argp_mandatory_argument (arg, state), NULL, "preserve-fds");
       break;
 
     case OPTION_CGROUP:
@@ -205,38 +204,21 @@ make_oci_process_user (const char *userspec)
   runtime_spec_schema_config_schema_process_user *u;
   char *endptr = NULL;
   char *gidstr = NULL;
-  long long l;
 
   if (userspec == NULL)
     return NULL;
 
   u = xmalloc0 (sizeof (runtime_spec_schema_config_schema_process_user));
-  errno = 0;
-  l = strtoll (userspec, &endptr, 10);
-  if (endptr == userspec)
-    libcrun_fail_with_error (0, "invalid UID specified");
-  if (errno == ERANGE)
-    libcrun_fail_with_error (0, "invalid UID specified");
-  if (l < INT_MIN || l > INT_MAX)
-    libcrun_fail_with_error (0, "invalid UID specified");
-  u->uid = (int) l;
+  u->uid = parse_id_or_fail (userspec, &endptr, "UID");
   if (*endptr == '\0')
     return u;
   if (*endptr != ':')
     libcrun_fail_with_error (0, "invalid USERSPEC specified");
 
-  errno = 0;
   gidstr = endptr + 1;
-  l = strtoll (gidstr, &endptr, 10);
-  if (endptr == gidstr)
-    libcrun_fail_with_error (0, "invalid GID specified");
-  if (errno == ERANGE)
-    libcrun_fail_with_error (0, "invalid GID specified");
-  if (l < INT_MIN || l > INT_MAX)
-    libcrun_fail_with_error (0, "invalid GID specified");
+  u->gid = parse_id_or_fail (gidstr, &endptr, "GID");
   if (*endptr != '\0')
     libcrun_fail_with_error (0, "invalid USERSPEC specified");
-  u->gid = (int) l;
   return u;
 }
 

src/libcrun/cgroup-systemd.c

@@ -300,6 +300,18 @@ setup_cpuset_for_systemd_v1 (runtime_spec_schema_config_linux_resources *resourc
   return 0;
 }
 
+/* Build PATH from the cgroup slice FROM, appending SUFFIX when set.  */
+static int
+compute_finalized_path (char **path, const char *from, const char *suffix, libcrun_error_t *err)
+{
+  if (suffix == NULL)
+    {
+      *path = xstrdup (from);
+      return 0;
+    }
+  return append_paths (path, err, from, suffix, NULL);
+}
+
 static int
 systemd_finalize (struct libcrun_cgroup_args *args, char **path_out,
                   int cgroup_mode, const char *suffix, libcrun_error_t *err)
@@ -344,15 +356,10 @@ systemd_finalize (struct libcrun_cgroup_args *args, char **path_out,
       if (UNLIKELY (to == NULL))
         return crun_make_error (err, 0, "cannot parse `%s`", PROC_SELF_CGROUP);
       *to = '\0';
-      if (suffix == NULL)
-        path = xstrdup (from);
-      else
-        {
-          ret = append_paths (&path, err, from, suffix, NULL);
-          if (UNLIKELY (ret < 0))
-            return ret;
-        }
+      ret = compute_finalized_path (&path, from, suffix, err);
       *to = '\n';
+      if (UNLIKELY (ret < 0))
+        return ret;
 
       if (geteuid ())
         return 0;
@@ -411,15 +418,10 @@ systemd_finalize (struct libcrun_cgroup_args *args, char **path_out,
         if (UNLIKELY (to == NULL))
           return crun_make_error (err, 0, "cannot parse `%s`", PROC_SELF_CGROUP);
         *to = '\0';
-        if (suffix == NULL)
-          path = xstrdup (from);
-        else
-          {
-            ret = append_paths (&path, err, from, suffix, NULL);
-            if (UNLIKELY (ret < 0))
-              return ret;
-          }
+        ret = compute_finalized_path (&path, from, suffix, err);
         *to = '\n';
+        if (UNLIKELY (ret < 0))
+          return ret;
 
         ret = append_paths (&dir, err, CGROUP_ROOT, path, NULL);
         if (UNLIKELY (ret < 0))
@@ -1326,9 +1328,6 @@ append_devices (sd_bus_message *m,
         return ret;
     }
 
-  if (resources == NULL)
-    return 0;
-
   for (i = find_first_rule_no_default (resources->devices, resources->devices_len); i < resources->devices_len; i++)
     {
       runtime_spec_schema_defs_linux_device_cgroup *d = resources->devices[i];
@@ -1729,6 +1728,21 @@ verify_ebpf_device_filter_installed (const char *cgroup_path, libcrun_error_t *e
   return 0;
 }
 
+/* Release the sd-bus objects used by a D-Bus call and return RET.  */
+static int
+cleanup_sd_bus_and_return (sd_bus *bus, sd_bus_message *m, sd_bus_message *reply,
+                           sd_bus_error *error, int ret)
+{
+  if (bus)
+    sd_bus_unref (bus);
+  if (m)
+    sd_bus_message_unref (m);
+  if (reply)
+    sd_bus_message_unref (reply);
+  sd_bus_error_free (error);
+  return ret;
+}
+
 static int
 enter_systemd_cgroup_scope (runtime_spec_schema_config_linux_resources *resources,
                             int cgroup_mode,
@@ -1933,14 +1947,7 @@ enter_systemd_cgroup_scope (runtime_spec_schema_config_linux_resources *resource
   ret = systemd_check_job_status (bus, &job_data, object, "creating", err);
 
 exit:
-  if (bus)
-    sd_bus_unref (bus);
-  if (m)
-    sd_bus_message_unref (m);
-  if (reply)
-    sd_bus_message_unref (reply);
-  sd_bus_error_free (&error);
-  return ret;
+  return cleanup_sd_bus_and_return (bus, m, reply, &error, ret);
 }
 
 static int
@@ -1999,14 +2006,7 @@ libcrun_destroy_systemd_cgroup_scope (struct libcrun_cgroup_status *cgroup_statu
   reset_failed_unit (bus, scope);
 
 exit:
-  if (bus)
-    sd_bus_unref (bus);
-  if (m)
-    sd_bus_message_unref (m);
-  if (reply)
-    sd_bus_message_unref (reply);
-  sd_bus_error_free (&error);
-  return ret;
+  return cleanup_sd_bus_and_return (bus, m, reply, &error, ret);
 }
 
 static const char *
@@ -2263,14 +2263,7 @@ libcrun_update_resources_systemd (struct libcrun_cgroup_status *cgroup_status,
   ret = 0;
 
 exit:
-  if (bus)
-    sd_bus_unref (bus);
-  if (m)
-    sd_bus_message_unref (m);
-  if (reply)
-    sd_bus_message_unref (reply);
-  sd_bus_error_free (&error);
-  return ret;
+  return cleanup_sd_bus_and_return (bus, m, reply, &error, ret);
 }
 
 #else