clang says:
src/dissect/dissect.c:292:17: warning: label followed by a declaration is a C23 extension [-Wc23-extensions]
Another option would be to raise the C standard to C23. I think that'd
make sense, but it's better to do it separately.
Some cosmetic differences in descriptions are made.
--quiet was miscategorized as a command (172fadda65),
--usr-hash and --usr-hash-sig were not listed (10b8d65f3f).
The behaviour that was implemented in systemd-dissect was that
both '--exec -- cmd' and '--exec cmd' result in 'cmd' as the command,
and '--' anywhere later is as a positional argument, so nesting is
possible, e.g.:
--exec -- cmd --opt -- another-cmd --another-opt
This is not obvious, so add some tests for this and keep it as a separate
commit.
Add tests for OPTION_STOPS_PARSING, OPTION_GROUP_MARKER, and
OPTION_OPTIONAL_ARG flags with manual Option arrays, and a separate
test exercising the OPTION, OPTION_LONG, OPTION_SHORT, OPTION_FULL,
and OPTION_GROUP macros via FOREACH_OPTION_FULL in a switch statement,
as they would be used in real code.
Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com>
The test cases are the same in both files. To make the test
more through, add case where "--" is used more than once and
also when options are present after "--".
The basic idea is that we'll have "one source of truth" for the list of
options. Currently, this is split between:
1. struct option options[] array for long options
2. the short option parameter to getopt_long()
3. --help
so it is easy to forget to add or update one of those places where
appropriate.
An option is defined through a macro that includes the option short
and long codes, and also the metavar and help. Those four items can
be used to generate the help string automatically.
The code is easier to read when various parts are written in the same
order.
We can define common options through a macro in the header file,
reducing boilerplate repeated in different files. Over time, if we
discover that the same pattern is used in multiple files, we can add
another "common option".
The macro is defined in a way that the editor can indent it like a
normal case statement.
The error message for ambiguous options is formatted a bit differently:
$ systemd-id128 --no-
systemd-id128: option '--no-' is ambiguous; possibilities: '--no-pager' '--no-legend'
$ build/systemd-id128 --no-
option '--no-' is ambiguous; possibilities: --no-pager, --no-legend
I think the formatting without commas is ugly, but OTOH, the quotes
around option names are superfluous, real option names are easy to
distinguish.
Also, in general we prefer variables that are always defined over
checking with #ifdef, so #if defined(HAVE_NO_STACK_PROTECTOR_ATTRIBUTE)
is something that we want to avoid.
This attribute was introduced in gcc 11, and our baseline is currently
8.4. So let's allow using _retain_ everywhere, but make it into a noop
if not supported.
Using __has_attribute was suggested, but with gcc-11.5.0-14.el9.x86_64,
__has__attribute(__retain__) is true, but we get a warning when the
attribute is actually used.
In one of the reviews one of the LLMs noticed that the pragma is set but
never unset, so it remains in effect for the rest of the translation
unit. From the comment, it's not clear how old those "old compilers" were,
so let's try if things work without this workaround.
Let's stop using --json-schema and instead have claude write a JSON
file in the repo root which we pass around as an artifact similar to
how we pass around the input. This works around the bug where claude
receives task notifications after producing structured output which
breaks the structured output.
We often have a pattern where the same verb function is used for
multiple actions. This leads to an antipattern where we figure out what
action needs to be taken from argv[0] multiple times: often once in
arse_argv() to figure out what options are allowed, then once again
implicitly in dispatch_verb(), and then again in the action verb itself.
Let's allow passing a parameter into the verb to simplify this.
As it seems impossible to prevent claude from receiving notifications
about subagents finishing after it has produced structured output, which
breaks the structured output as it has to be the final reply, let's stop
using subagents and background tasks completely to avoid the issue.
tpm_log_tagged_event() only measures the event to the TPM while
tpm_log_ipl_event() measures the event both to the TPM and CC. Fix the
inconsistency.
Note, this is a potentially breaking change for TDX guests as systemd will
now measure more stuff to the MRTD/RTMRs, reference values for attestation may
need to be adjusted.
Found by Claude Code Review.
If this new scheme of adding dependencies is supposed to be used more
widely we need to start making it easy to add them. So add a new
self-contained header that projects can simply include without the need
to link against libsystemd itself. This will allow them to generate
`.notes.dlopen` sections:
```
> readelf -p .note.dlopen ./l2md
String dump of section '.note.dlopen':
[ a] |@FDO
[ 10] [{"feature":"manifest-json","description":"Manifest-based change detection via gzip and JSON parsing","priority":"suggested","soname":["libz.so.1","libsystemd.so.0"]}]
[ c2] |@FDO
[ c8] [{"feature":"manifest-http","description":"HTTP transport for lore.kernel.org manifest fetch","priority":"suggested","soname":["libcurl.so.4"]}]
```
The SYSTEMD_COLORS=true/1/yes no longer forced colors
when stdout was not a TTY (e.g. piped), because the COLOR_TRUE bypass
of the terminal_is_dumb() check was accidentally dropped.
Restore the old behavior by guarding the TTY check with
`m != COLOR_TRUE`, so an explicit boolean "true" value continues to
unconditionally force color output regardless of whether stdout is a TTY
or whether $NO_COLOR is set.
claude keeps failing by its subagents completing after it has already
written the review for large prs. It seems to run out of turns, tries
to get the subagents to post partial reviews but doesn't seem to stop
them.
Let's insist that it waits for background tasks to stop but let's also
increase the max turns a bit so it doesn't run out as quickly.
claude wants to use python to access the JSON context so let's allow
it. Since python3 basically allows you to reimplement every other tool,
let's just enable all tools except the web related ones but enable network
isolation so it can't try to exfiltrate anything via python.
Add a --grain-size= CLI option to override the default 4 KiB partition
alignment grain. Setting --grain-size=1M matches the alignment used by
fdisk/parted and fixes misaligned partitions after small fixed-size
partitions like the 16 KiB verity-sig partition.
Also fix context_place_partitions() to re-align the start offset after
each partition, not just once per free area. Without this, a small
partition would cause all subsequent partitions in the same free area
to start at an unaligned offset.
In CI we get spurious failures about unitialized variables with gcc
versions older then (depending on the case) 12, 13, or 14. Let's only
try to do this check with newer gcc which returns more useful results.
At the same time, do compile with both gcc and clang at -O2, just
disable the warning.
The old logic seems to have been confused. We compile with -Wall, at
least in some cases, which includes -Wmaybe-unitialized. So if we
_don't_ want it, we need to explicitly disable it.
We often have a pattern where the same verb function is used for
multiple actions. This leads to an antipattern where we figure out what
action needs to be taken from argv[0] multiple times: often once in
parse_argv() to figure out what options are allowed, then once again
implicitly in dispatch_verb(), and then again in the action verb itself.
Let's allow passing a parameter into the verb to simplify this.
This matches a pattern we have in conf-parser.h, where we have both
void *userdata (more global) and void *data (per-config item). Here,
I opted for uintptr_t userdata. It seems that most of the time we'll
want to just pass an enum value. This works OK with no casts. I also
tried a void* and union. In both cases, much more boilerplate is needed:
either a cast or a macro to help avoid compiler warnings. uintptr_t
seems generic enough to cover foreseeable usecases with no fuss.
This is a noop refactoring. See next commit for an example.
