Use json unpack format style consistently

chu11 · chu11 · commit 9ce72810277a · 2023-06-21T13:36:15.000-07:00
Problem: While both the formats "s?:" and "s?" can be used
to deal with optional keys while unpacking json, it has often
used inconsistently.  Sometimes both styles were used within
the same line of code!

Solution: Use "s?" over "s?:" as it is more widely used in flux-core.
diff --git a/src/broker/boot_config.c b/src/broker/boot_config.c
@@ -409,7 +409,7 @@ static int gethostentry (json_t *hosts,
     /* N.B. entry already validated by boot_config_parse().
      */
     (void)json_unpack (entry,
-                      "{s:s s?:s s?:s}",
+                      "{s:s s?s s?s}",
                       "host", host,
                       "bind", bind,
                       "connect", uri);
diff --git a/src/broker/publisher.c b/src/broker/publisher.c
@@ -102,7 +102,7 @@ static void publish_cb (flux_t *h,
     flux_msg_t *event = NULL;
     const char *errmsg = NULL;
 
-    if (flux_request_unpack (msg, NULL, "{s:s s:i s?:s}",
+    if (flux_request_unpack (msg, NULL, "{s:s s:i s?s}",
                                         "topic", &topic,
                                         "flags", &flags,
                                         "payload", &payload) < 0)
diff --git a/src/cmd/flux-job.c b/src/cmd/flux-job.c
@@ -1666,7 +1666,7 @@ static void handle_output_redirect (struct attach_ctx *ctx, json_t *context)
     const char *path = NULL;
     if (!ctx->output_header_parsed)
         log_msg_exit ("stream redirect read before header");
-    if (json_unpack (context, "{ s:s s:s s?:s }",
+    if (json_unpack (context, "{ s:s s:s s?s }",
                               "stream", &stream,
                               "rank", &rank,
                               "path", &path) < 0)
@@ -1700,7 +1700,7 @@ static void handle_output_log (struct attach_ctx *ctx,
     json_error_t err;
 
     if (json_unpack_ex (context, &err, 0,
-                        "{ s?i s:i s:s s?:s s?:s s?:i }",
+                        "{ s?i s:i s:s s?s s?s s?i }",
                         "rank", &rank,
                         "level", &level,
                         "message", &msg,
@@ -2008,7 +2008,7 @@ static void setup_mpir_interface (struct attach_ctx *ctx, json_t *context)
     flux_future_t *f = NULL;
     int stop_tasks_in_exec = 0;
 
-    if (json_unpack (context, "{s?:b}", "sync", &stop_tasks_in_exec) < 0)
+    if (json_unpack (context, "{s?b}", "sync", &stop_tasks_in_exec) < 0)
         log_err_exit ("error decoding shell.init context");
 
     snprintf (topic, sizeof (topic), "%s.proctable", ctx->service);
@@ -2513,7 +2513,7 @@ void attach_event_continuation (flux_future_t *f, void *arg)
         int severity;
         const char *note = NULL;
 
-        if (json_unpack (context, "{s:s s:i s?:s}",
+        if (json_unpack (context, "{s:s s:i s?s}",
                          "type", &type,
                          "severity", &severity,
                          "note", &note) < 0)
diff --git a/src/common/libflux/test/rpc.c b/src/common/libflux/test/rpc.c
@@ -85,7 +85,7 @@ void rpctest_echo_error_cb (flux_t *h, flux_msg_handler_t *mh,
     int errnum;
     const char *errstr = NULL;
 
-    if (flux_request_unpack (msg, NULL, "{s:i s?:s}",
+    if (flux_request_unpack (msg, NULL, "{s:i s?s}",
                              "errnum", &errnum, "errstr", &errstr) < 0)
         goto error;
     if (errstr) {
diff --git a/src/common/libjob/result.c b/src/common/libjob/result.c
@@ -118,7 +118,7 @@ static int job_result_handle_exception (json_t *res,
         return -1;
 
     if (json_unpack (context,
-                     "{s:o s:o s?:o}",
+                     "{s:o s:o s?o}",
                      "type", &type,
                      "severity", &severity,
                      "note", &note) < 0)
diff --git a/src/modules/connector-local/local.c b/src/modules/connector-local/local.c
@@ -218,7 +218,7 @@ int parse_config (struct connector_local *ctx,
 
     if (flux_conf_unpack (conf,
                           &error,
-                          "{s?:{s?:b s?:b !}}",
+                          "{s?{s?b s?b !}}",
                           "access",
                             "allow-guest-user",
                             &allow_guest_user,
diff --git a/src/modules/content-s3/content-s3.c b/src/modules/content-s3/content-s3.c
@@ -155,7 +155,7 @@ static struct s3_config *parse_config (const flux_conf_t *conf,
 
     if (flux_conf_unpack (conf,
                           &error,
-                          "{s:{s:s, s:s, s:s, s?:b !} }",
+                          "{s:{s:s, s:s, s:s, s?b !} }",
                           "content-s3",
                           "credential-file",
                           &cred_file,
diff --git a/src/modules/job-archive/job-archive.c b/src/modules/job-archive/job-archive.c
@@ -272,7 +272,7 @@ void job_info_lookup_continuation (flux_future_t *f, void *arg)
 
     monotime (&t0);
 
-    if (flux_rpc_get_unpack (f, "{s:s s:s s?:s}",
+    if (flux_rpc_get_unpack (f, "{s:s s:s s?s}",
                              "eventlog", &eventlog,
                              "jobspec", &jobspec,
                              "R", &R) < 0) {
@@ -292,7 +292,7 @@ void job_info_lookup_continuation (flux_future_t *f, void *arg)
     }
 
     if (json_unpack_ex (job, &error, 0,
-                        "{s:i s?:s s:f s?:f s?:f s:f}",
+                        "{s:i s?s s:f s?f s?f s:f}",
                         "userid", &userid,
                         "ranks", &ranks,
                         "t_submit", &t_submit,
@@ -420,7 +420,7 @@ int job_info_lookup (struct job_archive_ctx *ctx, json_t *job)
     json_t *keys = NULL;
     double t_run = 0.0;
 
-    if (json_unpack (job, "{s:I s?:f}", "id", &id, "t_run", &t_run) < 0) {
+    if (json_unpack (job, "{s:I s?f}", "id", &id, "t_run", &t_run) < 0) {
         flux_log (ctx->h, LOG_ERR, "%s: parse t_run", __FUNCTION__);
         goto error;
     }
diff --git a/src/modules/job-exec/exec_config.c b/src/modules/job-exec/exec_config.c
@@ -87,7 +87,7 @@ int config_init (flux_t *h, int argc, char **argv)
     /*  Check configuration for exec.job-shell */
     if (flux_conf_unpack (flux_get_conf (h),
                           &err,
-                          "{s?:{s?s}}",
+                          "{s?{s?s}}",
                           "exec",
                             "job-shell", &default_job_shell) < 0) {
         flux_log (h, LOG_ERR,
@@ -99,7 +99,7 @@ int config_init (flux_t *h, int argc, char **argv)
     /*  Check configuration for exec.imp */
     if (flux_conf_unpack (flux_get_conf (h),
                           &err,
-                          "{s?:{s?s}}",
+                          "{s?{s?s}}",
                           "exec",
                             "imp", &flux_imp_path) < 0) {
         flux_log (h, LOG_ERR,
diff --git a/src/modules/job-exec/sdexec.c b/src/modules/job-exec/sdexec.c
@@ -301,7 +301,7 @@ static int sdexec_init (struct jobinfo *job)
         const char *method = NULL;
         if (flux_conf_unpack (flux_get_conf (h),
                               &err,
-                              "{s?:{s?s}}",
+                              "{s?{s?s}}",
                               "exec",
                                 "method", &method) < 0) {
             flux_log (h, LOG_ERR,
diff --git a/src/modules/job-ingest/worker.c b/src/modules/job-ingest/worker.c
@@ -173,9 +173,9 @@ static void worker_fulfill_future (struct worker *w, flux_future_t *f, const cha
         errnum = EINVAL;
         goto error;
     }
-    if (json_unpack (o, "{s:i s?:s s?:o}", "errnum", &errnum,
-                                           "errstr", &errstr,
-                                           "data", &data) < 0) {
+    if (json_unpack (o, "{s:i s?s s?o}", "errnum", &errnum,
+                                         "errstr", &errstr,
+                                         "data", &data) < 0) {
         flux_log (w->h, LOG_ERR, "%s: json_unpack '%s' failed", w->name, s);
         errnum = EINVAL;
         goto error;
diff --git a/src/modules/job-list/job_data.c b/src/modules/job-list/job_data.c
@@ -102,7 +102,7 @@ static int parse_jobspec_job_name (struct job *job,
 
     if (jobspec_job) {
         if (json_unpack_ex (jobspec_job, &error, 0,
-                            "{s?:s}",
+                            "{s?s}",
                             "name", &job->name) < 0) {
             flux_log (job->h, LOG_ERR,
                       "%s: job %s invalid job dictionary: %s",
@@ -175,7 +175,7 @@ static int parse_per_resource (struct job *job,
     json_t *o = NULL;
 
     if (json_unpack_ex (job->jobspec, &error, 0,
-                        "{s:{s?{s?:{s?:{s?:o}}}}}",
+                        "{s:{s?{s?{s?{s?o}}}}}",
                         "attributes",
                           "system",
                             "shell",
@@ -190,7 +190,7 @@ static int parse_per_resource (struct job *job,
     (*count) = 1;
     if (o) {
         if (json_unpack_ex (o, &error, 0,
-                            "{s:s s?:i}",
+                            "{s:s s?i}",
                             "type", type,
                             "count", count) < 0) {
             flux_log (job->h, LOG_ERR,
@@ -276,7 +276,7 @@ static int parse_jobspec (struct job *job, const char *s, bool allow_nonfatal)
     }
 
     if (json_unpack_ex (job->jobspec, &error, 0,
-                        "{s:{s?{s?:o}}}",
+                        "{s:{s?{s?o}}}",
                         "attributes",
                         "system",
                         "job",
@@ -309,7 +309,7 @@ static int parse_jobspec (struct job *job, const char *s, bool allow_nonfatal)
         goto nonfatal_error;
 
     if (json_unpack_ex (job->jobspec, &error, 0,
-                        "{s:{s?{s?:s s?s}}}",
+                        "{s:{s?{s?s s?s}}}",
                         "attributes",
                         "system",
                         "cwd", &job->cwd,
diff --git a/src/modules/job-list/job_state.c b/src/modules/job-list/job_state.c
@@ -1074,7 +1074,7 @@ static int exception_context_parse (flux_t *h,
 
     if (!context
         || json_unpack (context,
-                        "{s:s s:i s?:s}",
+                        "{s:s s:i s?s}",
                         "type", &type,
                         "severity", &severity,
                         "note", &note) < 0) {
diff --git a/src/modules/job-manager/alloc.c b/src/modules/job-manager/alloc.c
@@ -209,7 +209,7 @@ static void alloc_response_cb (flux_t *h, flux_msg_handler_t *mh,
 
     if (flux_response_decode (msg, NULL, NULL) < 0)
         goto teardown; // ENOSYS here if scheduler not loaded/shutting down
-    if (flux_msg_unpack (msg, "{s:I s:i s?:s s?:o}",
+    if (flux_msg_unpack (msg, "{s:I s:i s?s s?o}",
                               "id", &id,
                               "type", &type,
                               "note", &note,
@@ -415,7 +415,7 @@ static void ready_cb (flux_t *h, flux_msg_handler_t *mh,
     struct job *job;
     const char *sender;
 
-    if (flux_request_unpack (msg, NULL, "{s:s s?:i}",
+    if (flux_request_unpack (msg, NULL, "{s:s s?i}",
                                         "mode", &mode,
                                         "limit", &limit) < 0)
         goto error;
diff --git a/src/modules/job-manager/job.c b/src/modules/job-manager/job.c
@@ -161,7 +161,7 @@ static int jobspec_redacted_parse_queue (struct job *job)
          * optional
          */
         if (json_unpack (job->jobspec_redacted,
-                         "{s:{s?{s?s}}}",
+                         "{s?{s?{s?s}}}",
                          "attributes",
                          "system",
                          "queue", &job->queue) < 0) {
diff --git a/src/modules/job-manager/jobtap.c b/src/modules/job-manager/jobtap.c
@@ -376,7 +376,7 @@ static int jobtap_conf_entry (struct jobtap *jobtap,
     json_t *conf = NULL;
 
     if (json_unpack_ex (entry, &json_err, 0,
-                        "{s?:s s?:o s?:s}",
+                        "{s?s s?o s?s}",
                         "load", &load,
                         "conf", &conf,
                         "remove", &remove) < 0) {
@@ -471,7 +471,7 @@ static int jobtap_parse_config (const flux_conf_t *conf,
     if (!jobtap->configured) {
         if (flux_conf_unpack (conf,
                               &error,
-                              "{s?:{s?:o}}",
+                              "{s?{s?o}}",
                               "job-manager",
                                 "plugins", &plugins) < 0) {
             return errprintf (errp,
diff --git a/src/modules/job-manager/plugins/perilog.c b/src/modules/job-manager/plugins/perilog.c
@@ -510,7 +510,7 @@ static int conf_init (flux_t *h, struct perilog_conf *conf)
 
     if (flux_conf_unpack (flux_get_conf (h),
                           &error,
-                          "{s?:{s?:{s?o s?F !} s?:{s?o !}}}",
+                          "{s?{s?{s?o s?F !} s?{s?o !}}}",
                           "job-manager",
                           "prolog",
                             "command", &prolog,
diff --git a/src/modules/job-manager/raise.c b/src/modules/job-manager/raise.c
@@ -114,7 +114,7 @@ void raise_handle_request (flux_t *h,
     const char *note = NULL;
     const char *errstr = NULL;
 
-    if (flux_request_unpack (msg, NULL, "{s:I s:i s:s s?:s}",
+    if (flux_request_unpack (msg, NULL, "{s:I s:i s:s s?s}",
                                         "id", &id,
                                         "severity", &severity,
                                         "type", &type,
@@ -215,7 +215,7 @@ void raiseall_handle_request (flux_t *h,
 
     if (flux_request_unpack (msg,
                              NULL,
-                             "{s:b s:i s:i s:i s:s s?:s}",
+                             "{s:b s:i s:i s:i s:s s?s}",
                              "dry_run",
                              &dry_run,
                              "userid",
diff --git a/src/modules/job-manager/start.c b/src/modules/job-manager/start.c
@@ -245,9 +245,9 @@ static void start_response_cb (flux_t *h, flux_msg_handler_t *mh,
         int xseverity;
         const char *xtype;
         const char *xnote = NULL;
-        if (json_unpack (data, "{s:i s:s s?:s}", "severity", &xseverity,
-                                                 "type", &xtype,
-                                                 "note", &xnote) < 0) {
+        if (json_unpack (data, "{s:i s:s s?s}", "severity", &xseverity,
+                                                "type", &xtype,
+                                                "note", &xnote) < 0) {
             errno = EPROTO;
             flux_log_error (h, "start: exception response: malformed data");
             goto error;
diff --git a/src/modules/job-manager/wait.c b/src/modules/job-manager/wait.c
@@ -92,7 +92,7 @@ static int decode_job_result (struct job *job,
         const char *note = NULL;
 
         if (json_unpack (context,
-                         "{s:s s?:s}",
+                         "{s:s s?s}",
                          "type",
                          &type,
                          "note",
diff --git a/src/modules/resource/drain.c b/src/modules/resource/drain.c
@@ -372,7 +372,7 @@ static void drain_cb (flux_t *h,
 
     if (flux_request_unpack (msg,
                              NULL,
-                             "{s:s s?:s s?:s}",
+                             "{s:s s?s s?s}",
                              "targets", &s,
                              "reason", &reason,
                              "mode", &mode) < 0)
diff --git a/src/modules/resource/resource.c b/src/modules/resource/resource.c
@@ -70,7 +70,7 @@ static int parse_config (struct resource_ctx *ctx,
 
     if (flux_conf_unpack (conf,
                           &error,
-                          "{s?:{s?:s s?:o s?:s s?:b s?b !}}",
+                          "{s?{s?s s?o s?s s?b s?b !}}",
                           "resource",
                             "path", &path,
                             "config", &config,
diff --git a/src/shell/gpubind.c b/src/shell/gpubind.c
@@ -40,7 +40,7 @@ int get_shell_gpus (flux_shell_t *shell,
 
     if (flux_shell_rank_info_unpack (shell,
                                      -1,
-                                     "{s:i s:{s?:s}}",
+                                     "{s:i s:{s?s}}",
                                      "ntasks", ntasks,
                                      "resources",
                                        "gpus", &gpu_list) < 0) {
diff --git a/src/shell/input.c b/src/shell/input.c
@@ -250,7 +250,7 @@ static int shell_input_parse_type (struct shell_input *in)
     int ret;
 
     if ((ret = flux_shell_getopt_unpack (in->shell, "input",
-                                         "{s?:{s?:s}}",
+                                         "{s?{s?s}}",
                                          "stdin", "type", &typestr)) < 0)
         return -1;
 
@@ -265,7 +265,7 @@ static int shell_input_parse_type (struct shell_input *in)
         in->stdin_type = FLUX_INPUT_TYPE_FILE;
 
         if (flux_shell_getopt_unpack (in->shell, "input",
-                                      "{s:{s?:s}}",
+                                      "{s:{s?s}}",
                                       "stdin", "path", &(fp->path)) < 0)
             return -1;
 
diff --git a/src/shell/jobspec.c b/src/shell/jobspec.c
@@ -235,7 +235,7 @@ struct jobspec *jobspec_parse (const char *jobspec, json_error_t *error)
      *  json_t * objects)
      */
     if (json_unpack_ex (job->jobspec, error, 0,
-                        "{s:i s:o s:o s:{s?{s?:s s?:O s?:{s?:O}}}}",
+                        "{s:i s:o s:o s:{s?{s?s s?O s?{s?O}}}}",
                         "version", &job->version,
                         "resources", &resources,
                         "tasks", &tasks,
diff --git a/src/shell/output.c b/src/shell/output.c
diff --git a/src/shell/pty.c b/src/shell/pty.c
