For one-liners, Lst_ForEach creates a lot of overhead, both at runtime
and for reading and understanding the code.
In this simple case where the structure of the traversed nodes is not
modified, a simple loop is enough. This avoids a lot of conversions to
void * and thus prevents type mistakes.
This occurred in the posix1.mk test, even though it is disabled in
unit-tests. But in tests/usr.bin/make it still runs. There, it should
have produced an "expected failure" but crashed instead.
The archive-suffix test is the stripped-down version of the posix1 test.
In SuffParseTransform, the parameter names have been renamed to make the
"side effects" comment redundant.
In Suff_AddSuffix and Suff_AddLib, the parameter has been made const.
In SuffRemoveSrc, the unused variable has been removed, and the return
type has been fixed.
Lst_Find is called with a "comparison" function that returns the integer
0 if the desired node is found. This leads to confusion since there are
so many different return value conventions for int, such as 0/1 for
mimicking false/true, -1/0 as in close(2), and the sign as in strcmp(3).
This API is much easier to understand if the "comparison" function is
not called a comparison function (since that is too close to strcmp),
but a "match" function that just returns a boolean.
In Lst_FindFromB, the node argument may be null. This deviates from the
other Lst functions, which require Lst and LstNode to generally be
non-null. In this case it is useful though to make the calling code
simpler.
In arch.c, this makes a lot of the previous documentation redundant.
In cond.c, the documentation is reduced a little bit since it had
already been cleaned up before. It also removes the strange negation
from CondFindStrMatch.
In dir.c, the documentation collapses as well.
In main.c, separating the ReadMakefile function from the callbacks for
Lst_FindB allows the former to get back its natural function signature,
with proper types and no unused parameters.
To catch any accidental mistakes during the migration from Lst_Find to
Lst_FindB, the code can be compiled with -DUSE_DOUBLE_BOOLEAN, which
will complain about incompatible function pointer types.
This is quite hard to trigger in a real-life scenario since it requires
precise timing.
Instead, I created /tmp/sys.mk and ran "./make -m /tmp -f /dev/null" in
the debugger, after setting a breakpoint in this line:
ln = Lst_Find(sysMkPath, ReadMakefile, NULL);
Once this line was reached, I removed /tmp/sys.mk to make ReadMakefile
fail. Just adding a few parse errors won't help since ReadMakefile only
fails if the file cannot be found.
Having Boolean aliased to int creates ambiguities since int is widely
used. Allow to occasionally compile make with -DUSE_DOUBLE_BOOLEAN to
check that the type definitions still agree.
Var_Subst never returns NULL.
In Main_ExportMAKEFLAGS, don't compare ints with booleans.
In MainParseArgs, use char for the current character. First, that's
more precise and correct, and second, it makes debugging easier for
those who don't know the ASCII table by heart.
The old name implied that the function would read multiple files, which
was not the case.
The comment above that function was highly confusing. It's not that the
function returns a boolean, but rather 0 or non-zero, and 0 means that
Lst_Find should stop searching.
One of the next refactorings will be to make Lst_Find return the first
list node for which the function returns TRUE. This will reduce the
confusion about the several functions called SomethingP in suff.c. The
P suffix means to return TRUE or FALSE, not 0 or non-zero.
The functions LstIsValid and LstNodeIsValid are only used in assertions.
Without the always-false assertion, Enum_ValueToString could have
returned undefined behavior.
Remove redundant parts of the function comments. Move the "side
effects" to the main section, since these effects are main effects, not
side effects.
Remove the redundant prototype for ArchFree.
In the first version of this test, I had completely misunderstood the
whole topic.
To test the interrupt, the make process has to be interrupted, not the
shell. This generates the correct message that the target is removed.
The filename for .PHONY targets is removed even though .PHONY targets
usually don't correspond to a file. The message is only printed if
there actually is a corresponding file. That's why this message does
not appear when interrupting "make clean".
Finally, since files get created and removed during a single run of
make, the file cache needs to be disabled. This is done by prefixing
the filenames with "././", see Dir_FindFile.
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.
