shared/format-table: fix potential memleaks of d->formatted

We don't always return d->formatted, even if it is available. And
depending on the cell type, we'd either overwrite it directly or free
first. Let's always free it upfront and then set unconditionally.
(In this case, we don't need to spend effort on preserving the
existing value. It's just a cache.)  Setting the variable directly
allows many temporary variables to be eliminated.

Also use asprintf_safe() to simplify the allocation of the buffer.
This is probably a tiny bit slower than the direct allocation, but
table formatting shouldn't be a hot path.

diff --git a/src/shared/format-table.c b/src/shared/format-table.c
index 18cebf8628551ec46dbdde81c4644c227f55a122..f400602b343d5f1394560bb16a9d78cb34a0eda7 100644 (file)
--- a/src/shared/format-table.c
+++ b/src/shared/format-table.c
@@ -1616,6 +1616,8 @@ static const char* table_data_format(
             (d->type != TABLE_STRV_WRAPPED || d->formatted_for_width == column_width))
                 return d->formatted;
 
+        d->formatted = mfree(d->formatted);
+
         switch (d->type) {
         case TABLE_EMPTY:
                 return table_ersatz_string(t);
@@ -1649,18 +1651,13 @@ static const char* table_data_format(
 
                         *q = 0;
                         return d->formatted;
-                } else if (d->type == TABLE_FIELD) {
-                        d->formatted = strjoin(s, ":");
-                        if (!d->formatted)
-                                return NULL;
-
-                        return d->formatted;
                 }
 
-                if (bn) {
-                        d->formatted = TAKE_PTR(bn);
-                        return d->formatted;
-                }
+                if (d->type == TABLE_FIELD)
+                        return (d->formatted = strjoin(s, ":"));
+
+                if (bn)
+                        return (d->formatted = TAKE_PTR(bn));
 
                 return d->string;
         }
@@ -1669,26 +1666,20 @@ static const char* table_data_format(
                 if (strv_isempty(d->strv))
                         return table_ersatz_string(t);
 
-                d->formatted = strv_join(d->strv, "\n");
-                if (!d->formatted)
-                        return NULL;
-                break;
+                return (d->formatted = strv_join(d->strv, "\n"));
 
-        case TABLE_STRV_WRAPPED: {
+        case TABLE_STRV_WRAPPED:
                 if (strv_isempty(d->strv))
                         return table_ersatz_string(t);
 
-                char *buf = format_strv_width(d->strv, column_width);
-                if (!buf)
+                d->formatted = format_strv_width(d->strv, column_width);
+                if (!d->formatted)
                         return NULL;
 
-                free_and_replace(d->formatted, buf);
                 d->formatted_for_width = column_width;
                 if (have_soft)
                         *have_soft = true;
-
-                break;
-        }
+                return d->formatted;
 
         case TABLE_BOOLEAN:
                 return yes_no(d->boolean);
@@ -1702,12 +1693,12 @@ static const char* table_data_format(
         case TABLE_TIMESTAMP_RELATIVE_MONOTONIC:
         case TABLE_TIMESTAMP_LEFT:
         case TABLE_TIMESTAMP_DATE: {
-                _cleanup_free_ char *p = NULL;
                 char *ret;
 
-                p = new(char,
-                        IN_SET(d->type, TABLE_TIMESTAMP_RELATIVE, TABLE_TIMESTAMP_RELATIVE_MONOTONIC, TABLE_TIMESTAMP_LEFT) ?
-                                FORMAT_TIMESTAMP_RELATIVE_MAX : FORMAT_TIMESTAMP_MAX);
+                _cleanup_free_ char *p = new(
+                                char,
+                                IN_SET(d->type, TABLE_TIMESTAMP_RELATIVE, TABLE_TIMESTAMP_RELATIVE_MONOTONIC, TABLE_TIMESTAMP_LEFT) ?
+                                        FORMAT_TIMESTAMP_RELATIVE_MAX : FORMAT_TIMESTAMP_MAX);
                 if (!p)
                         return NULL;
 
@@ -1726,16 +1717,13 @@ static const char* table_data_format(
                 if (!ret)
                         return "-";
 
-                d->formatted = TAKE_PTR(p);
-                break;
+                return (d->formatted = TAKE_PTR(p));
         }
 
         case TABLE_TIMESPAN:
         case TABLE_TIMESPAN_MSEC:
         case TABLE_TIMESPAN_DAY: {
-                _cleanup_free_ char *p = NULL;
-
-                p = new(char, FORMAT_TIMESPAN_MAX);
+                _cleanup_free_ char *p = new(char, FORMAT_TIMESPAN_MAX);
                 if (!p)
                         return NULL;
 
@@ -1744,344 +1732,137 @@ static const char* table_data_format(
                                      d->type == TABLE_TIMESPAN_MSEC ? USEC_PER_MSEC : USEC_PER_DAY))
                         return "-";
 
-                d->formatted = TAKE_PTR(p);
-                break;
+                return (d->formatted = TAKE_PTR(p));
         }
 
         case TABLE_SIZE: {
-                _cleanup_free_ char *p = NULL;
-
-                p = new(char, FORMAT_BYTES_MAX);
+                _cleanup_free_ char *p = new(char, FORMAT_BYTES_MAX);
                 if (!p)
                         return NULL;
 
                 if (!format_bytes(p, FORMAT_BYTES_MAX, d->size))
                         return table_ersatz_string(t);
 
-                d->formatted = TAKE_PTR(p);
-                break;
+                return (d->formatted = TAKE_PTR(p));
         }
 
         case TABLE_BPS: {
-                _cleanup_free_ char *p = NULL;
-                size_t n;
-
-                p = new(char, FORMAT_BYTES_MAX+2);
+                _cleanup_free_ char *p = new(char, FORMAT_BYTES_MAX+2);
                 if (!p)
                         return NULL;
 
                 if (!format_bytes_full(p, FORMAT_BYTES_MAX, d->size, FORMAT_BYTES_BELOW_POINT))
                         return table_ersatz_string(t);
 
-                n = strlen(p);
+                size_t n = strlen(p);
                 strscpy(p + n, FORMAT_BYTES_MAX + 2 - n, "bps");
 
-                d->formatted = TAKE_PTR(p);
-                break;
-        }
-
-        case TABLE_INT: {
-                _cleanup_free_ char *p = NULL;
-
-                p = new(char, DECIMAL_STR_WIDTH(d->int_val) + 1);
-                if (!p)
-                        return NULL;
-
-                sprintf(p, "%i", d->int_val);
-                d->formatted = TAKE_PTR(p);
-                break;
-        }
-
-        case TABLE_INT8: {
-                _cleanup_free_ char *p = NULL;
-
-                p = new(char, DECIMAL_STR_WIDTH(d->int8) + 1);
-                if (!p)
-                        return NULL;
-
-                sprintf(p, "%" PRIi8, d->int8);
-                d->formatted = TAKE_PTR(p);
-                break;
-        }
-
-        case TABLE_INT16: {
-                _cleanup_free_ char *p = NULL;
-
-                p = new(char, DECIMAL_STR_WIDTH(d->int16) + 1);
-                if (!p)
-                        return NULL;
-
-                sprintf(p, "%" PRIi16, d->int16);
-                d->formatted = TAKE_PTR(p);
-                break;
-        }
-
-        case TABLE_INT32: {
-                _cleanup_free_ char *p = NULL;
-
-                p = new(char, DECIMAL_STR_WIDTH(d->int32) + 1);
-                if (!p)
-                        return NULL;
-
-                sprintf(p, "%" PRIi32, d->int32);
-                d->formatted = TAKE_PTR(p);
-                break;
-        }
-
-        case TABLE_INT64: {
-                _cleanup_free_ char *p = NULL;
-
-                p = new(char, DECIMAL_STR_WIDTH(d->int64) + 1);
-                if (!p)
-                        return NULL;
-
-                sprintf(p, "%" PRIi64, d->int64);
-                d->formatted = TAKE_PTR(p);
-                break;
-        }
-
-        case TABLE_UINT: {
-                _cleanup_free_ char *p = NULL;
-
-                p = new(char, DECIMAL_STR_WIDTH(d->uint_val) + 1);
-                if (!p)
-                        return NULL;
-
-                sprintf(p, "%u", d->uint_val);
-                d->formatted = TAKE_PTR(p);
-                break;
-        }
-
-        case TABLE_UINT8: {
-                _cleanup_free_ char *p = NULL;
-
-                p = new(char, DECIMAL_STR_WIDTH(d->uint8) + 1);
-                if (!p)
-                        return NULL;
-
-                sprintf(p, "%" PRIu8, d->uint8);
-                d->formatted = TAKE_PTR(p);
-                break;
-        }
-
-        case TABLE_UINT16: {
-                _cleanup_free_ char *p = NULL;
-
-                p = new(char, DECIMAL_STR_WIDTH(d->uint16) + 1);
-                if (!p)
-                        return NULL;
-
-                sprintf(p, "%" PRIu16, d->uint16);
-                d->formatted = TAKE_PTR(p);
-                break;
-        }
-
-        case TABLE_UINT32: {
-                _cleanup_free_ char *p = NULL;
-
-                p = new(char, DECIMAL_STR_WIDTH(d->uint32) + 1);
-                if (!p)
-                        return NULL;
-
-                sprintf(p, "%" PRIu32, d->uint32);
-                d->formatted = TAKE_PTR(p);
-                break;
+                return (d->formatted = TAKE_PTR(p));
         }
 
-        case TABLE_UINT32_HEX: {
-                _cleanup_free_ char *p = NULL;
-
-                p = new(char, 8 + 1);
-                if (!p)
-                        return NULL;
-
-                sprintf(p, "%" PRIx32, d->uint32);
-                d->formatted = TAKE_PTR(p);
-                break;
-        }
-
-        case TABLE_UINT32_HEX_0x: {
-                _cleanup_free_ char *p = NULL;
-
-                p = new(char, 2 + 8 + 1);
-                if (!p)
-                        return NULL;
-
-                sprintf(p, "0x%" PRIx32, d->uint32);
-                d->formatted = TAKE_PTR(p);
-                break;
-        }
+        case TABLE_INT:
+                return (d->formatted = asprintf_safe("%i", d->int_val));
 
-        case TABLE_UINT64: {
-                _cleanup_free_ char *p = NULL;
+        case TABLE_INT8:
+                return (d->formatted = asprintf_safe("%" PRIi8, d->int8));
 
-                p = new(char, DECIMAL_STR_WIDTH(d->uint64) + 1);
-                if (!p)
-                        return NULL;
+        case TABLE_INT16:
+                return (d->formatted = asprintf_safe("%" PRIi16, d->int16));
 
-                sprintf(p, "%" PRIu64, d->uint64);
-                d->formatted = TAKE_PTR(p);
-                break;
-        }
+        case TABLE_INT32:
+                return (d->formatted = asprintf_safe("%" PRIi32, d->int32));
 
-        case TABLE_UINT64_HEX: {
-                _cleanup_free_ char *p = NULL;
+        case TABLE_INT64:
+                return (d->formatted = asprintf_safe("%" PRIi64, d->int64));
 
-                p = new(char, 16 + 1);
-                if (!p)
-                        return NULL;
+        case TABLE_UINT:
+                return (d->formatted = asprintf_safe("%u", d->uint_val));
 
-                sprintf(p, "%" PRIx64, d->uint64);
-                d->formatted = TAKE_PTR(p);
-                break;
-        }
+        case TABLE_UINT8:
+                return (d->formatted = asprintf_safe("%" PRIu8, d->uint8));
 
-        case TABLE_UINT64_HEX_0x: {
-                _cleanup_free_ char *p = NULL;
+        case TABLE_UINT16:
+                return (d->formatted = asprintf_safe("%" PRIu16, d->uint16));
 
-                p = new(char, 2 + 16 + 1);
-                if (!p)
-                        return NULL;
+        case TABLE_UINT32:
+                return (d->formatted = asprintf_safe("%" PRIu32, d->uint32));
 
-                sprintf(p, "0x%" PRIx64, d->uint64);
-                d->formatted = TAKE_PTR(p);
-                break;
-        }
+        case TABLE_UINT32_HEX:
+                return (d->formatted = asprintf_safe("%" PRIx32, d->uint32));
 
-        case TABLE_PERCENT: {
-                _cleanup_free_ char *p = NULL;
+        case TABLE_UINT32_HEX_0x:
+                return (d->formatted = asprintf_safe("0x%" PRIx32, d->uint32));
 
-                p = new(char, DECIMAL_STR_WIDTH(d->percent) + 2);
-                if (!p)
-                        return NULL;
+        case TABLE_UINT64:
+                return (d->formatted = asprintf_safe("%" PRIu64, d->uint64));
 
-                sprintf(p, "%i%%" , d->percent);
-                d->formatted = TAKE_PTR(p);
-                break;
-        }
+        case TABLE_UINT64_HEX:
+                return (d->formatted = asprintf_safe("%" PRIx64, d->uint64));
 
-        case TABLE_IFINDEX: {
-                _cleanup_free_ char *p = NULL;
+        case TABLE_UINT64_HEX_0x:
+                return (d->formatted = asprintf_safe("0x%" PRIx64, d->uint64));
 
-                if (format_ifname_full_alloc(d->ifindex, FORMAT_IFNAME_IFINDEX, &p) < 0)
-                        return NULL;
+        case TABLE_PERCENT:
+                return (d->formatted = asprintf_safe("%i%%" , d->percent));
 
-                d->formatted = TAKE_PTR(p);
-                break;
-        }
+        case TABLE_IFINDEX:
+                (void) format_ifname_full_alloc(d->ifindex, FORMAT_IFNAME_IFINDEX, &d->formatted);
+                return d->formatted;
 
         case TABLE_IN_ADDR:
-        case TABLE_IN6_ADDR: {
-                _cleanup_free_ char *p = NULL;
-
-                if (in_addr_to_string(d->type == TABLE_IN_ADDR ? AF_INET : AF_INET6,
-                                      &d->address, &p) < 0)
-                        return NULL;
-
-                d->formatted = TAKE_PTR(p);
-                break;
-        }
-
-        case TABLE_ID128: {
-                char *p;
+        case TABLE_IN6_ADDR:
+                (void) in_addr_to_string(d->type == TABLE_IN_ADDR ? AF_INET : AF_INET6,
+                                         &d->address,
+                                         &d->formatted);
+                return d->formatted;
 
-                p = new(char, SD_ID128_STRING_MAX);
-                if (!p)
+        case TABLE_ID128:
+                d->formatted = new(char, SD_ID128_STRING_MAX);
+                if (!d->formatted)
                         return NULL;
 
-                d->formatted = sd_id128_to_string(d->id128, p);
-                break;
-        }
-
-        case TABLE_UUID: {
-                char *p;
+                return sd_id128_to_string(d->id128, d->formatted);
 
-                p = new(char, SD_ID128_UUID_STRING_MAX);
-                if (!p)
+        case TABLE_UUID:
+                d->formatted = new(char, SD_ID128_UUID_STRING_MAX);
+                if (!d->formatted)
                         return NULL;
 
-                d->formatted = sd_id128_to_uuid_string(d->id128, p);
-                break;
-        }
-
-        case TABLE_UID: {
-                char *p;
+                return sd_id128_to_uuid_string(d->id128, d->formatted);
 
+        case TABLE_UID:
                 if (!uid_is_valid(d->uid))
                         return table_ersatz_string(t);
 
-                p = new(char, DECIMAL_STR_WIDTH(d->uid) + 1);
-                if (!p)
-                        return NULL;
-                sprintf(p, UID_FMT, d->uid);
-
-                d->formatted = p;
-                break;
-        }
-
-        case TABLE_GID: {
-                char *p;
+                return (d->formatted = asprintf_safe(UID_FMT, d->uid));
 
+        case TABLE_GID:
                 if (!gid_is_valid(d->gid))
                         return table_ersatz_string(t);
 
-                p = new(char, DECIMAL_STR_WIDTH(d->gid) + 1);
-                if (!p)
-                        return NULL;
-                sprintf(p, GID_FMT, d->gid);
-
-                d->formatted = p;
-                break;
-        }
-
-        case TABLE_PID: {
-                char *p;
+                return (d->formatted = asprintf_safe(GID_FMT, d->gid));
 
+        case TABLE_PID:
                 if (!pid_is_valid(d->pid))
                         return table_ersatz_string(t);
 
-                p = new(char, DECIMAL_STR_WIDTH(d->pid) + 1);
-                if (!p)
-                        return NULL;
-                sprintf(p, PID_FMT, d->pid);
-
-                d->formatted = p;
-                break;
-        }
+                return (d->formatted = asprintf_safe(PID_FMT, d->pid));
 
         case TABLE_SIGNAL: {
-                const char *suffix;
-                char *p;
-
-                suffix = signal_to_string(d->int_val);
+                const char *suffix = signal_to_string(d->int_val);
                 if (!suffix)
                         return table_ersatz_string(t);
 
-                p = strjoin("SIG", suffix);
-                if (!p)
-                        return NULL;
-
-                d->formatted = p;
-                break;
+                return (d->formatted = strjoin("SIG", suffix));
         }
 
-        case TABLE_MODE: {
-                char *p;
-
+        case TABLE_MODE:
                 if (d->mode == MODE_INVALID)
                         return table_ersatz_string(t);
 
-                p = new(char, 4 + 1);
-                if (!p)
-                        return NULL;
-
-                sprintf(p, "%04o", d->mode & 07777);
-                d->formatted = p;
-                break;
-        }
+                return (d->formatted = asprintf_safe("%04o", d->mode & 07777));
 
         case TABLE_MODE_INODE_TYPE:
-
                 if (d->mode == MODE_INVALID)
                         return table_ersatz_string(t);
 
@@ -2091,28 +1872,18 @@ static const char* table_data_format(
                 if (devnum_is_zero(d->devnum))
                         return table_ersatz_string(t);
 
-                if (asprintf(&d->formatted, DEVNUM_FORMAT_STR, DEVNUM_FORMAT_VAL(d->devnum)) < 0)
-                        return NULL;
-
-                break;
+                return (d->formatted = asprintf_safe(DEVNUM_FORMAT_STR, DEVNUM_FORMAT_VAL(d->devnum)));
 
-        case TABLE_JSON: {
+        case TABLE_JSON:
                 if (!d->json)
                         return table_ersatz_string(t);
 
-                char *p;
-                if (sd_json_variant_format(d->json, /* flags= */ 0, &p) < 0)
-                        return NULL;
-
-                d->formatted = p;
-                break;
-        }
+                (void) sd_json_variant_format(d->json, /* flags= */ 0, &d->formatted);
+                return d->formatted;
 
         default:
                 assert_not_reached();
         }
-
-        return d->formatted;
 }
 
 static const char* table_data_format_strip_ansi(