Previously, each time a .for directive pushed its buffer on the input
file stack, the current filename was duplicated. This was a waste of
memory.
The name of a file is typically only used while it is read in. There is
one situation when the filename is needed for longer, which is when a
target is defined.
Since .for loops are implemented as a special form of included files,
each .for loop duplicated the current filename as well.
$ cat << EOF > for.mk
.for i in 1 2 3 4 5 6 7 8 9 0
.for i in 1 2 3 4 5 6 7 8 9 0
.for i in 1 2 3 4 5 6 7 8 9 0
.for i in 1 2 3 4 5 6 7 8 9 0
.for i in 1 2 3 4 5 6 7 8 9 0
.for i in 1 2 3 4 5 6 7 8 9 0
.for i in 1 2 3 4 5 6 7 8 9 0
.endfor
.endfor
.endfor
.endfor
.endfor
.endfor
.endfor
all:
@ps -o rsz -p ${.MAKE.PID}
EOF
$ make-2021.12.13.03.55.16 -r -f for.mk
RSZ
10720
$ ./make -r -f for.mk
RSZ
1716
The difference is 8 MB, which amounts to 1 million .for loops.
In the past few months I had accidentally used C99 features in the make
code. According to tools/README, tools that are used in the build
system should restrict themselves to C90.
This allows make to build with GCC's options "-pedantic
-Wno-system-headers -Dinline= -Wno-error=cast-qual".
I didn't notice anyone actively complaining though, I just wanted to see
how much work this backporting would be. The identifier __func__ is
still used, as in other tools.
No functional change.
These markers had been used inconsistently. Furthermore the source code
had not been formatted automatically before 2020 at all, otherwise there
wouldn't have been any trailing whitespace left.
The previous version of lint(1) from a few hours ago didn't catch all
occurrences. And even the current one doesn't catch everything.
Function arguments and return types still need some work. The "return
quietly" from shouldDieQuietly still implicitly converts from int to
_Bool.
No functional change.
Most of the make code already followed the style of explicitly writing
(ptr != NULL) instead of the shorter (ptr) in conditions.
The remaining 50 instances have been found by an experimental,
unpublished check in lint(1) that treats bool expressions as
incompatible to any other scalar type, just as in Java, C#, Pascal and
several other languages.
The only unsafe operation on Boolean that is left over is (flags &
FLAG), for an enum implementing a bit set. If Boolean is an ordinary
integer type (the default), some high bits may get lost. But if Boolean
is the same as _Bool (by compiling with -DUSE_C99_BOOLEAN), C99 6.3.1.2
defines that a conversion from any scalar to the type _Bool acts as a
comparison to 0, which cannot lose any bits.
The main changes are in the comments, which have been shortened and
corrected.
Some local variables changed their names.
In ParseErrorInternal, the scope of va_start is now narrower.
In ParseDoDependency, the type of tOp has been fixed.
ParseGetLine doesn't take flags anymore but instead a parsing mode.
Previously, the flags had not been combined anyway.
At the beginning of Parse_File, fatals is already guaranteed to be 0, and
even if not, it would be wrong to just discard the fatal errors.
* 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.
These macros typically evaluate one of their arguments twice. Until
2020-08-31, they had not parenthesized their arguments properly. They
are only used in a few places, therefore it doesn't hurt much to have
them expanded.
This mainly affects the void pointers in callback functions for lists.
These had been necessary once when the parameter type was still
ClientData instead of void pointer.
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.
These blocks mostly consisted of redundant structure, following the same
#ifndef pattern over and over, with only minimal variation.
It's easier to maintain if the common structure is only written once and
encapsulated in a macro.
To avoid "defined but unused" warnings from GCC in the case where
MAKE_NATIVE is not defined, I had to add volatile. Adding
MAKE_ATTR_UNUSED alone would not preserve the rcsid variable in the
resulting binary.
The API is much simpler, and there is less detail that is exposed by
default and fewer punctuation to type on the caller's side. To see that
there is some memory to be freed, one would have to look into the
struct. Having part of the return value as the actual return value and
the rest in output parameters was unnecessarily asymmetrical.
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.
The new functions have a simpler interface, and str_concat3 is even more
general-purpose, since the middle string is no longer required to be
exactly of length 1.
The previous documentation mentioned Str_Concat, but str_concat has been
written in lowercase for years. The "flags" are not flags since they
cannot be combined, not even when they are written in hex.
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.
The variables are renamed to reflect to which memory region each pointer
belongs.
The variable "curlen" was always zero.
The type of "ch" has changed to char, and its scope is now limited to
its actual use.
Comparisons of pointers are now consistently written as p != NULL
instead of !p, like character comparisons are written as ch != '\0'.
The "store_words_buf" is updated when the function returns, not before.
rillig