In general we almost never hit those asserts in production code, so users see
them very rarely, if ever. But either way, we just need something that users
can pass to the developers.
We have quite a few of those asserts, and some have fairly nice messages, but
many are like "WTF?" or "???" or "unexpected something". The error that is
printed includes the file location, and function name. In almost all functions
there's at most one assert, so the function name alone is enough to identify
the failure for a developer. So we don't get much extra from the message, and
we might just as well drop them.
Dropping them makes our code a tiny bit smaller, and most importantly, improves
development experience by making it easy to insert such an assert in the code
without thinking how to phrase the argument.
We recently started making more use of malloc_usable_size() and rely on
it (see the string_erase() story). Given that we don't really support
sytems where malloc_usable_size() cannot be trusted beyond statistics
anyway, let's go fully in and rework GREEDY_REALLOC() on top of it:
instead of passing around and maintaining the currenly allocated size
everywhere, let's just derive it automatically from
malloc_usable_size().
I am mostly after this for the simplicity this brings. It also brings
minor efficiency improvements I guess, but things become so much nicer
to look at if we can avoid these allocation size variables everywhere.
Note that the malloc_usable_size() man page says relying on it wasn't
"good programming practice", but I think it does this for reasons that
don't apply here: the greedy realloc logic specifically doesn't rely on
the returned extra size, beyond the fact that it is equal or larger than
what was requested.
(This commit was supposed to be a quick patch btw, but apparently we use
the greedy realloc stuff quite a bit across the codebase, so this ends
up touching *a*lot* of code.)
I think this formatting was originally used because it simplified
adding new options to the help messages. However, these days, most
tools their help message end with "\nSee the %s for details.\n" so
the final line almost never has to be edited which eliminates the
benefit of the custom formatting used for printf() help messages.
Let's make things more consistent and use the same formatting for
printf() help messages that we use everywhere else.
Prompted by https://github.com/systemd/systemd/pull/18355#discussion_r567241580
Fixed below systemd codesonar warning.
isprint() is invoked here with an argument of signed
type char, but only has defined behavior for int arguments that are
either representable as unsigned char or equal to the value
of macro EOF(-1).
As per codesonar report, in a number of libc implementations, isprint()
function implemented using lookup tables (arrays): passing in a
negative value can result in a read underrun.
Found by inspecting results of running this small program:
int main(int argc, const char **argv) {
for (int i = 1; i < argc; i++) {
FILE *f;
char line[1024], prev[1024], *r;
int lineno;
prev[0] = '\0';
lineno = 1;
f = fopen(argv[i], "r");
if (!f)
exit(1);
do {
r = fgets(line, sizeof(line), f);
if (!r)
break;
if (strcmp(line, prev) == 0)
printf("%s:%d: error: dup %s", argv[i], lineno, line);
lineno++;
strcpy(prev, line);
} while (!feof(f));
fclose(f);
}
}
This is convenient when working with device units in systemd. Instead of
converting the systemd unit name to a path to feed to udevadm, udevadm
info|trigger can be called directly on the unit name.
The man page is reworked a bit to describe the modern syntax with positional
arguments first. It's just simpler to use than the positional options.
udevadm would dump help() output, instead of printing a message about what is
wrong. That's just bad UX. Let's use a different message if the argument is
missing, and a different one if it is invalid.
Also, rework the code to separate the business logic from argument parsing.
Let's not use "default:" in switch statements. This way, the compiler will warn
us if we miss one of the cases.
This effectively reverts 2bc54be485
and relevant changes in #9920, as it is used to determine the version
of udev, e.g., dracut.
Fixesdracutdevs/dracut#468.
$ git grep -e 'This program is free software' -l |grep -v LICENSE | \
xargs perl -i -0pe 's/ \* This program.*?for more details.\s*\*\n( \* You should have.*licenses.>.\n)?//gms'
For some reason they were missed previously. All those files seem to
have proper SDPX tags.
This drops a good number of type-specific _cleanup_ macros, and patches
all users to just use the generic ones.
In most recent code we abstained from defining type-specific macros, and
this basically removes all those added already, with the exception of
the really low-level ones.
Having explicit macros for this is not too useful, as the expression
without the extra macro is generally just 2ch wider. We should generally
emphesize generic code, unless there are really good reasons for
specific code, hence let's follow this in this case too.
Note that _cleanup_free_ and similar really low-level, libc'ish, Linux
API'ish macros continue to be defined, only the really high-level OO
ones are dropped. From now on this should really be the rule: for really
low-level stuff, such as memory allocation, fd handling and so one, go
ahead and define explicit per-type macros, but for high-level, specific
program code, just use the generic _cleanup_() macro directly, in order
to keep things simple and as readable as possible for the uninitiated.
Note that before this patch some of the APIs (notable libudev ones) were
already used with the high-level macros at some places and with the
generic _cleanup_ macro at others. With this patch we hence unify on the
latter.
This adds missing options, mainly '--version' in getopt(), removes
an unused option from getopt().
Also, this adds a deprecate message in `udevadm hwdb`, and cleanups
help messages.
Follow-up for 65eb4378c3.
We defined both $(VERSION) and $(PACKAGE_VERSION) with the same contents.
$(PACKAGE_VERSION) is slightly more descriptive, so settle on that, and
drop the other define.