commit 0b795256f75466f2b3d58fb7124a75a5e6829a9e
parent d6a2729ad86467cebdf450c60daa65df68d05bd6
Author: Roberto E. Vargas Caballero <k0ga@shike2.com>
Date:   Mon,  4 Nov 2024 18:03:35 +0100
make: Make sure to have a location for messages
Calling expandstring was removing any trace to the current line of the
current file, and actions didn't have a location, so any error executing
them could not be traced to the line in the Makefile.
Diffstat:
4 files changed, 83 insertions(+), 48 deletions(-)
diff --git a/src/cmd/scc-make/make.h b/src/cmd/scc-make/make.h
@@ -16,6 +16,16 @@ enum {
 	MAKEFLAGS,
 };
 
+struct loc {
+	char *fname;
+	int lineno;
+};
+
+struct action {
+	char *line;
+	struct loc loc;
+};
+
 struct target {
 	char *name;
 	char *target;
@@ -27,7 +37,7 @@ struct target {
 	struct target **deps;
 
 	int nactions;
-	char **actions;
+	struct action *actions;
 
 	struct target *next;
 };
@@ -39,7 +49,7 @@ extern char *estrdup(char *);
 extern void dumprules(void);
 extern void dumpmacros(void);
 
-extern char *expandstring(char *, Target *);
+extern char *expandstring(char *, Target *, struct loc *);
 extern void addtarget(char *, int);
 extern void inject(char *);
 extern int build(char *);
@@ -49,7 +59,8 @@ extern void debug(char *, ...);
 extern void error(char *, ...);
 extern void warning(char *, ...);
 extern void adddep(char *, char *);
-extern void addrule(char *, char **, int);
+extern void addrule(char *, struct action *, int);
+extern void freeloc(struct loc *);
 
 extern char *getmacro(char *);
 extern void setmacro(char *, char *, int, int);
diff --git a/src/cmd/scc-make/parser.c b/src/cmd/scc-make/parser.c
@@ -29,11 +29,6 @@ enum {
 	STEND,
 };
 
-struct loc {
-	char *fname;
-	int lineno;
-};
-
 struct input {
 	int siz;
 	int type;
@@ -187,6 +182,12 @@ setmacro(char *name, char *val, int where, int export)
 	}
 }
 
+void
+freeloc(struct loc *loc)
+{
+	free(loc->fname);
+}
+
 static struct loc *
 getloc(void)
 {
@@ -241,8 +242,9 @@ pop(void)
 	struct input *ip = input->prev;
 
 	if (input->type == FTFILE) {
-		fclose(input->fp);
-		free(input->loc.fname);
+		if (input->fp)
+			fclose(input->fp);
+		freeloc(&input->loc);
 	}
 	free(input->buf);
 	free(input);
@@ -253,7 +255,7 @@ pop(void)
 static void
 push(int type, ...)
 {
-	int len, pos;
+	int line, len, pos;
 	FILE *fp = NULL;
 	char *buf, *s, *fname = NULL;
 	va_list va;
@@ -262,8 +264,9 @@ push(int type, ...)
 	va_start(va, type);
 	switch (type) {
 	case FTFILE:
-		s = va_arg(va, char *);
 		fp = va_arg(va, FILE *);
+		s = va_arg(va, char *);
+		line = va_arg(va, int);
 		fname = estrdup(s);
 		buf = emalloc(BUFSIZ);
 		pos = len = BUFSIZ;
@@ -271,7 +274,7 @@ push(int type, ...)
 	case FTEXPAN:
 		s = va_arg(va, char *);
 		buf = estrdup(s);
-		pos = 0;
+		line = pos = 0;
 		len = strlen(s);
 		break;
 	}
@@ -283,7 +286,7 @@ push(int type, ...)
 	ip->type = type;
 	ip->fp = fp;
 	ip->loc.fname = fname;
-	ip->loc.lineno = 1;
+	ip->loc.lineno = line;
 	ip->pos = pos;
 	ip->prev = input;
 
@@ -312,14 +315,14 @@ include(char *s)
 	char *fil, *t;
 
 	s = trim(s);
-	fil = expandstring(s, NULL);
+	fil = expandstring(s, NULL, getloc());
 
 	t = trim(fil);
 	if (strlen(t) != 0) {
 		debug("including '%s'", t);
 		if ((fp = fopen(t, "r")) == NULL)
 			error("opening %s:%s", t, strerror(errno));
-		push(FTFILE, t, fp);
+		push(FTFILE, fp, t, 0);
 	}
 
 	free(fil);
@@ -336,7 +339,7 @@ nextline(void)
 
 repeat:
 	fp = input->fp;
-	if (feof(fp))
+	if (!fp || feof(fp))
 		return NULL;
 
 	lim = &input->buf[input->siz];
@@ -462,7 +465,7 @@ validchar(int c)
 static char *
 expandmacro(char *name)
 {
-	return expandstring(getmacro(name), NULL);
+	return expandstring(getmacro(name), NULL, getloc());
 }
 
 static void
@@ -635,7 +638,7 @@ expansion(Target *tp)
 				name[namei++] = c;
 				name[namei] = '\0';
 				st = STINTERNAL;
-				s = expandstring(name, tp);
+				s = expandstring(name, tp, getloc());
 				break;
 			}
 
@@ -680,7 +683,7 @@ expansion(Target *tp)
 	repl[repli] = '\0';
 	to[toi] = '\0';
 
-	erepl = expandstring(repl, tp);
+	erepl = expandstring(repl, tp, getloc());
 	replace(s, erepl, to);
 
 	free(erepl);
@@ -700,13 +703,14 @@ no_replace:
  * it later.
  */
 char *
-expandstring(char *line, Target *tp)
+expandstring(char *line, Target *tp, struct loc *loc)
 {
 	int c, n;
 	char *s;
 	struct input *ip = input;
 
 	input = NULL;
+	push(FTFILE, NULL, loc->fname, loc->lineno);
 	push(FTEXPAN, line);
 
 	n = 0;
@@ -850,18 +854,23 @@ readmacrodef(void)
 	return line;
 }
 
-static char *
+static struct action
 readcmd(void)
 {
 	int n, c;
-	char *line;
+	struct loc *loc;
+	struct action act;
 
 	skipspaces();
 
+	loc = getloc();
+	act.loc.fname = estrdup(loc->fname);
+	act.loc.lineno = loc->lineno;
+
 	n = 0;
-	line = NULL;
+	act.line = NULL;
 	while ((c = nextc()) != EOF) {
-		line = erealloc(line, n+1);
+		act.line = erealloc(act.line, n+1);
 		if (c == '\n')
 			break;
 		if (c == '\\') {
@@ -873,20 +882,21 @@ readcmd(void)
 			back(c);
 			c = '\\';
 		}
-		line[n++] = c;
+		act.line[n++] = c;
 	}
 	if (c == EOF)
 		error("EOF while looking for end of command");
-	line[n] = '\0';
+	act.line[n] = '\0';
 
-	return line;
+	return act;
 }
 
 static void
 rule(char *targets[], int ntargets)
 {
 	int c, i, j, ndeps, nactions;
-	char **actions, **deps = NULL;
+	struct action *acts;
+	char **deps = NULL;
 
 	if (ntargets == 0)
 		error("missing target");
@@ -900,11 +910,11 @@ rule(char *targets[], int ntargets)
 		error("garbage at the end of the line");
 
 	nactions = 0;
-	actions = NULL;
+	acts = NULL;
 	if (tok == ';') {
 		nactions++;
-		actions = erealloc(actions, nactions * sizeof(char *));
-		actions[nactions-1] = readcmd();
+		acts = erealloc(acts, nactions * sizeof(*acts));
+		acts[nactions-1] = readcmd();
 	}
 
 	for (;;) {
@@ -915,8 +925,8 @@ rule(char *targets[], int ntargets)
 		if (c != '\t')
 			break;
 		nactions++;
-		actions = erealloc(actions, nactions * sizeof(char *));
-		actions[nactions-1] = readcmd();
+		acts = erealloc(acts, nactions * sizeof(*acts));
+		acts[nactions-1] = readcmd();
 	}
 	back(c);
 
@@ -925,16 +935,18 @@ rule(char *targets[], int ntargets)
 		for (j = 0; j < ndeps; j++)
 			adddep(targets[i], deps[j]);
 		if (nactions > 0)
-			addrule(targets[i], actions, nactions);
+			addrule(targets[i], acts, nactions);
 	}
 
 	for (i = 0; i < ndeps; i++)
 		free(deps[i]);
 	free(deps);
 
-	for (i = 0; i < nactions; i++)
-		free(actions[i]);
-	free(actions);
+	for (i = 0; i < nactions; i++) {
+		free(acts[i].line);
+		freeloc(&acts[i].loc);
+	}
+	free(acts);
 }
 
 static void
@@ -1002,7 +1014,7 @@ parse(char *fname)
 	}
 
 	debug("parsing %s", fname);
-	push(FTFILE, fname, fp);
+	push(FTFILE, fp, fname, 0);
 	parseinput();
 
 	return 1;
@@ -1011,6 +1023,7 @@ parse(char *fname)
 void
 inject(char *s)
 {
+	push(FTFILE, NULL, "<internal>", 0);
 	push(FTEXPAN, s);
 	parseinput();
 }
diff --git a/src/cmd/scc-make/rules.c b/src/cmd/scc-make/rules.c
@@ -24,7 +24,7 @@ dumprules(void)
 				printf(" %s", (*q)->name);
 			putchar('\n');
 			for (i = 0; i < p->nactions; i++)
-				printf("\t%s\n", p->actions[i]);
+				printf("\t%s\n", p->actions[i].line);
 			putchar('\n');
 		}
 	}
@@ -138,11 +138,18 @@ adddep(char *target, char *dep)
 	debug("adding dependency %s <- %s", target, dep);
 }
 
+static void
+freeaction(struct action *act)
+{
+	free(act->line);
+	freeloc(&act->loc);
+}
+
 void
-addrule(char *target, char **actions, int n)
+addrule(char *target, struct action  *acts, int n)
 {
 	int i;
-	char **v;
+	struct action *v;
 	Target *tp = lookup(target);
 
 	debug("adding actions for target %s", target);
@@ -150,13 +157,16 @@ addrule(char *target, char **actions, int n)
 	if (tp->actions) {
 		debug("overring actions of target %s", target);
 		for (i = 0; i < tp->nactions; i++)
-			free(tp->actions[i]);
+			freeaction(&tp->actions[i]);
 		free(tp->actions);
 	}
 
-	v = emalloc(n * sizeof(char *));
-	for (i = 0; i < n; i++)
-		v[i] = estrdup(actions[i]);
+	v = emalloc(n * sizeof(*v));
+	for (i = 0; i < n; i++) {
+		v[i].line = estrdup(acts[i].line);
+		v[i].loc.lineno = acts[i].loc.lineno;
+		v[i].loc.fname = estrdup(acts[i].loc.fname);
+	}
 
 	tp->nactions = n;
 	tp->actions = v;
@@ -292,7 +302,8 @@ run(Target *tp)
 	}
 
 	for (i = 0; i < tp->nactions; i++) {
-		s = expandstring(tp->actions[i], tp);
+		struct action *p = &tp->actions[i];
+		s = expandstring(p->line, tp, &p->loc);
 		r = execline(tp, s, ignore, silent);
 		free(s);
 
diff --git a/tests/make/execute/0057-default.sh b/tests/make/execute/0057-default.sh
@@ -6,7 +6,7 @@ tmp1=tmp1.$$
 tmp2=tmp2.$$
 
 cat > $tmp2 <<'EOF'
-make: error: <stdin>:4: DEFAULT rule with prerequisites
+make: error: <stdin>:3: DEFAULT rule with prerequisites
 EOF
 
 scc-make -f- <<EOF > $tmp1 2>&1