To trigger the faulty code path, the file where the targets gets its
first command must be included via its relative path. That was the case
when running 'cd usr.bin/make && make test' but not when running the
tests via ATF.
This warning flags the second semicolon of 'return;;' as being
unreachable. It does not warn about these superfluous semicolons in
general though.
Seen in usr.bin/make/bmake_malloc.c.
In meta mode, the affected variable patterns do not contain a '$'.
Outside of meta mode, Global_SetExpand was only called a single time, so
inline that call.
No functional change.
When the file for the debug log cannot be opened, make exits
immediately. This doesn't give the test a chance to clean up the
temporary log file.
Instead of trying to treat a regular file as a directory and create a
file in it, assume that the directory /nonexistent-$uuid is actually
nonexistent. This leads to the same kind of error message, independent
of strerror(3).
This applies to all 4 situations in which the output of an external
command is used for modifying a variable or an expression:
* the assignment operator '!='
* the assignment modifier '::!='
* the SUN shell modifier ':sh'
* the other shell modifier ':!cmd!'
Previously, only the shell modifier ':!cmd!' had debug logging.
Suggested by Christoph Badura.
When run via 'cd usr.bin/make/unit-tests && make test', the tests are in
the current directory. When run via ATF, the tests are in
/usr/tests/usr.bin/make/unit-tests, while the current directory is a
temporary directory. Allow both variants, plus others that may occur in
the bmake distribution.
This change leaves only literal format strings in parse.c. It allows
for more detailed error messages than the current "non-zero status" or
"exited on a signal".
No functional change.
The assignment 'VAR != cmd' generates a warning, the others generate an
error message. That error message is ignored for backwards
compatibility though.
The assignment via the expression ${VAR::!=cmd} only uses the output of
the command if there was no error, the other places use the output
nevertheless.
In a .for loop that contains an unclosed .if directive,
Cond_restore_depth generates an error message. If stack traces are
enabled using the option '-dp', the details of the .for loop are added
to the stack trace, but at that point, the ForLoop had already been
freed. To reproduce:
make-2022.01.09.00.33.57 -r -f unit-tests/directive-for.mk -dp
The buffer of a .for loop is always either empty or ends with '\n'. A
variable name consists of arbitrary non-whitespace characters.
Therefore a variable name can never reach the end of the buffer.
No functional change.
The previous version of the code gave the wrong impression that For_Eval
would modify CurFile. That happens only later, in For_Run.
No functional change.
Previously, multi-line directives like '.info' or '.error' reported the
line number of their last line instead of their first line, which is
more usual. This also affected the debug log from '-dp'.
Put related decisions on the same indentation level, remove unnecessary
negation, keep the code for the '.for' directive together.
No functional change.
The input buffer is guaranteed to be terminated by '\n'. This means
that after a '\\', there is no need to check for the end of that buffer.
While here, condense ReadLowLevelLine.
No functional change.
The previous variable name suggested that the variable would point to
the first '#' character of the line, instead it points to the whitespace
before the first '#'.
No functional change.
The comment in ApplyDependencySourceOther repeated the code, its second
half didn't match any current code.
The comment above ParseDependencySourcesEmpty repeated the code.
No binary change, except for assertion line numbers.
Sort ForLoop members in natural reading order.
Remove redundant condition in ForLoop_ParseItems; at that point, the
number of variables is non-zero.
Rename Buf_AddEscaped since that function is not part of the Buffer API,
it is specific to .for loops.
No functional change.
Since the body of a .for loop is scanned from start to end, there is no
need to remember the length of a variable name.
Using memcmp for comparing the variable name was probably overkill since
the variable names are usually very short, so rather compare them byte
by byte.
No functional change.
For lines that use backslash continuation, the human-readable line
number does not equal the number of raw lines that have been read from
the file.
The big comment in PrintStackTrace has become outdated, it still
referred to first_lineno. Due to the bugs documented in
opt-debug-parse.mk, that function needs to be redone completely.
No functional change.
This enables GCC 11 to inline ApplyModifier_Time, like all the other
modifiers. Similar pattern as for ':M' and ':N', as well as for ':D'
and ':U'.
No functional change.
Use consistent wording (zulu -> gmt), make VarStrftime parameter order
consistent with strftime, rename confusing 'time_t utc' to 't',
eliminate common subexpression in error message.
No functional change.
It is not necessary anymore to modify the passed-in line. It had been
necessary when the parsing function was several hundred lines long, to
avoid gotos.
No functional change.
Having only a single moving pointer is less confusing than the previous
copying between tgt and cp. For example, it did not make sense that the
target would start with '!'.
No functional change.
In PrintLocation, fname is not an iterator, so prefer fname[0] over
*fname.
List stdout and stderr in this order, for consistency with main.c.
No functional change.
Calling malloc(0) may return a null pointer, but callers of bmake_malloc
do not expect that.
Reported by Chris Pinnock, found by cross-compiling NetBSD on OpenBSD,
where tools/groff creates Makefile.dep files of size 0.
In ParseWord, the expressions '*p' and 'ch' are the same.
In ParseDependencyTargetWord, clean up a wordy comment and join two
conditions.
In the test cond-token-number, clarify that make doesn't convert from
hex to decimal but only from hex to internal representation.
No functional change.
Remove redundant comments.
Rename IFile.first_lineno to forBodyLineno since it only contains a
useful value in .for loops but not in .include files. Also clarify that
this line number is the start of the loop body, since in PrintStackTrace
this line is used as a human-readable line number. For a .for loop in
which the loop head spans multiple lines, this line number is wrong
anyway.
No functional change.
This is a preparation for cleaning up the code for loading and parsing
files, especially the part for including other files and for .for loops.
No functional change.
Do not reserve extra space "just in case a makefile does not end in
'\n'" since that doesn't happen often.
The assertion for 'buf.len <= buf.cap' was redundant.
No functional change.
GCC generates more efficient code; previously it wasn't aware that (end
- start) was always positive, thus allowing to omit the code for
dividing a negative number by 2.
No functional change.
It's a rather theoretical case that 'name' would point at the very end
of the address space and the string there would be "V=", but in that
case, the expression 'name + 3' would wrap around.
Combining two similar but fundamentally different parsing tasks in a
single function only increased the complexity, of the implementation as
well as the call sites.
The code makes it obvious now that a function argument is a bare word
surrounded by parentheses.
The special case of an empty word is only needed for the function
argument, it cannot occur in a bare word. The code for that has been
moved to the caller. Such an empty word not only occurs for 'defined()'
but also for 'defined(${:U})'.
No functional change.
Merge the two return values (bool, string) into a single return value.
As before, the caller cannot observe the difference between a parse
error and an empty word, both are handled in the same way.
In CondParser_ComparisonOrLeaf, the word cannot be empty since the
calling function CondParser_Token already handles all cases that could
lead to an empty word.
No functional change.
The result of irrelevant leaves is effectively ignored by CondParser_And
and CondParser_Or. Use the 'doEval &&' pattern to make the code
consistent with CondParser_Comparison and CondParser_FuncCall.
No functional change.
When a condition contains an irrelevant function call, it doesn't matter
whether the function call evaluates to true or to false, it will be
discarded anyway by either CondParser_And or CondParser_Or.
Returning false instead of true makes the code simpler, plus it is more
common to return false for irrelevant results.
No functional change.
The code for looking up the function from the table forced the compiler
to use a specific memory layout. Replacing the table with explicit code
provides the compiler more opportunities to optimize the code. Another
side effect is that there are fewer pointer operations.
Previously, is_token checked that the character after the word does not
continue the word, this is now done separately since for the function
lookup, this check was unnecessary. The newly added skip_string
provides a higher abstraction level, it is no longer necessary to pass
the string length as a separate, redundant parameter.
No functional change.
A colon at the end of a line requires at least 1 follow-up line, but
xlint cannot know whether lint2 will find anything to complain about.
Having a colon followed by nothing creates unnecessary confusion.
When make is run without the '-f' option, it searches for the files
'makefile' and 'Makefile' in the current directory. The function
ReadFirstDefaultMakefile allocated memory for these filenames, added the
filenames to opts.makefiles and then freed the memory. From that
moment, opts.makefiles contained dangling pointers.
The function main_CleanUp cleans the list, but only if make is compiled
with -DCLEANUP. Since main.c 1.557 from 2021.12.27.23.11.55, the
strings in opts.makefiles are freed as well, before that, only the list
nodes were freed. Freeing the strings led to the double-free.
Fix this bug by using a separate list for these short-lived strings. At
the point where ReadFirstDefaultMakefile is called, opts.makefiles is
not used anymore, therefore there are no side effects.
To reproduce, run 'make test-coverage', which compiles with -DCLEANUP.
The test opt-chdir failed with a segmentation fault in main_Cleanup.
This test may be the only one that doesn't use the option '-f'.
Before 2020, there had been a huge function for parsing a dependency
line, with lots of local variables that were reused for different
purposes. When that function was split up into smaller functions, that
was done mechanically, without eliminating redundant variables.
No functional change.
The list is only used when a single target name is parsed, in case the
name contains wildcards. There is no need to keep it any longer or
reuse it.
Clean up outdated and redundant comments.
No functional change.
The prefix 'Parse' was ambiguous since it was both the module name and a
verb. Rename those functions that don't actually parse anything.
No functional change.
The interesting part of the .ORDER constraint is what is made before
what, so reveal this information in the debug log.
The debug output from the test looks a bit strange since it forces
'three' to be made before 'one', but that's because the test exercises
the edge case of introducing a circular dependency.
The previous log format "ParseReadLine (%d): '%s'" focused on the
implementation, it was not immediately obvious to a casual reader that
the number in parentheses was the line number. Additionally, having
both a colon and quotes in a log message is uncommon. The quotes have
been added in parse.c 1.127 from 2007-01-01.
The new log format "Parsing line %d: %s" is meant to be easier readable
by humans. The quotes are not needed since ParseReadLine always strips
trailing whitespace, leaving no room for ambiguities. The other log
messages follow common punctuation rules, which makes the beginning of
the line equally unambiguous. Before var.c 1.911 from 2021-04-05,
variable assignments were logged with the format "%s:%s = %s", without a
space after the colon.
Rename 'spec' to 'special', for consistency with the previous commits.
Rename 'tOp' to 'targetAttr' since it is not an dependency operator like
':', it's an attribute like '.SILENT'.
No binary change, except for the line number of the assertion in line
1618.
A .USE target is not a candidate, so .USEBEFORE shouldn't either.
Since make.h 1.36 from 2001-07-03. In that commit, OP_USEBEFORE should
have been added to OP_NOTARGET.
The variable name 'end' suggested pointing to the end of the string, but
instead it pointed to the last possible starting position of the word to
be searched. Remove this possible misunderstanding.
No functional change.
Renaming savederr to saved_errno makes the comment redundant.
Group the conditions for setting errfmt, retaining their relative order.
No functional change.
This is a similar pattern as in the other situations where a string is
fed through Var_Subst. In this case though, the unexpanded string may
need to be freed, therefore the FStr_Done that is not needed in the
other places.
No functional change.
Even though the name of the debug log file currently only occurs in
strings of the form '-dFname' or '-dF+name', the code for replacing '%d'
with the PID accesses the passed string out of bounds. That's not a
problem in practice but looks suspicious anyway.
Since a non-writable file is not a syntax error, there is no point in
showing the usage in this situation. Showing the usage may have been a
copy-and-paste mistake from a few lines below, when this option was
added back in main.c 1.133 from 2006-10-15.
Breaking out of the first 'for' loop was unnecessarily complicated. The
call to strlen was not necessary since f already pointed at the end of
the string.
No functional change.
Several years ago, the command line options were individual global
variables. The global variable could therefore not be named 'silent'
since that would have conflicted with local variables of the same name.
After moving the global variable to the namespace 'struct CmdOpts',
there is no conflict anymore.
There doesn't seem to be any risk of naming collisions for the names
'touch' and 'query'.
No functional change.
The name eunlink suggested a relation with the similarly named functions
emalloc or esnprintf, but that was misleading. Instead, unlink_file
works like unlink, except that it refuses to remove an empty directory.
No functional change.
For the result of the comma operator, it doesn't matter whether the
comma itself comes from a system header or not. Instead, it's the main
operator of the right operand.
Since 2021-11-16.
Rename 'struct kwtab' to 'struct keyword' since a single keyword is not
a whole keyword table.
Sync comment for lex_name with reality: sbuf_t no longer contains the
hash value.
Remove redundant tests for EOF, as EOF is neither a space nor a digit
nor an xdigit.
No functional change.
The implementation from March 2021 added proper support for designators
but didn't model the brace levels correctly. In particular, it could
not handle additional braces or omitted braces. In such a case, lint
skipped the remaining initializers from the initialization. Due to
this, type errors in the remaining initializers went unnoticed. Another
effect was that arrays of unknown size were wrongly reported as having
size 0.
Both GCC and Clang recommend placing braces around each sub-type that is
initialized, such as a struct, union or array. Postfix does not follow
these recommendations, therefore lint had to be disabled in
external/ibm-public/postfix/Makefile.inc. This commit fixes the bugs
mentioned there.
The newly added tests triggered the assertion in begin_designation since
for incomplete types the initialization is stopped before handling the
first brace.
The previous name could be mistaken to mean "increase the indentation of
the debug output". Instead, the function prints the current indentation.
In externs1.h, the macro definition was a duplicate, the macros were
sorted differently than the functions a few lines above.
No binary change.
This will be necessary to properly implement handling of initializers
and braced initializer-lists.
No functional change for now since the designation is already reset
after each expression and '}'. To handle initializations properly, the
designation must not be reset after each expression, it must advance to
the next member instead.
There is no need to store this information at every brace level since in
any translation unit that survives a conforming C99 compiler, an array
of unknown size is only possible once per initialization, not once per
brace level.
This change is a prerequisite for fixing the current bugs in handling
initializations. Part of the fix will be designation_pop, which is
costly for a singly linked list.
As a side benefit, memory management becomes simpler and needs fewer
malloc calls.
No functional change.
Having the code in separate functions did not add to the clarity of the
code. The additional information from the function names can be grasped
as easily from the case labels.
No functional change.
The prototype declarations define the correct parameter types of these
functions so that they are no longer subject to the default argument
promotions (C11 6.5.2.2p6).
The GCC builtins are only recognized in GCC mode (-g).
Passing an arbitrary tokenizer symbol left too much freedom and
uncertainty to the caller, and 0 was a magic number in this context.
No functional change.
rillig