When both the expected and the actual expression are written in the same
line of the same file, it is easier to compare them and to document
anything interesting. The exp file doesn't provide any space for
comments or explanations.
This macro was obviously wrong since it would have converted file
numbers above 127 to very large numbers, close to 2^32.
The fact that it didn't blow up at all is that this macro was only ever
given the file descriptor 4 as an argument, which can well be
represented as a char, be it signed or unsigned. And this is how the
story goes:
In Job_Init, two jobs are started. The server job allocates file
descriptors 15 and above. The childExitJob is created next, and the
pipe that it creates is [3, 4]. Using F_DUPFD, fd 3 is mapped to fd 5,
and fd 3 is closed. After that, fd 4 is mapped to fd 3 (which had just
been closed), and fd 4 is closed.
After this initialization, file descriptors 0, 1, 2, 3 and 5 are taken.
This leaves a gap at file descriptor 4, and this gap is filled whenever
cmdFILE is created.
Because of this particular order of events, the macro is not necessary.
If it should ever become necessary on platforms like the old SunOS, the
parameter minfd for JobCreatePipe should be increased to 5, which would
leave the file descriptors 3 and 4 to be used by stdio streams.
On 1995-11-03, when the macro was added, SunOS must have been in its
very early development. In Solaris 8 (released in January 2000),
FILE._file is already an unsigned char, therefore the seeming need for
this macro must have been due to an older version, probably from the 2.x
series of SunOS.
There's more to know about variable names than fits in a one-liner.
While here, enforce that the name is not modified by splitting it into
the established (var + var_freeIt) pattern.
Since the name is not modified and not freed in the middle of evaluating
an expression, there is no need to make a backup copy of it. That code
had been necessary more than 12 years ago, but not anymore since the
code got a lot cleaner since then.
While trying to compile the code with GCC's -Wformat-truncation, the
snprintf calls felt quite complicated. The function Dir_FindHereOrAbove
is not in a bottleneck execution path, therefore it doesn't hurt to
dynamically allocate the memory instead of using size-limited stack
memory.
In job.c, GCC 5 complains about the macro FILENO that it has type
unsigned int, which is then passed as the argument of dup2, which
expects a simple int. Maybe this workaround from 1995 is not necessary
anymore, or maybe it is but can be restricted to the few affected
platforms.
This leaves meta.c and the filemon files to be converted. I didn't do
them since they are not well covered by the unit tests.
Some types have changed from int to unsigned int, size_t or time_t.
The variable i in hash.c has been kept as int since it counts down to
-1, which generates efficient machine code, at least on x86_64.
The bug had been introduced with dir.c 1.155 on 2020-10-02 22:20:25. In
that commit, openDirectories was replaced with a combination of a list
with a hash table, for more efficient lookup by name.
Upon cleanup, OpenDirs_Done is called, which in turn called
Dir_ClearPath. Dir_ClearPath takes full ownership of the given list and
empties it. This was no problem before since afterwards the list was
empty and calling Lst_Free just frees the remaining list pointer.
With OpenDirs, this list was combined with a hash table, and the hash
table contains the list nodes, assuming that the OpenDirs functions have
full ownership of both the list and the hash table. This assumption was
generally correct, except for the one moment during cleanup where full
ownership of the list was passed to Dir_ClearPath, while the hash table
still contained pointers to the (now freed) list nodes. This by itself
was not a problem since the hash table would be freed afterwards. But
as part of Dir_ClearPath, OpenDirs_Remove was called, which looked up
the freed directory by name and now found the freed list node, trying to
free it again. Boom.
Fixed by replacing the call to Dir_ClearPath with code that only frees
the directories, without giving up control over the list.
The bug had been introduced with dir.c 1.155 on 2020-10-02 22:20:25. In
that commit, openDirectories was replaced with a combination of a list
with a hash table, for more efficient lookup by name.
Upon cleanup, OpenDirs_Done is called, which in turn called
Dir_ClearPath. Dir_ClearPath takes full ownership of the given list and
empties it. This was no problem before since afterwards the list was
empty and calling Lst_Free just frees the remaining list pointer.
With OpenDirs, this list was combined with a hash table, and the hash
table contains the list nodes, assuming that the OpenDirs functions have
full ownership of both the list and the hash table. This assumption was
generally correct, except for the one moment during cleanup where full
ownership of the list was passed to Dir_ClearPath, while the hash table
still contained pointers to the (now freed) list nodes. This by itself
was not a problem since the hash table would be freed afterwards. But
as part of Dir_ClearPath, OpenDirs_Remove was called, which looked up
the freed directory by name and now found the freed list node, trying to
free it again. Boom.
Fixed by replacing the call to Dir_ClearPath with code that only frees
the directories, without giving up control over the list.
In that compilation variant, TRUE is defined to 255, to see whether all
boolean expressions evaluate to either 1 or 0. The field If.doNot in
cond.c doesn't do this since it uses the actual value of TRUE.
Therefore, change the evaluation slightly to also handle this case.
* dhcpcd: Backticks have been removed from quoting filenames
* dhcpcd: Only manipulate stdin, stdout and stderr if they are valid
* duid: Adjust option so the type can be specified
* logerr: Don't leak logfile fd to scripts
* privsep: Run the launcher process in the sandbox
* BSD: Use `ifi_link_state` as the single source of truth about carrier
* BSD: Ignore vether(4) devices by default
Even with ``memcmp fix'', GCC 9.4 miscompiles this function for -O[12].
But the situation was slightly changed from that with GCC 8.3:
* -O0 and -O1 work but -O2 fails for 68060 and 68040 (real hardware)
* -O0 and -O2 work but -O1 fails for 68020 and 68010 (TME)
