Refactor hse_err_to_string() to be more sprintf()-like

hse_err_to_string() now returns a size_t representing the needed size of the buffer.
onlyone0001 · Jul 8, 2021 · 1c2cfda · 1c2cfda
1 parent 08a651d
commit 1c2cfda
Show file tree

Hide file tree

Showing 25 changed files with 120 additions and 104 deletions.
diff --git a/cli/cli_util.c b/cli/cli_util.c
@@ -310,7 +310,7 @@ kvdb_info_print(
         if (hse_err_to_errno(err) == ENOENT)
             goto errout;
 
-        hse_err_to_string(err, buf, sizeof(buf), NULL);
+        hse_err_to_string(err, buf, sizeof(buf));
         yaml_field_fmt(yc, "error", "\"kvdb_info_props failed: %s\"", buf);
     }
 
@@ -539,7 +539,7 @@ kvdb_compact_request(const char *kvdb_home, const char *request_type, u32 timeou
         handle = 0;
         if (hse_err_to_errno(err) != EEXIST && hse_err_to_errno(err) != EBUSY) {
             char buf[256];
-            hse_err_to_string(err, buf, sizeof(buf), NULL);
+            hse_err_to_string(err, buf, sizeof(buf));
             fprintf(stderr, "kvdb open %s failed: %s\n", kvdb_home, buf);
             goto err_out;
         }
@@ -552,7 +552,7 @@ kvdb_compact_request(const char *kvdb_home, const char *request_type, u32 timeou
         err = hse_err_to_errno(hse_kvdb_kvs_names_get(handle, &kvs_cnt, &kvs_list));
         if (err) {
             char buf[256];
-            hse_err_to_string(err, buf, sizeof(buf), NULL);
+            hse_err_to_string(err, buf, sizeof(buf));
             fprintf(stderr, "unable to get %s kvs names: %s\n", kvdb_home, buf);
             goto err_out;
         }
@@ -563,7 +563,7 @@ kvdb_compact_request(const char *kvdb_home, const char *request_type, u32 timeou
             err = hse_kvdb_kvs_open(handle, kvs_list[i], 0, NULL, &k);
             if (err) {
                 char buf[256];
-                hse_err_to_string(err, buf, sizeof(buf), NULL);
+                hse_err_to_string(err, buf, sizeof(buf));
                 fprintf(stderr, "kvs open %s failed: %s\n", kvs_list[i], buf);
                 goto err_out;
             }
@@ -585,15 +585,15 @@ kvdb_compact_request(const char *kvdb_home, const char *request_type, u32 timeou
         err = rest_kvdb_comp(socket_path, policy);
         if (err) {
             char buf[256];
-            hse_err_to_string(err, buf, sizeof(buf), NULL);
+            hse_err_to_string(err, buf, sizeof(buf));
             fprintf(stderr, "rest_kvdb_comp(%s) failed: %s\n", policy, buf);
             goto err_out;
         }
 
         err = rest_kvdb_status(socket_path, sizeof(stat_buf), stat_buf);
         if (err) {
             char buf[256];
-            hse_err_to_string(err, buf, sizeof(buf), NULL);
+            hse_err_to_string(err, buf, sizeof(buf));
             fprintf(stderr, "rest_kvdb_status failed: %s\n", buf);
             goto err_out;
         }
@@ -612,7 +612,7 @@ kvdb_compact_request(const char *kvdb_home, const char *request_type, u32 timeou
             err = rest_kvdb_status(socket_path, sizeof(stat_buf), stat_buf);
             if (err) {
                 char buf[256];
-                hse_err_to_string(err, buf, sizeof(buf), NULL);
+                hse_err_to_string(err, buf, sizeof(buf));
                 fprintf(stderr, "rest_kvdb_status failed: %s\n", buf);
                 goto err_out;
             }
@@ -629,7 +629,7 @@ kvdb_compact_request(const char *kvdb_home, const char *request_type, u32 timeou
                 err = rest_kvdb_comp(socket_path, "cancel");
                 if (err) {
                     char buf[256];
-                    hse_err_to_string(err, buf, sizeof(buf), NULL);
+                    hse_err_to_string(err, buf, sizeof(buf));
                     fprintf(stderr, "rest_kvdb_comp cancel failed: %s\n", buf);
                 } else {
                     err = ETIMEDOUT;
@@ -645,7 +645,7 @@ kvdb_compact_request(const char *kvdb_home, const char *request_type, u32 timeou
         err = rest_kvdb_comp(socket_path, "cancel");
         if (err) {
             char buf[256];
-            hse_err_to_string(err, buf, sizeof(buf), NULL);
+            hse_err_to_string(err, buf, sizeof(buf));
             fprintf(stderr, "rest_kvdb_comp cancel failed: %s\n", buf);
             goto err_out;
         }
@@ -657,7 +657,7 @@ kvdb_compact_request(const char *kvdb_home, const char *request_type, u32 timeou
         err = rest_kvdb_status(socket_path, sizeof(stat_buf), stat_buf);
         if (err) {
             char buf[256];
-            hse_err_to_string(err, buf, sizeof(buf), NULL);
+            hse_err_to_string(err, buf, sizeof(buf));
             fprintf(stderr, "rest_kvdb_status failed: %s\n", buf);
             goto err_out;
         }
@@ -700,7 +700,7 @@ hse_kvdb_params(const char *kvdb_home, bool get)
         err = rest_kvdb_params(kvdb_home, bufsz, buf);
         if (err) {
             char buf[256];
-            hse_err_to_string(err, buf, sizeof(buf), NULL);
+            hse_err_to_string(err, buf, sizeof(buf));
             fprintf(
                 stderr,
                 "rest_kvdb_params failed: %s\n"

diff --git a/cli/hse_cli.c b/cli/hse_cli.c
@@ -514,7 +514,7 @@ print_hse_err(struct cli *cli, const char *api, hse_err_t err)
 {
     char msg[256];
 
-    hse_err_to_string(err, msg, sizeof(msg), 0);
+    hse_err_to_string(err, msg, sizeof(msg));
     fprintf(stderr, "%s: error from %s: %s\n", cli->cmd->cmd_path, api, msg);
 }
 

diff --git a/hsejni/src/hsejni.c b/hsejni/src/hsejni.c
@@ -87,7 +87,7 @@ throw_err(JNIEnv *env, const char *func, uint64_t err)
     char err_buf[200];
     char msg[300];
 
-    hse_err_to_string(err, err_buf, sizeof(err_buf), 0);
+    hse_err_to_string(err, err_buf, sizeof(err_buf));
 
     snprintf(msg, sizeof(msg), "%s: %s", func, err_buf);
 
@@ -108,7 +108,7 @@ td_exit_cursor(void *arg)
 
     rc = hse_kvs_cursor_destroy((struct hse_kvs_cursor *)arg);
     if (rc) {
-        hse_err_to_string(rc, err_buf, sizeof(err_buf), 0);
+        hse_err_to_string(rc, err_buf, sizeof(err_buf));
         syslog(LOG_ERR, "(HSE JNI) %s: hse_kvs_cursor_destroy: %s", __func__, err_buf);
     }
 }
@@ -123,7 +123,7 @@ Java_org_micron_hse_API_init(JNIEnv *env, jobject jobj, jlong valBufSize)
     if (hse_rc) {
         char buf[1024];
 
-        hse_err_to_string(hse_rc, buf, 1024, 0);
+        hse_err_to_string(hse_rc, buf, 1024);
         syslog(LOG_ERR, "(HSE JNI) %s: hse_init: %s", __func__, buf);
     }
 

diff --git a/include/hse/hse.h b/include/hse/hse.h
@@ -100,12 +100,11 @@ hse_fini(void);
  * @param err:      Error value returned from an HSE API function
  * @param buf:      Buffer to hold the formatted string
  * @param buf_len:  Length of buffer
- * @param need_len: [out] If non-NULL, the referent size_t will be the needed
- *                  buffer length
- * @return The error's NULL-terminated string representation, possibly truncated
+ * @return The number of characters (excluding the terminating null byte) which would
+ * have been written to the final string if enough space had been available
  */
-HSE_EXPORT char *
-hse_err_to_string(hse_err_t err, char *buf, size_t buf_len, size_t *need_len);
+HSE_EXPORT size_t
+hse_err_to_string(hse_err_t err, char *buf, size_t buf_len);
 
 /**
  * Return an hse_err_t value's errno representation

diff --git a/lib/binding/kvdb_interface.c b/lib/binding/kvdb_interface.c
@@ -1072,10 +1072,14 @@ hse_kvdb_compact_status_get(struct hse_kvdb *handle, struct hse_kvdb_compact_sta
     return 0;
 }
 
-char *
-hse_err_to_string(hse_err_t err, char *buf, size_t buf_sz, size_t *need_sz)
+size_t
+hse_err_to_string(hse_err_t err, char *buf, size_t buf_sz)
 {
-    return merr_strinfo(err, buf, buf_sz, need_sz);
+    size_t need_sz;
+
+    merr_strinfo(err, buf, buf_sz, &need_sz);
+
+    return need_sz;
 }
 
 int

diff --git a/lib/util/src/hse_err.c b/lib/util/src/hse_err.c
@@ -116,37 +116,27 @@ merr_strerror(merr_t err, char *buf, size_t buf_sz)
 char *
 merr_strinfo(merr_t err, char *buf, size_t buf_sz, size_t *need_sz)
 {
-    int off = 0, sz = 0;
+    ssize_t     sz = 0;
+    const char *file = NULL;
 
     if (err) {
-        if (merr_file(err))
-            sz = snprintf(buf, buf_sz, "%s:%d: ", merr_file(err), merr_lineno(err));
+        file = merr_file(err);
+        if (file)
+            sz = snprintf(buf, buf_sz, "%s:%d: ", file, merr_lineno(err));
         if (sz < 0) {
-            size_t tmp_sz = strlcpy(buf, "<error formating error message>", buf_sz);
-
-            if (need_sz)
-                *need_sz = tmp_sz;
-
-            return buf;
+            sz = strlcpy(buf, "<error formating error message>", buf_sz);
+            goto out;
         }
+        if (sz >= buf_sz)
+            goto out;
 
-        if (sz >= buf_sz) {
-            /* This is the case where even the file and line # didn't fit. We can't safely call
-             * merr_strerror() at this point so we tell the caller as much as we can about how
-             * much space is likely needed.
-             */
-            if (need_sz)
-                *need_sz = sz + 200;
-        } else {
-            off = sz;
-            sz = merr_strerror(err, buf + off, buf_sz - off);
-
-            if (need_sz)
-                *need_sz = off + sz;
-        }
+        sz = merr_strerror(err, buf + sz, buf_sz - sz);
     } else {
-        snprintf(buf, buf_sz, "success");
+        sz = strlcpy(buf, "success", buf_sz);
     }
 
+out:
+    if (need_sz)
+        *need_sz = sz < 0 ? 0 : (size_t)sz;
     return buf;
 }
diff --git a/samples/ex1_create.c b/samples/ex1_create.c
@@ -18,7 +18,7 @@ report_error(const char *api, hse_err_t err)
 {
     char message[256];
 
-    hse_err_to_string(err, message, sizeof(message), NULL);
+    hse_err_to_string(err, message, sizeof(message));
     printf("%s: %s\n", api, message);
 }
 

diff --git a/samples/ex3_cursor.c b/samples/ex3_cursor.c
@@ -58,7 +58,8 @@ main(int argc, char **argv)
 
     rc = hse_kvdb_open(kvdb_home, 0, NULL, &kvdb);
     if (rc) {
-        printf("Cannot open kvdb: %s\n", hse_err_to_string(rc, errbuf, sizeof(errbuf), 0));
+        hse_err_to_string(rc, errbuf, sizeof(errbuf));
+        printf("Cannot open kvdb: %s\n", errbuf);
         exit(1);
     }
 

diff --git a/samples/ex4_transactions.c b/samples/ex4_transactions.c
@@ -68,25 +68,22 @@ main(int argc, char **argv)
     /* Open the KVDB and the KVS instances in it */
     rc = hse_kvdb_open(kvdb_home, 0, NULL, &kvdb);
     if (rc) {
-        printf("Cannot open kvdb: %s\n", hse_err_to_string(rc, errbuf, sizeof(errbuf), 0));
+        hse_err_to_string(rc, errbuf, sizeof(errbuf));
+        printf("Cannot open kvdb: %s\n", errbuf);
         exit(1);
     }
 
     rc = hse_kvdb_kvs_open(kvdb, kvs_name1, 0, NULL, &kvs1);
     if (rc) {
-        printf(
-            "Cannot open kvs %s: %s\n",
-            kvs_name1,
-            hse_err_to_string(rc, errbuf, sizeof(errbuf), 0));
+        hse_err_to_string(rc, errbuf, sizeof(errbuf));
+        printf("Cannot open kvs %s: %s\n", kvs_name1, errbuf);
         exit(1);
     }
 
     rc = hse_kvdb_kvs_open(kvdb, kvs_name2, 0, NULL, &kvs2);
     if (rc) {
-        printf(
-            "Cannot open kvs %s: %s\n",
-            kvs_name2,
-            hse_err_to_string(rc, errbuf, sizeof(errbuf), 0));
+        hse_err_to_string(rc, errbuf, sizeof(errbuf));
+        printf("Cannot open kvs %s: %s\n", kvs_name2, errbuf);
         exit(1);
     }
 

diff --git a/samples/ex5_large_val.c b/samples/ex5_large_val.c
@@ -195,13 +195,15 @@ main(int argc, char **argv)
 
     rc = hse_init(0, NULL);
     if (rc) {
-        err_print("Failed to initialize kvdb: %s\n", hse_err_to_string(rc, ebuf, sizeof(ebuf), 0));
+        hse_err_to_string(rc, ebuf, sizeof(ebuf));
+        err_print("Failed to initialize kvdb: %s\n", ebuf);
         exit(1);
     }
 
     rc = hse_kvdb_open(kvdb_home, 0, NULL, &kvdb);
     if (rc) {
-        err_print("Cannot open kvdb: %s\n", hse_err_to_string(rc, ebuf, sizeof(ebuf), 0));
+        hse_err_to_string(rc, ebuf, sizeof(ebuf));
+        err_print("Cannot open kvdb: %s\n", ebuf);
         exit(1);
     }
 

diff --git a/tests/framework/src/fixtures.c b/tests/framework/src/fixtures.c
@@ -27,9 +27,10 @@ mtf_print_errinfo(
     vfprintf(stderr, fmt, ap);
     va_end(ap);
 
-    if (err)
-        fprintf(stderr, "\nError detail: %s\n",
-            hse_err_to_string(err, msgbuf, sizeof(msgbuf), 0));
+    if (err) {
+        hse_err_to_string(err, msgbuf, sizeof(msgbuf));
+        fprintf(stderr, "\nError detail: %s\n", msgbuf);
+    }
 }
 
 void

diff --git a/tools/bnt/bnt.c b/tools/bnt/bnt.c
@@ -295,7 +295,7 @@ eprint(hse_err_t err, const char *fmt, ...)
 
     if (err) {
         emsglen += strlen(strcat(emsg + emsglen, ": "));
-        hse_err_to_string(err, emsg + emsglen, emsgsz - emsglen, NULL);
+        hse_err_to_string(err, emsg + emsglen, emsgsz - emsglen);
         emsglen += strlen(emsg + emsglen);
     }
 

diff --git a/tools/cn_kbdump/cn_kbdump.c b/tools/cn_kbdump/cn_kbdump.c
@@ -77,7 +77,7 @@ fatal(hse_err_t err, char *fmt, ...)
     va_end(ap);
 
     char buf[ERROR_BUF_SIZE];
-    hse_err_to_string(err, buf, sizeof(buf), NULL);
+    hse_err_to_string(err, buf, sizeof(buf));
     fprintf(stderr, "%s: %s: %s\n", progname, msg, buf);
     exit(1);
 }

diff --git a/tools/cn_metrics/cn_metrics.c b/tools/cn_metrics/cn_metrics.c
@@ -633,7 +633,7 @@ main(int argc, char **argv)
 done:
     if (errmsg) {
         char errbuf[1000];
-        hse_err_to_string(rc, errbuf, sizeof(errbuf), 0);
+        hse_err_to_string(rc, errbuf, sizeof(errbuf));
         fprintf(stderr, "Error: %s failed: %s\n", errmsg, errbuf);
     }
 

diff --git a/tools/cndb_log/cndb_log.c b/tools/cndb_log/cndb_log.c
@@ -56,7 +56,7 @@ void
 fatal(char *who, hse_err_t err)
 {
     char buf[ERROR_BUF_SIZE];
-    hse_err_to_string(err, buf, sizeof(buf), NULL);
+    hse_err_to_string(err, buf, sizeof(buf));
 
     hse_log(HSE_ERR "cndb_log: %s: %s", who, buf);
     exit(1);
@@ -351,7 +351,7 @@ printbuf(struct tool_info *ti, u32 cndb_version, struct cndb_hdr_omf *hdr)
     if (err) {
         char errbuf[300];
 
-        hse_err_to_string(err, errbuf, sizeof(errbuf), NULL);
+        hse_err_to_string(err, errbuf, sizeof(errbuf));
 
         printf("%04x: ", fileoff);
         printf("error unpacking record type %d: %s\n", type, errbuf);

diff --git a/tools/common.c b/tools/common.c
@@ -34,7 +34,7 @@ error(hse_err_t err, char *fmt, va_list ap)
     n = vsnprintf(user_msg, sizeof(user_msg), fmt, ap);
 
     if (err) {
-        hse_err_to_string(err, err_msg + off, sizeof(err_msg) - off, &n);
+        hse_err_to_string(err, err_msg + off, sizeof(err_msg) - off);
         off = min(off + n, sizeof(err_msg) - 1);
 
         n = snprintf(err_msg + off, sizeof(err_msg) - off, " (0x%lx)", err);

diff --git a/tools/ctxn_validation/ctxn_validation.c b/tools/ctxn_validation/ctxn_validation.c
@@ -137,7 +137,7 @@ fatal(uint64_t err, const char *fmt, ...)
     va_end(ap);
 
     if (err) {
-        hse_err_to_string(err, errbuf, sizeof(errbuf), 0);
+        hse_err_to_string(err, errbuf, sizeof(errbuf));
         fprintf(stderr, "%s: %s: %s\n", progname, msg, errbuf);
     }
 
@@ -168,9 +168,10 @@ ctxn_validation_fini(void)
         return;
 
     rc = hse_kvdb_close(kvdb);
-    if (rc)
-        eprint("hse_kvdb_close: %s\n",
-               hse_err_to_string(rc, errbuf, sizeof(errbuf), 0));
+    if (rc) {
+        hse_err_to_string(rc, errbuf, sizeof(errbuf));
+        eprint("hse_kvdb_close: %s\n", errbuf);
+    }
 
     kvdb = NULL;
 }