* Replace character literal 0 with '\0'.
* Replace pointer literal 0 with NULL.
* Remove redundant parentheses.
* Parentheses in multi-line conditions are not redundant at the
beginning of a line.
* Replace a few !ptr with ptr == NULL.
* Replace a few ptr with ptr != NULL.
* Replace (expr & mask) == 0 with !(expr & mask).
* Remove redundant braces for blocks in cases where the generated code
stays the same. (Assertions further down in the code would get
different line numbers.)
* Rename parameters in CondParser_String to reflect the data flow.
* Replace #ifdef notdef with #if 0.
The generated code stays exactly the same, at least with GCC 5.5.0 on
NetBSD 8.0 amd64 using the default configuration.
Robust programs don't have extern variable declarations in .c files, as
that risks incomatible definitions that are not detected by the compiler
and invoke undefined behavior. Make make a little more robust.
Since there is only a single variable left that needs to be freed at the
end (and probably never actually needs to be freed since nobody defines
an environment variable named .OBJDIR), there is no need to loop over
these variables.
Nobody defines a global variable named .TARGET since that would have
many unpredictable effects, applying to all targets at once.
Nobody defines an environment variable named .TARGET since that's
against the naming conventions for environment variables and would have
the same effect.
Because of this, there is no point looking up the variables that are
local to a GNode anywhere else. This means they cannot come from the
environment and thus their value doesn't need to be freed after use,
which makes the code simpler.
The newly added accessor functions in make.h refer to external
functions, but since that header is not used anywhere outside of
usr.bin/make, it doesn't matter. Between 2020-08-25 and 2020-10-30,
that header had been referenced by usr.bin/xinstall.
The only purpose of the parameter freeIt is to free the memory
associated with the return value. To do this, no pointer arithmetic is
needed. Therefore, change to a void pointer, to catch accidental use of
that pointer.
Using Lst_Find and Lst_FindFrom to implement Lst_RemoveIf was a bad
idea. It made the code much more complicated than necessary. There is
no predefined Lst_RemoveIf, but that can be implemented easily. By
inlining the list handling, path_match does not need void pointers
anymore.
Freeing the path from the missingFiles list had been implemented in a
surprisingly complicated way, intermangling it unnecessarily with the
list operations, even though these are completely independent.
The function call variant takes more screen space than the direct field
access. Having an abstract API is usually a good idea, in this case of
simple read-only member access it makes the code more difficult to read.
LstNode_Set has been kept as a function since it is not a read-only
accessor function.
The FD_* macros from sys/sys/fd_set.h use signed integers on NetBSD 8
and thus produce conversion errors. On NetBSD 9, these macros are fixed
to use 1U instead of 1.
These functions made the code larger than necessary. The prev and next
fields are published intentionally since navigating in a doubly-linked
list is simple to do and there is no need to wrap this in a layer of
function calls, not even syntactically. (On the execution level, the
function calls had been inlined anyway.)
Since the callback function returns a terminating condition, this is not
really a foreach loop.
Many of the calls to Lst_ForEachUntil don't make use of the terminating
condition, and several don't modify the list structurally, which means
they don't need this complicated implementation.
In a follow-up commit, Lst_ForEach will be added back with a much
simpler implementation that iterates over the list naively, without a
terminating condition and without taking the iteration state from
Lst_Open/Lst_Next/Lst_Close into account. The migration to this simpler
implementation will be done step by step since each callback function
needs to be examined closely.
These typedefs are only intended to help human readers, they do not
provide any type-safety. They also make the pointers explicit, which
had been hidden before by the typedef for Lst and LstNode. Typing a few
'*' is less work than finding out which of the many types are pointers
and which aren't.
In meta.c, the variable "ln" served two completely different purposes,
which have been split again. Register allocation is the job of the
compiler, not of the human source code reader.
Only st_mtime and st_mode are actually filled, the remaining fields had
been set to zero. To prevent these from ever being accessed, a custom
struct make_stat replaces the previously used struct stat.
The fields in struct make_stat are intentionally named different from
the fields in struct stat because NetBSD and some other operating
systems define st_mtime as a macro, and that would not work in a field
declaration.
When I migrated the Lst_FindFrom to the strict API variant, I forgot
that Lst_FindFrom requires both arguments (list and node) to be
non-null. I had only checked that the list is non-null.
There are only very few calls to Lst_FindFrom, and they are all ok now.
The migration from null-passing Lst functions to argument-checking Lst
functions is completed.
There were 2 surprises: The targets list may be NULL, and in Dir_AddDir,
the path may be NULL. The latter case is especially surprising since
that function turns into an almost-nop in that case. This is another
case where probably 2 independent functions have been squeezed into a
single function. This may be improved in a follow-up commit.
All other lists were fine. They were always defined and thus didn't
need much work.
In most cases the Lst functions are only called when the arguments are
indeed valid. It's not guaranteed though, therefore each function call
needs to be analyzed and converted individually.
While here, remove a few statements that were only useful when the Lst
functions handled circular lists.
The general-purpose list library that is included in make allows to call
Lst_AtEnd for invalid lists, silently ignoring this programming error.
This is a flexibility that make doesn't need.
Another unneeded "feature" is that list items can theoretically be null
pointers. This doesn't make sense as well and is therefore not needed
by make.
These programming errors are now caught early by assertions.
Up to now, the list library didn't distinguish between programming
mistakes (violations of invariants, illegal parameter values) and
actually interesting situations like "element not found in list".
The current code contains many branches for conditions that are neither
exercised by the unit tests nor by real-world usage. There is no point
in keeping this unnecessary code.
The list functions will be migrated from their lenient variants to the
stricter variants in small parts, each function getting the S suffix
when it is made strict, to avoid any confusion about how strict a
particular function is. When all functions have been migrated, they
will be renamed back to their original names.
While here, the comments of the functions are cleaned up since they
mention irrelevant implementation details in the API comments, as well
as "side effects" that are really main effects.
The list library had probably been imported from a general-purpose
library that also supported circular lists. These are not used by make
though.
After replacing Lst_Init(FALSE) with Lst_Init(), only a single call to
Lst_Init remained with a non-constant argument, and that was in
Lst_Concat, which was to be expected.
NetBSD make is intended to be maximally portable, therefore it uses only
C89. This was not declared in the Makefile before.
There are still a few places in parse.c and metachar.c that use
end-of-line comments. These will be fixed in a follow-up commit.
rillig