Several parts of make intentionally depend on the guarantee that
snprintf and vsnprintf do not overflow their buffer. If an
implementation cannot provide this guarantee, refuse to use it.
Concatenating identifiers makes it difficult to spot them when searching
the code. This gets rid of the special case for OP_MEMBER and MEMBER.
The same pattern is applied in the DEBUG macro, by the way.
The previous output format had a %-20s conversion specifier. This
produced different output depending on the length of the pathname, which
was too difficult to normalize. By moving the directory name to the
end, it is no longer necessary to fill up any space, and the numbers are
always aligned properly.
As a result, 3 of the unit tests no longer need any special
postprocessing of their output.
This is only used interactively, not in the official builds, therefore
the additional dependency on Perl doesn't matter. The same result could
have been achieved in any other programming language, but probably not
as concisely.
Before, the conditions in the output had been expanded, which made them
illegible. The expanded conditions were unrealistic as well since the
evaluation flags differ between a condition and normal evaluation
(VARE_WANTRES, VARE_UNDEFERR).
The function name had been too ambiguous since it didn't mention the
particular directory that was initialized. Instead of that function,
Dir_InitCur is called directly from main_Init.
The pseudo CachedDir entry ".DOTLAST" is initialized at the very
beginning. The observable behavior is unchanged since this a
memory-only object with no connection to the file system.
The special path entry is called .DOTLAST, therefore the local variable
should have the same name.
A variable named 'base' must not point to the slash of a pathname. It
may only point to the character after the slash, everything else is
confusing, even if it's only for a brief moment.
Calling CachedDir_Assign requires that the variable be initialized. On
most systems, NULL is represented as all-zero bits already. This change
is only for the few other systems.
Add some comments explaining the implementation of Dir_AddDir since that
is tricky to read from the code alone.
Previously, the reference count for a newly created CacheDir had been
set to 1 in CacheNewDir. This was wrong because at that point, the
object had not been referenced by any nonlocal variable. The reference
count is no longer incremented at this point.
All callers of CacheNewDir either append the newly created CachedDir to
a SearchPath via Lst_Append and CachedDir_Ref, or they assign it to a
global variable via CachedDir_Assign.
Since the reference count is no longer wrongly incremented, it does not
need to be decremented more than necessary in Dir_End. To keep the code
simple and maintainable, all assignments to global variables are now
handled by CachedDir_Assign. Adding a CachedDir to a list is still done
manually via Lst_Append, and the corresponding code for decrementing is
in SearchPath_Clean and SearchPath_Free. These details may be cleaned
up in a follow-up commit.
As a result, when OpenDirs_Done is called in the unit tests, the list of
open directories is empty. It had been non-empty in a single unit test
before (dep-wildcards.mk), as a result of calling Dir_Expand.
The additional debug logging for the reference counting is not enabled
by default since it contains memory addresses, which makes the output
dependent on the memory allocator.
The function CachedDir_Destroy has been merged into CachedDir_Undef,
which had only been used in Dir_End before. The new name emphasizes
that it corresponds to CachedDir_Ref.
The memory management for dotLast is quite simple. It is initialized
exactly once main_Init > Init_Objdir > Dir_InitDir and freed exactly
once in main_CleanUp > Dir_End. Previously, dotLast was not freed at all.
The first call to CachedDir_Unref decremented the refCount to 0 but
didn't free anything. Next, CachedDir_Destroy was called, which
decremented the reference count to -1, therefore skipping the actual
freeing. This was probably an implementation mistake.
Since Dir_End is called at the very end of main_CleanUp, no code
accesses dotLast after it has been freed.
In a makefile with repeated ".CURDIR=." lines, Dir_AddDir is called with
a NULL path, once per line. Since the path is NULL, the search for
OpenDirs_Find is skipped and the directory is always read from disk.
The freshly read directory has a refCount of 1, and the refCount never
raises above 2.
In Dir_InitCur, the directory of the previous .CURDIR has a refCount of
2, which is decremented twice and then freed. After this, the new
directory is placed in the global 'cur', after incrementing its refCount
to 2.
It still seems wrong that the refCount of 'cur' is 2 instead of 1, but
it works well.
The function Dir_Destroy is an implementation detail of the cached
directories, and it should not be exported to the other modules. The
search paths, on the other hand, are the high-level API that may be used
by the other modules, as the concept of search paths is documented in
the manual page.
Previously, the documentation of that test was much too short to explain
all the effects that happened in the bug situation from 2020-06-28 until
2020-07-02.
When compiling make in all-in-one mode, these variable names conflict.
They could have been merged into a single variable, but that would have
required to make it a global variable for the other modules as well.
The parse module has a similar variable called 'fatals'. All these can
possibly be merged into a single variable, but not now.
When compiling make in all-in-one mode, the variable name nfds conflicts
with the local variable name nfds in meta_compat_parent.
The variable name jobfds was misleading. It has nothing to do with file
descriptors, it's just an array of jobs.
This test is just meant to cover the existing code, it still needs to be
cleaned up to serve as a tutorial and to highlight the really
interesting points.
This means no more unnecessary void pointers in function signatures and
no more abstraction level at checking a single element of a list. In
most cases it is more appropriate to define a function that operates on
the list as a whole, thereby hiding implementation details like the
ListNode from the caller.
Having an enum whose constants must be ordered in a certain way may be
unexpected to casual readers. Hide this implementation detail in
separate functions.
Fix one bug, find 4 new ones. All these bugs have been around since
2005-05-08, when dependencies on the .BEGIN, .END and .INTERRUPT nodes
were implemented. Before that, checking gn->made == ERROR was
appropriate, but adding the dependencies made ABORTED a new possible
error value from Compat_Make.
Given only the state names and their individual documentation, it is
hard to see the full picture. To make this easier, provide typical
examples of the ways that a GNode takes through these states.
The output from the directory cache made the regular NetBSD build fail
because the pathname to the working directory differs, thus affecting
the spacing.
This time, document why the directory cache needs to be excluded from
the output.
This avoids passing invisible void pointers around in Lst_Append. It
also provides a convenient place to document what it means to "add a
candidate to the searcher".
Having a simple list of candidates is not enough. It is currently
possible to construct endless loops with huge memory usage, as
demonstrated in suff-transform-endless.mk.
To fix this, a straight-forward idea is to remember which candidates
have already been searched and to not search them again.
This also fixes a small inconsistency in the code. Most parameters had
been named slst (the s came from a time when Candidate was named Src),
except for Suff_FindDeps, where the variable was named srcs. The
confusing thing about this was that the name srcs is used throughout the
file for a different purpose. Only in FindThem there were two
parameters of the same type, which made this even more confusing.
The suffix code still doesn't behave as expected. Make sure that at
least setting the main target works as expected. It does, and the added
debug logging provides further hints for debugging the suffix handling
code.
The configuration section of unit-tests/Makefile is already complicated
enough to read, due to the excessive use of regular expressions.
Therefore, to keep the structuring elements at a minimum, inline the
.for loop.
Previously, the variables section had been omitted. This was because
the variables had been output in hashcode order until 2020-10-18, and
because some of the variable values are specific to the test environment
or the individual run (MACHINE_ARCH, MAKE.PPID).
