During the refactorings of the last months, several comments have become
outdated, some are now redundant since the code is as clear as the
comment, and some code benefits from a bit of explanation.
These are easier to read than hex constants.
There was no need to skip bits 2 and 3 (there were no constants for 0x04
and 0x08). Close this gap, to avoid confusing future readers. Keep the
relative order of the flags since that affects the debug output of -dv.
No functional change.
There are only very few places in var.c that contain really duplicate
code anymore.
There is still lots of _almost_ duplicate, for example the code for
parsing variable modifiers. It differs subtly in behavior:
* The modifiers ':M' and ':N' use '$$' to escape a '$' sign, while
almost all other modifiers use '\$' for this purpose.
* The modifiers ':M', ':N', ':S', ':@' and several others parse
balanced parentheses and braces, allowing '(' to '}' to match.
The modifiers ':D' and ':U' only treat the end character special but
not the other 3 of '(){}'.
* When parsing the modifier ':S' but not evaluating it, the code for
nested variable expressions is parsed differently from when it is in
evaluation mode (VARE_WANTRES). This applies to an outer ':S'
modifier and an inner ':D' or ':M' modifier.
Since these inconsistencies affect the behavior in edge cases and some
users of make might depend on it, they cannot be fixed by
behavior-preserving refactorings.
Correctly handle (ie: ignore completely) \0 chars (nuls) in the
shell command input stream (script, dot file, or stdin).
Previously nul chars were ignored correctly in the line in which
they occurred, but would cause trailing chars of that line to reappear
as the start of the following line. If there was just one \0 skipped,
this would generally result in an extra \n in the sh input, which in
most cases has no effect. With multiple \0's in a single line, more
of the end of that line was duplicated into the following one. This
usually manifested as a weird "command not found" error.
Note that any \0 chars in the sh input make the script non-conforming,
so fixing this is not crucial (no \0's should really ever be seen) but
it was an obvious bug in the code, which was attempting to ignore nul
chars (as do many other shells), so let it be fixed.
XXX pullup -9
This fixes the MSAN detected reference to an unitialised variable
(an unitialised field in a struct) which happens when a command is
not found after a PATH search.
Aside from skipping some known to be going to fail exec*() calls
in some cases, the setting of the relevant field is irrelevant,
so this problem makes no practical difference to the shell, or any
shell script.
XXX (maybe) pullup -9
locked and referenced across the call to swap_off() and finally
use it from vfs_unmountall1() to remove swap after unmounting
the last file system.
Adresses PR kern/54969 (Disk cache is no longer flushed on shutdown)
Prevents null pointer dereferences when ZFS replaces this
illegal (according to IS_EPHEMERAL()) id with another
illegal id in operation zfs_fuid_create_cred() and
finally zfs_log_create() dereferences fuidp being NULL.
Adresses PR misc/55042 (Panic when creating a directory on a NFS served ZFS)
Add a sh ATF test to demonstrate a bug in the way that \0 characters
are dropped from scripts. This test will eventually be extended to
test other potential sh script input related issues.
When initially committed, this test should fail. It should succeed
when the fix for the PR is committed (soon).
Nb: this tests only the \0 related issues from the PR, the MSAN
detected uninitialised variable (struct field) can only be detected
by MSAN, as it has no visible impact on the operation of the shell
when running on any real (or even emulated) hardware.
(It will, however, also be fixed).
In lists/base/mi, the directory ./libdata/firmware contains several
subdirectories for which the entries do not look aligned right now, even
after normalizing the alignment.
This is because the indentation for a directory is not determined
globally for all directory entries from the complete file, but instead
for each group of items, after sorting, that have the same directory.
This results in several entries having only a single item per group,
such as ./libdata/firmware/nouveau, and this single item is obviously
consistently indented since all of its 1 lines are already indented to
the same depth.
One possible solution for this is to sort the entries in another order,
keeping all entries from a directory together. This allows to quickly
see all entries from a certain directory, but on the other hand, when
adding a new directory plus some entries, the directory has to be listed
far away from its entries. (This would be done automatically by
fmt-list though, which weakens this argument a bit.)
Another possible solution for this is to first determine the indentation
for all entries from each directory, no matter how far these entries are
apart, and then indent these entries to the common indentation. This
may or may not help since there will still be single entries between
groups of differently indented entries.
Writing down the intuitive rules for how to align the entries properly
is a nontrivial task. See pkglint/varalignblock, which is around 650
lines of code, plus around 4000 lines of test code.
No functional change.
SUsPerPU is only really supported for a value of 1, and since the
first PSID is 0, the last will be numStripe-1. Also update the
setting of pending_writes to reflect the change to lastPSID.
Needs pullups to -8 and -9.
Seen in lists/modules/ad.aarch64, among others. These lines are not
intended as comments but as regular entries that just happen to be
commented out.
This is the same as how pkglint treats commented variable assignments,
which take part in aligning blocks of variable assignments even though
they are syntactically comments.
The type describes the definedness of an expression, not a general
status, therefore the new name is more precise.
The constants are renamed as well since their prefix 'VES' does not
match the type name anymore, it was correct 3 days ago when the type was
still named VarExprStatus. The name VES_NONE was misleading since
'none' does not describe its actual effect. That name came from the
time when the status was a bit set, and 'none' simply meant 'none of the
bits are set'.
The names used in debug logging will be renamed in a follow-up commit,
to demonstrate that the changes in this commit indeed have no functional
change, especially not the change from '!=' to '==' in line 4304.
No functional change.
The details of how variable expressions are evaluated is controlled by
several parameters: startc and endc differ for $(VAR) and ${VAR}, the
value of the expression can be interpreted as a single big word, and
when joining several words (such as with ':M' or ':S'), there may be a
custom word separator (defined with ':ts*').
The scope of half of these parameters is the whole variable expression,
the other half of the parameters are reset after each chain of indirect
modifiers. To make this distinction obvious in the code, extract Expr
from ApplyModifiersState. Previously, these details were hidden in how
parameters are passed and restored among ApplyModifiersIndirect and
ApplyModifiers.
The changes in the individual ApplyModifier functions are numerous but
straight-forward. They mostly replace 'st' with 'expr'.
The changes in ApplyModifiers and ApplyModifiersIndirect are more
subtle. The value of the expression is no longer passed around but is
stored in a fixed location, in Expr, which makes it easier to reason
about memory management.
The code in ApplyModifiers after 'cleanup' looks quite different but
preserves the existing behavior. Expr_SetValueRefer is nothing else
than the combination of FStr_Done followed by FStr_InitRefer. Storing
exprStatus back at the end was responsible for passing the definedness
of the expression after applying the indirect modifiers back to the
outer ApplyModifiersState. The same effect is now achieved by having
Expr.status with a wider scope.
No functional change.
The caller has already provided daif state. No need to keep updating
daif via pic_set_priority if it's already in the state we need (interrupts
disabled).