The variadic pair-wise variant was only used by the hand-written help()
functions to synchronize their verb and option tables. Those are all
gone, and the verbs machinery uses table_sync_all_column_widths().
Mixed-type cells were compared by their insertion indices, while cells
of the same type were compared by value. Combining those rules
could make `cell_data_compare()` non-transitive.
Give different non-empty `TableDataType` values a stable order. Define
empty cells as greater than non-empty cells, so they appear last in
ascending sorts and first when the order is reversed. This ensures that
the comparator defines a consistent ordering.
Fixes#43128.
In format-table.h, TABLE_IN_ADDR is commented as "Takes a union in_addr_union
(or a struct in_addr)". However, if we pass struct in_addr to table_add_many(),
the function reads more than the size of the struct.
We generally want to print an error message if table_print()
fails. Add a helper function for this and use it consistently.
This does one of the three things depending on the call site:
- a no-change reformatting of the code
- change from a custom message to the generic one
- addition of the error message where previously none was printed
In the third case, the actual use impact is very small, since the
table formatting is very unlikely to fail. But if it did, we would
often return an error without any message whatsoever, which we
never want to do.
Almost all callers of table_print() specify stdout or NULL (equivalent
to stdout) as the output stream. Simplify things by not requiring the
the stream to be specified.
In almost all cases, the printing of the table is surrounded by normal
printfs() that don't do explicit flushing and for which we don't check
the output stream status. Let's simplify most callers and skip this
step. The reason is not so much to avoid the extra step itself, but
instead to avoid the _handling_ of the potential failure. We generally
only want to print an error message for ENOMEM and other "internal"
errors, so strictly speaking we should filter out the errors from the
stream. By skipping the flush&check step we implicitly do this.
Some functions are in the hot path of a sort. While it is more
correct to also use assert() here for the passed pointers it is
also a runtime cost that should never be needed when e.g. qsort()
is used as it will always pass non-NULL pointers.
So for these critical functions use POINTER_MAY_BE_NULL() which
is compiled out and add a comment to them. Not all _compare()
functions are changed, it was a bit of a judgement call because
some (like ether_addr_compare) are not called with that much data.
But happy to get advise here and tweak accordingly.
The column index is moved to the first position. I think we're unlikely
to want to synchronize widths of *different* columns, and having just
one column argument makes the callers simpler. Also, the type is changed
to size_t to match other functions, and this avoids the need to cast
to size_t in the callers.
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.
For various usecases it's useful that we can embed ANSI sequences in
cells of tables. For example, I hope we can eventually switch "systemctl
status" output to use the table formatter, and multiple of its fields
contain ANSI sequences (since they pack multiple different pieces
information into the same field, and highlight parts of it to
communicate relevance of distinct parts).
Add a distinct cell type for this, which gets special processing when we
output to a terminal that doesn't support ANSI sequences, and to JSON:
we strip the sequences.
Now that the necessary functions from log.h have been moved to macro.h,
we can stop including log.h in macro.h. This requires modifying source
files all over the tree to include log.h instead.
Admittedly, some of our glyphs _are_ special, e.g. "O=" for SPECIAL_GLYPH_TOUCH ;)
But we don't need this in the name. The very long names make some invocations
very wordy, e.g. special_glyph(SPECIAL_GLYPH_SLIGHTLY_UNHAPPY_SMILEY).
Also, I want to add GLYPH_SPACE, which is not special at all.
We often used a pattern like if (!FLAGS_SET(flags, SD_JSON_FORMAT_OFF)),
which is rather verbose and also contains a double negative, which we try
to avoid. Add a little helper to avoid an explicit bit check.
This change clarifies an aditional thing: in some cases we treated
SD_JSON_FORMAT_OFF as a flag (flags & SD_JSON_FORMAT_OFF), while in other cases
we treated it as an independent enum value (flags == SD_JSON_FORMAT_OFF).
In the first form, flags like SD_JSON_FORMAT_SSE do _not_ turn the json
output on, while in the second form they do. Let's use the first form
everywhere.
No functional change intended.
Initially I wasn't sure if this helper should be made public or just internal,
but it seems such a common pattern that if we expose the flags, we might just
as well expose it too, to make life easier for any consumers.
First, when displaying JSON we convert dashes into underscores. We want
to avoid using dashes in JSON field names in new code, because some
JSON parsers don't support dashes very well.
Second, we make the first character of every word lower-case. This
better matches our JSON field name style, and makes the automatic
JSON name mangling a lot more useful for vertical tables, where fields
are given a display name. For example, "Foo Bar" would be converted into
"foo_bar" instead of "Foo_Bar", which much better matches our style.
We don't make the whole string lowercase to support cases like:
"fooBar" should stay as "fooBar".
Some situations don't behave quite perfectly, such as "Foo BarBaz" gets
converted into "foo_barBaz", or all-caps headings get mangled
incorrectly. In these situations, the JSON field should be overridden
manually. In most cases, or at least more cases than before, this
heuristic does good enough.
Lets you conveniently set JSON field names in table_add_many. Especially
useful for vertical tables. For example:
table_add_many(t,
TABLE_FIELD, "Display Name",
TABLE_STRING, obj->display_name,
TABLE_SET_JSON_FIELD_NAME, "displayName",
TABLE_FIELD, "Timestamp",
TABLE_TIMESTAMP, obj->timestamp,
TABLE_SET_JSON_FIELD_NAME, "timestampUSec");
This is preparation for making our Varlink API a public API. Since our
Varlink API is built on top of our JSON API we need to make that public
first (it's a nice API, but JSON APIs there are already enough, this is
purely about the Varlink angle).
I made most of the json.h APIs public, and just placed them in
sd-json.h. Sometimes I wasn't so sure however, since the underlying data
structures would have to be made public too. If in doubt I didn#t risk
it, and moved the relevant API to src/libsystemd/sd-json/json-util.h
instead (without any sd_* symbol prefixes).
This is mostly a giant search/replace patch.
This allows using it with CLEANUP_ARRAY(). For the 2 call sites
where we don't need to free the array, we do a regular for loop
calling json_variant_unref() instead.
When closing the FILE handle attached to a memstream, it may attempt to
do a realloc() that may fail during OOM situations, in which case we are
left with the buffer pointer pointing to NULL and buffer size > 0. For
example:
```
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
void *realloc(void *ptr, size_t size) {
return NULL;
}
int main(int argc, char *argv[])
{
FILE *f;
char *buf;
size_t sz = 0;
f = open_memstream(&buf, &sz);
if (!f)
return -ENOMEM;
fputs("Hello", f);
fflush(f);
printf("buf: 0x%lx, sz: %lu, errno: %d\n",
(unsigned long) buf, sz, errno);
fclose(f);
printf("buf: 0x%lx, sz: %lu, errno: %d\n",
(unsigned long) buf, sz, errno);
return 0;
}
```
```
$ gcc -o main main.c
$ ./main
buf: 0x74d4a0, sz: 5, errno: 0
buf: 0x0, sz: 5, errno: 0
```
This might do unexpected things if the underlying code expects a valid
pointer to the memstream buffer after closing the handle.
Found by Nallocfuzz.
This just adds the base functionality and some unit tests.
With TABLE_TIMESTAMP_RELATIVE we print "5s ago" and "5s left",
with the new TABLE_TIMESTAMP_LEFT, we print "5s ago" but "5s".
This is more useful for cases where we generally only want to
print timestamps in the future.
Zbigniew Jędrzejewski-Szmek
logical-misha
Yu Watanabe
Lennart Poettering
Michael Vogt
Lennart Poettering
Daan De Meyer
Zbigniew Jędrzejewski-Szmek
Mike Yuan
Adrian Vovk
Frantisek Sumsal