millert@ already saw all of this.
This solves a few minor/major problems.
- Theo wants no blank line at end of ^C error messages. This blank line
actually comes from the shell/tty, so we have to refrain printing out the
last \n in that situation.
- make sure errors go thru handle_all_signals(), so that signals are handled
properly.
1/- We have outgrown Buf_KillTrailingSpaces. More importantly, this fixes some
posix compliance bug, since spaces at end of variable assignments are NOT
supposed to get stripped.
So far, one non-compliant Makefile got fixed in xenocara. src seems very happy
with this. Basically, this means that for POSIX;
A = some value # and some comment
Then A will be set to exactly 'some value '.
(spaces around the = sign get stripped, but spaces before a comment DON'T)
- compat mode should expand .USE right away. It can never be an aliased target.
And accordingly, this turns into a straight switch() right away
2/- fix the cond parser to actually LOOK for its "special keywords" instead
of blindly ignoring other stuff (this is the cause of .if existS fix in cc,
btw). Use isspace more consistently while there...
- make parse/cond error messages look MORE like the new regular error message.
This can be tweaked some more if necessary.
- zap an extra cond error message that's not needed since the .if/.endif parser
is verbose enough.
- remove some MORE verbosity that's not needed.
- unclosed var specs in varmodifiers should be treated the same as the common
one.
(there are some other Errors in varmodifiers that may or may not need to
be turned into errors, but this can wait).
Out of this, I'd like feedback on the improved messages, and most importantly
TESTS about the more bitchy cond parser and posix compliant general parser.
Note that this can easily be cut into a few easy to read, independent patches.
But I'm mostly interested in ppl running all of that and reporting problems.
If any problems, I expect they will come from 1/ and 2/ mostly.
Index: buf.c
===================================================================
RCS file: /home/openbsd/cvs/src/usr.bin/make/buf.c,v
retrieving revision 1.24
diff -u -p -r1.24 buf.c
--- buf.c 21 Sep 2012 07:55:20 -0000 1.24
+++ buf.c 13 Oct 2012 14:22:50 -0000
@@ -161,13 +161,3 @@ Buf_Init(Buffer bp, size_t size)
bp->inPtr = bp->endPtr = bp->buffer = emalloc(size);
bp->endPtr += size;
}
-
-void
-Buf_KillTrailingSpaces(Buffer bp)
-{
- while (bp->inPtr > bp->buffer + 1 && isspace(bp->inPtr[-1])) {
- if (bp->inPtr[-2] == '\\')
- break;
- bp->inPtr--;
- }
-}
Index: buf.h
===================================================================
RCS file: /home/openbsd/cvs/src/usr.bin/make/buf.h,v
retrieving revision 1.21
diff -u -p -r1.21 buf.h
--- buf.h 2 Oct 2012 10:29:30 -0000 1.21
+++ buf.h 13 Oct 2012 14:22:55 -0000
@@ -131,10 +131,6 @@ do { \
* Adds characters between s and e to buffer. */
#define Buf_Addi(b, s, e) Buf_AddChars((b), (e) - (s), (s))
-/* Buf_KillTrailingSpaces(buf);
- * Removes non-backslashed spaces at the end of a buffer. */
-extern void Buf_KillTrailingSpaces(Buffer);
-
extern void Buf_printf(Buffer, const char *, ...);
#define Buf_puts(b, s) Buf_AddChars((b), strlen(s), (s))
Index: compat.c
===================================================================
RCS file: /home/openbsd/cvs/src/usr.bin/make/compat.c,v
retrieving revision 1.77
diff -u -p -r1.77 compat.c
--- compat.c 21 Sep 2012 07:55:20 -0000 1.77
+++ compat.c 14 Oct 2012 15:15:21 -0000
@@ -86,6 +86,12 @@ CompatMake(void *gnp, /* The node to mak
*/
if (gn == pgn)
return;
+ /* handle .USE right away */
+ if (gn->type & OP_USE) {
+ Make_HandleUse(gn, pgn);
+ return;
+ }
+
look_harder_for_target(gn);
if (pgn != NULL && is_sibling(gn, pgn))
@@ -103,9 +109,8 @@ CompatMake(void *gnp, /* The node to mak
} while (sib != gn);
}
- if (gn->type & OP_USE) {
- Make_HandleUse(gn, pgn);
- } else if (gn->built_status == UNKNOWN) {
+ switch(gn->built_status) {
+ case UNKNOWN:
/* First mark ourselves to be made, then apply whatever
* transformations the suffix module thinks are necessary.
* Once that's done, we can descend and make all our children.
@@ -232,30 +237,29 @@ CompatMake(void *gnp, /* The node to mak
print_errors();
exit(1);
}
- } else if (gn->built_status == ERROR)
+ break;
+ case ERROR:
/* Already had an error when making this beastie. Tell the
* parent to abort. */
pgn->must_make = false;
- else {
- switch (gn->built_status) {
- case BEINGMADE:
- Error("Graph cycles through %s", gn->name);
- gn->built_status = ERROR;
- pgn->must_make = false;
- break;
- case MADE:
- if ((gn->type & OP_EXEC) == 0) {
- pgn->childMade = true;
- Make_TimeStamp(pgn, gn);
- }
- break;
- case UPTODATE:
- if ((gn->type & OP_EXEC) == 0)
- Make_TimeStamp(pgn, gn);
- break;
- default:
- break;
+ break;
+ case BEINGMADE:
+ Error("Graph cycles through %s", gn->name);
+ gn->built_status = ERROR;
+ pgn->must_make = false;
+ break;
+ case MADE:
+ if ((gn->type & OP_EXEC) == 0) {
+ pgn->childMade = true;
+ Make_TimeStamp(pgn, gn);
}
+ break;
+ case UPTODATE:
+ if ((gn->type & OP_EXEC) == 0)
+ Make_TimeStamp(pgn, gn);
+ break;
+ default:
+ break;
}
}
Index: cond.c
===================================================================
RCS file: /home/openbsd/cvs/src/usr.bin/make/cond.c,v
retrieving revision 1.46
diff -u -p -r1.46 cond.c
--- cond.c 11 Oct 2012 14:56:17 -0000 1.46
+++ cond.c 14 Oct 2012 16:47:17 -0000
@@ -198,40 +198,42 @@ CondGetArg(const char **linePtr, struct
const char *cp;
cp = *linePtr;
+ /* Set things up to return faster in case of problem */
+ arg->s = cp;
+ arg->e = cp;
+ arg->tofree = false;
+
+ /* make and defined are not really keywords, so if CondGetArg doesn't
+ * work...
+ */
if (parens) {
- while (*cp != '(' && *cp != '\0')
+ while (isspace(*cp))
cp++;
if (*cp == '(')
cp++;
+ else
+ return false;
}
- if (*cp == '\0') {
- /* No arguments whatsoever. Because 'make' and 'defined' aren't
- * really "reserved words", we don't print a message. I think
- * this is better than hitting the user with a warning message
- * every time s/he uses the word 'make' or 'defined' at the
- * beginning of a symbol... */
- arg->s = cp;
- arg->e = cp;
- arg->tofree = false;
+ if (*cp == '\0')
return false;
- }
- while (*cp == ' ' || *cp == '\t')
+ while (isspace(*cp))
cp++;
-
cp = VarName_Get(cp, arg, NULL, true, find_cond);
- while (*cp == ' ' || *cp == '\t')
- cp++;
- if (parens && *cp != ')') {
- Parse_Error(PARSE_WARNING,
- "Missing closing parenthesis for %s()", func);
- return false;
- } else if (parens)
- /* Advance pointer past close parenthesis. */
+ while (isspace(*cp))
cp++;
+ if (parens) {
+ if (*cp == ')')
+ cp++;
+ else {
+ Parse_Error(PARSE_WARNING,
+ "Missing closing parenthesis for %s()", func);
+ return false;
+ }
+ }
*linePtr = cp;
return true;
@@ -747,7 +749,7 @@ CondToken(bool doEval)
return t;
}
- while (*condExpr == ' ' || *condExpr == '\t')
+ while (isspace(*condExpr))
condExpr++;
switch (*condExpr) {
case '(':
@@ -1164,8 +1166,9 @@ Cond_End(void)
condTop == 0 ? "at least ": "", MAXIF-condTop,
MAXIF-condTop == 1 ? "" : "s");
for (i = MAXIF-1; i >= condTop; i--) {
- fprintf(stderr, "\t at line %lu of %s\n",
- condStack[i].origin.lineno,
condStack[i].origin.fname);
+ fprintf(stderr, "\t(%s:%lu)\n",
+ condStack[i].origin.fname,
+ condStack[i].origin.lineno);
}
}
condTop = MAXIF;
Index: error.c
===================================================================
RCS file: /home/openbsd/cvs/src/usr.bin/make/error.c,v
retrieving revision 1.23
diff -u -p -r1.23 error.c
--- error.c 2 Oct 2012 10:29:30 -0000 1.23
+++ error.c 13 Oct 2012 23:43:13 -0000
@@ -149,13 +149,19 @@ static void
ParseVErrorInternal(const Location *origin, int type, const char *fmt,
va_list ap)
{
- if (origin->fname)
- (void)fprintf(stderr, "\"%s\", line %lu: ", origin->fname,
origin->lineno);
- if (type == PARSE_WARNING)
- (void)fprintf(stderr, "warning: ");
- (void)vfprintf(stderr, fmt, ap);
+ static bool first = true;
+ fprintf(stderr, "*** %s",
+ type == PARSE_WARNING ? "Warning" : "Parse error");
+ if (first) {
+ fprintf(stderr, " in %s: ", Var_Value(".CURDIR"));
+ first = false;
+ } else
+ fprintf(stderr, ": ");
+ vfprintf(stderr, fmt, ap);
va_end(ap);
- (void)fprintf(stderr, "\n");
+ if (origin->fname)
+ fprintf(stderr, " (%s:%lu)", origin->fname, origin->lineno);
+ fprintf(stderr, "\n");
if (type == PARSE_FATAL)
fatal_errors ++;
}
Index: job.c
===================================================================
RCS file: /home/openbsd/cvs/src/usr.bin/make/job.c,v
retrieving revision 1.130
diff -u -p -r1.130 job.c
--- job.c 9 Oct 2012 19:46:33 -0000 1.130
+++ job.c 14 Oct 2012 15:23:15 -0000
@@ -160,6 +160,9 @@ static void kill_with_sudo_maybe(pid_t,
static void debug_kill_printf(const char *, ...);
static void debug_vprintf(const char *, va_list);
static void may_remove_target(Job *);
+static const char *really_kill(Job *, int);
+static void print_error(Job *, bool);
+static void internal_print_errors(bool);
static void
kill_with_sudo_maybe(pid_t pid, int signo, const char *p)
@@ -226,7 +229,7 @@ may_remove_target(Job *j)
}
static void
-print_error(Job *j)
+print_error(Job *j, bool last)
{
static bool first = true;
@@ -254,13 +257,13 @@ print_error(Job *j)
if ((j->flags & (JOB_SILENT | JOB_IS_EXPENSIVE)) == JOB_SILENT)
fprintf(stderr, ": %.120s%s", j->cmd,
strlen(j->cmd) > 120 ? "..." : "");
- fprintf(stderr, ")\n");
+ fprintf(stderr, last ? ")" : ")\n");
free(j->cmd);
may_remove_target(j);
}
-void
-print_errors(void)
+static void
+internal_print_errors(bool die_from_signal)
{
Job *j, *k, *jnext;
@@ -273,7 +276,8 @@ print_errors(void)
for (j = k; j != NULL; j = jnext) {
jnext = j->next;
if (j->location->fname == k->location->fname)
- print_error(j);
+ print_error(j,
+ die_from_signal && jnext == NULL);
else {
j->next = errorJobs;
errorJobs = j;
@@ -282,6 +286,13 @@ print_errors(void)
}
}
+void
+print_errors(void)
+{
+ handle_all_signals();
+ internal_print_errors(false);
+}
+
static void
setup_signal(int sig)
{
@@ -842,7 +853,7 @@ handle_fatal_signal(int signo)
}
}
loop_handle_running_jobs();
- print_errors();
+ internal_print_errors(true);
/* die by that signal */
sigprocmask(SIG_BLOCK, &sigset, NULL);
@@ -850,6 +861,8 @@ handle_fatal_signal(int signo)
kill(getpid(), signo);
sigprocmask(SIG_SETMASK, &emptyset, NULL);
/*NOTREACHED*/
+ fprintf(stderr, "This should never happen\n");
+ exit(1);
}
/*
Index: lowparse.c
===================================================================
RCS file: /home/openbsd/cvs/src/usr.bin/make/lowparse.c,v
retrieving revision 1.30
diff -u -p -r1.30 lowparse.c
--- lowparse.c 2 Oct 2012 10:29:31 -0000 1.30
+++ lowparse.c 13 Oct 2012 14:23:14 -0000
@@ -279,11 +279,9 @@ Parse_ReadNextConditionalLine(Buffer lin
if (c == '\n')
current->origin.lineno++;
}
- if (c == EOF) {
- Parse_Error(PARSE_FATAL,
- "Unclosed conditional");
+ if (c == EOF)
+ /* Unclosed conditional, reported by cond.c */
return NULL;
- }
}
current->origin.lineno++;
}
@@ -452,7 +450,6 @@ Parse_ReadNormalLine(Buffer linebuf)
return NULL;
else {
read_logical_line(linebuf, c);
- Buf_KillTrailingSpaces(linebuf);
return Buf_Retrieve(linebuf);
}
}
@@ -489,10 +486,8 @@ Parse_FillLocation(Location *origin)
void
Parse_ReportErrors(void)
{
- if (fatal_errors) {
- fprintf(stderr,
- "Fatal errors encountered -- cannot continue\n");
+ if (fatal_errors)
exit(1);
- } else
+ else
assert(current == NULL);
}
Index: parse.c
===================================================================
RCS file: /home/openbsd/cvs/src/usr.bin/make/parse.c,v
retrieving revision 1.107
diff -u -p -r1.107 parse.c
--- parse.c 9 Oct 2012 19:45:34 -0000 1.107
+++ parse.c 13 Oct 2012 14:23:17 -0000
@@ -1048,7 +1048,6 @@ strip_comments(Buffer copy, const char *
break;
}
Buf_Addi(copy, line, p);
- Buf_KillTrailingSpaces(copy);
return Buf_Retrieve(copy);
}
}
Index: varmodifiers.c
===================================================================
RCS file: /home/openbsd/cvs/src/usr.bin/make/varmodifiers.c,v
retrieving revision 1.32
diff -u -p -r1.32 varmodifiers.c
--- varmodifiers.c 12 Oct 2012 13:20:11 -0000 1.32
+++ varmodifiers.c 13 Oct 2012 10:15:01 -0000
@@ -1514,7 +1514,7 @@ VarModifiers_Apply(char *str, const stru
printf("Result is \"%s\"\n", str);
}
if (*tstr == '\0')
- Error("Unclosed variable specification");
+ Parse_Error(PARSE_FATAL, "Unclosed variable specification");
else
tstr++;
