On Sun, Oct 14, 2012 at 06:49:34PM +0200, Marc Espie wrote:
> 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.
This part was confusing. Put on hold for now.
> - make sure errors go thru handle_all_signals(), so that signals are handled
> properly.
Implied by posix, so keep.
The rest doesn't change, new patch follows (smaller actually)
>
> 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 17:13:36 -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 *);
+static void internal_print_errors(void);
static void
kill_with_sudo_maybe(pid_t pid, int signo, const char *p)
@@ -259,8 +262,8 @@ print_error(Job *j)
may_remove_target(j);
}
-void
-print_errors(void)
+static void
+internal_print_errors()
{
Job *j, *k, *jnext;
@@ -282,6 +285,13 @@ print_errors(void)
}
}
+void
+print_errors(void)
+{
+ handle_all_signals();
+ internal_print_errors();
+}
+
static void
setup_signal(int sig)
{
@@ -842,7 +852,7 @@ handle_fatal_signal(int signo)
}
}
loop_handle_running_jobs();
- print_errors();
+ internal_print_errors();
/* die by that signal */
sigprocmask(SIG_BLOCK, &sigset, NULL);
@@ -850,6 +860,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++;
