[Rpm-maint] [PATCH] Check for undefined macros

Alexey Tourbin alexey.tourbin at gmail.com
Sun Jan 27 05:27:09 UTC 2013


This change introduces a generalized routine rpmExpandMacros() to expand
macros on behalf of user input, with improved diagnostic facilities.
In particular, one of its arguments is a callback function which is
called whenever an undefined macro is encountered in user input.

When specfile parser calls this routine, it supplies a custom function
to handle undefined macros.  This function is keenly aware of specfile
syntax: valid specfile tokens, such as %prep and %setup, when found in
the right context, are not treated as undefined macros.  Otherwise,
there are 3 possibilities.
- Undefined macros in preamble and %pre/%post/... scriptlets raise
  an error, unless a non-build parse is performed; this is to prevent
  malformed packages from being built.
- In other cases, a warning is issued.
- Some tokens are permitted beyond their context (e.g. %ghost in
  %changelog), to comply with traditional/sloppy usage.  Also, undefined
  macros found in comments normally do not produce a warning.
---
 build/files.c             |  66 ++++++++++++++++++--
 build/pack.c              |  11 ++--
 build/parseSpec.c         | 154 +++++++++++++++++++++++++++++++++++++++++++---
 build/rpmbuild_internal.h |  11 ++++
 build/spec.c              |   3 +
 rpmio/macro.c             |  67 +++++++++++---------
 rpmio/rpmmacro.h          |  28 ++++++---
 7 files changed, 287 insertions(+), 53 deletions(-)

diff --git a/build/files.c b/build/files.c
index 10d7fff..881176e 100644
--- a/build/files.c
+++ b/build/files.c
@@ -1657,6 +1657,62 @@ exit:
     return rc;
 }
 
+/* Check if str looks like attr */
+static int eqAttr(const char *s, const char *a)
+{
+    size_t len = strlen(a);
+    if (rstreqn(s, a, len)) {
+	int c = s[len];
+	if (!(risalnum(c) || c == '_'))
+	    return 1;
+    }
+    return 0;
+}
+
+int isFileAttr(const char *s)
+{
+    /* parseForVerify */
+    if (eqAttr(s, "%verify") ||
+	eqAttr(s, "%defverify"))
+	    return 1;
+    /* parseForAttr */
+    if (eqAttr(s, "%attr") ||
+	eqAttr(s, "%defattr"))
+	    return 1;
+    /* parseForDev */
+    if (eqAttr(s, "%dev"))
+	    return 1;
+    /* parseForConfig */
+    if (eqAttr(s, "%config"))
+	    return 1;
+    /* parseForLang */
+    if (eqAttr(s, "%lang"))
+	    return 1;
+    /* parseForCaps */
+    if (eqAttr(s, "%caps"))
+	    return 1;
+    /* parseForSimple */
+    for (VFA_t *vfa = virtualAttrs; vfa->attribute != NULL; vfa++)
+	if (eqAttr(s, vfa->attribute))
+	    return 1;
+    return 0;
+}
+
+/* Handle undefined macros */
+static void undefined(const char *fileName, int lineNum,
+	const char *s, const char *f, const char *fe,
+	const char *exp, int level, void *arg)
+{
+    if (s + 1 == f && isFileAttr(s))
+	return;
+    rpmlog(RPMLOG_WARNING,
+	    _("%s:%d: Undefined macro %%%.*s\n"),
+	    fileName, lineNum, (int)(fe - f), f);
+    (void) exp;
+    (void) level;
+    (void) arg;
+}
+
 static rpmRC readFilesManifest(rpmSpec spec, Package pkg, const char *path)
 {
     char *fn, buf[BUFSIZ];
@@ -1676,13 +1732,15 @@ static rpmRC readFilesManifest(rpmSpec spec, Package pkg, const char *path)
 	goto exit;
     }
 
+    int lineNum = 0;
     while (fgets(buf, sizeof(buf), fd)) {
 	handleComments(buf);
-	if (expandMacros(spec, spec->macros, buf, sizeof(buf))) {
-	    rpmlog(RPMLOG_ERR, _("line: %s\n"), buf);
+	char *exp = rpmExpandMacros(spec->macros, buf,
+			basename(fn), ++lineNum, undefined, NULL);
+	if (exp == NULL)
 	    goto exit;
-	}
-	argvAdd(&(pkg->fileList), buf);
+	argvAdd(&(pkg->fileList), exp);
+	free(exp);
     }
 
     if (ferror(fd))
diff --git a/build/pack.c b/build/pack.c
index 9679d87..a8d1862 100644
--- a/build/pack.c
+++ b/build/pack.c
@@ -22,6 +22,7 @@
 #include "build/rpmbuild_misc.h"
 
 #include "debug.h"
+#include <libgen.h>
 
 typedef struct cpioSourceArchive_s {
     rpm_loff_t	cpioArchiveSize;
@@ -91,12 +92,14 @@ static rpmRC addFileToTag(rpmSpec spec, const char * file,
 	}
     }
 
+    int lineNum = 0;
     while (fgets(buf, sizeof(buf), f)) {
-	if (expandMacros(spec, spec->macros, buf, sizeof(buf))) {
-	    rpmlog(RPMLOG_ERR, _("%s: line: %s\n"), fn, buf);
+	char *exp = rpmExpandMacros(spec->macros, buf,
+			basename(fn), ++lineNum, NULL, NULL);
+	if (exp == NULL)
 	    goto exit;
-	}
-	appendStringBuf(sb, buf);
+	appendStringBuf(sb, exp);
+	free(exp);
     }
     headerPutString(h, tag, getStringBuf(sb));
     rc = RPMRC_OK;
diff --git a/build/parseSpec.c b/build/parseSpec.c
index fcec5c5..3b12d41 100644
--- a/build/parseSpec.c
+++ b/build/parseSpec.c
@@ -16,6 +16,7 @@
 #include "build/rpmbuild_internal.h"
 #include "build/rpmbuild_misc.h"
 #include "debug.h"
+#include <libgen.h>
 
 #define SKIPSPACE(s) { while (*(s) && risspace(*(s))) (s)++; }
 #define SKIPNONSPACE(s) { while (*(s) && !risspace(*(s))) (s)++; }
@@ -63,10 +64,13 @@ static const struct PartRec {
     {0, 0, 0}
 };
 
-int isPart(const char *line)
+static int isPartToken(const char *line, int wb)
 {
     const struct PartRec *p;
 
+    if (*line != '%')
+	return PART_NONE;
+
     for (p = partList; p->token != NULL; p++) {
 	char c;
 	if (rstrncasecmp(line, p->token, p->len))
@@ -74,11 +78,139 @@ int isPart(const char *line)
 	c = *(line + p->len);
 	if (c == '\0' || risspace(c))
 	    break;
+	if (wb && !(risalnum(c) || c == '_'))
+	    break;
     }
 
     return (p->token ? p->part : PART_NONE);
 }
 
+int isPart(const char *line)
+{
+    return isPartToken(line, 0);
+}
+
+/* Check for conditional/include directives */
+static int isCond(const char *s)
+{
+    if (ISMACROWITHARG(s, "%if") ||
+	ISMACROWITHARG(s, "%ifarch") ||
+	ISMACROWITHARG(s, "%ifnarch") ||
+	ISMACROWITHARG(s, "%ifos") ||
+	ISMACROWITHARG(s, "%ifnos"))
+	    return 1;
+    if (ISMACRO(s, "%else") ||
+	ISMACRO(s, "%endif") ||
+	ISMACRO(s, "%include"))
+	    return 1;
+    return 0;
+}
+
+/* Check for %prep specials */
+static int isPrep(const char *s)
+{
+    if (rstreqn(s, "%setup", sizeof("%setup") - 1) ||
+	rstreqn(s, "%patch", sizeof("%patch") - 1))
+	    return 1;
+    return 0;
+}
+
+/* Handle undefined macros */
+static void undefined(const char *fileName, int lineNum,
+	const char *s, const char *f, const char *fe,
+	const char *exp, int level, void *arg)
+{
+    rpmSpec spec = arg;
+    assert(spec);
+
+    /* effective part */
+    int part = spec->parsePart;
+
+    /* skip concatenated lines */
+    const char *e;
+    while ((e = strchr(exp, '\n')) != NULL) {
+	int p = isPart(exp);
+	if (p != PART_NONE)
+	    part = p;
+	exp = e + 1;
+    }
+
+    /* too much noise about comments */
+    if (*exp == '#' && level == 1)
+	return;
+
+    /* whether s is a simple %token */
+    int token = (s + 1 == f);
+
+    /* token at line start */
+    if (token && *exp == '\0') {
+	/* next part */
+	if (isPart(s))
+	    return;
+	/* %prep special */
+	if (part == PART_PREP && isPrep(s))
+	    return;
+    }
+
+    /* conditional, possibly indented */
+    if (token && isCond(s)) {
+	e = exp;
+	SKIPSPACE(e);
+	if (*e == '\0')
+	    return;
+    }
+
+    /* treat the rest of %part line as preamble */
+    if (isPart(exp))
+	part = PART_PREAMBLE;
+
+    /* permit some tokens in some sections */
+    switch (part) {
+    case PART_CHANGELOG:
+	if (token && isPartToken(s, 1))
+	    return;
+	/* fallthrough */
+    case PART_PREP:
+    case PART_BUILD:
+    case PART_INSTALL:
+    case PART_CHECK:
+    case PART_CLEAN:
+    case PART_FILES:
+	if (token && isFileAttr(s))
+	    return;
+	break;
+    }
+
+    /* warning vs error */
+    int rc = 0;
+    switch (part) {
+    case PART_PREAMBLE:
+    case PART_PRE:
+    case PART_POST:
+    case PART_PREUN:
+    case PART_POSTUN:
+    case PART_PRETRANS:
+    case PART_POSTTRANS:
+    case PART_TRIGGERPREIN:
+    case PART_TRIGGERIN:
+    case PART_TRIGGERUN:
+    case PART_TRIGGERPOSTUN:
+	if (!(spec->flags & RPMSPEC_FORCE))
+	    rc = 1;
+	break;
+    }
+
+    /* XXX tolerate sloppy %global usage in preamble */
+    if (part == PART_PREAMBLE && level > 1)
+        rc = 0;
+
+    /* wages of sin */
+    rpmlog(rc ? RPMLOG_ERR : RPMLOG_WARNING,
+	    _("%s:%d: Undefined macro %%%.*s\n"),
+	    fileName, lineNum, (int)(fe - f), f);
+    spec->errors += rc;
+}
+
 /**
  */
 static int matchTok(const char *token, const char *line)
@@ -204,11 +336,15 @@ static int copyNextLineFromOFI(rpmSpec spec, OFI_t *ofi)
 	spec->lbufOff = 0;
 
 	/* Don't expand macros (eg. %define) in false branch of %if clause */
-	if (spec->readStack->reading &&
-	    expandMacros(spec, spec->macros, spec->lbuf, spec->lbufSize)) {
-		rpmlog(RPMLOG_ERR, _("line %d: %s\n"),
-			spec->lineNum, spec->lbuf);
+	if (spec->readStack->reading) {
+	    char *exp = rpmExpandMacros(spec->macros, spec->lbuf,
+				basename(ofi->fileName), ofi->lineNum,
+				undefined, spec);
+	    if (exp == NULL)
 		return -1;
+	    size_t len = rstrlcpy(spec->lbuf, exp, spec->lbufSize);
+	    assert(len < spec->lbufSize);
+	    free(exp);
 	}
 	spec->nextline = spec->lbuf;
     }
@@ -556,12 +692,13 @@ static void addTargets(Package Pkgs)
 static rpmSpec parseSpec(const char *specFile, rpmSpecFlags flags,
 			 const char *buildRoot, int recursing)
 {
-    int parsePart = PART_PREAMBLE;
     int initialPackage = 1;
     rpmSpec spec;
     
     /* Set up a new Spec structure with no packages. */
     spec = newSpec();
+#define parsePart (spec->parsePart)
+    parsePart = PART_PREAMBLE;
 
     spec->specFile = rpmGetPath(specFile, NULL);
     pushOFI(spec, spec->specFile);
@@ -702,10 +839,13 @@ static rpmSpec parseSpec(const char *specFile, rpmSpecFlags flags,
 	if (!headerIsEntry(pkg->header, RPMTAG_DESCRIPTION)) {
 	    rpmlog(RPMLOG_ERR, _("Package has no %%description: %s\n"),
 		   headerGetString(pkg->header, RPMTAG_NAME));
-	    goto errxit;
+	    spec->errors++;
 	}
     }
 
+    if (spec->errors)
+	goto errxit;
+
     /* Add arch, os and platform, self-provides etc for each package */
     addTargets(spec->packages);
 
diff --git a/build/rpmbuild_internal.h b/build/rpmbuild_internal.h
index fb6198d..59c201b 100644
--- a/build/rpmbuild_internal.h
+++ b/build/rpmbuild_internal.h
@@ -80,6 +80,9 @@ struct rpmSpec_s {
     StringBuf parsed;		/*!< parsed spec contents */
 
     Package packages;		/*!< Package list. */
+
+    int parsePart;		/*!< current part */
+    int errors;			/*!< pending errors */
 };
 
 /** \ingroup rpmbuild
@@ -189,6 +192,14 @@ RPM_GNUC_INTERNAL
 int isPart(const char * line)	;
 
 /** \ingroup rpmbuild
+ * Check for a valid file attribute (in %files section).
+ * @param s		string to check
+ * @return		1 if file attr, 0 if not
+ */
+RPM_GNUC_INTERNAL
+int isFileAttr(const char *s);
+
+/** \ingroup rpmbuild
  * Parse %%build/%%install/%%clean section(s) of a spec file.
  * @param spec		spec file control structure
  * @param parsePart	current rpmParseState
diff --git a/build/spec.c b/build/spec.c
index 4b6b680..93b7448 100644
--- a/build/spec.c
+++ b/build/spec.c
@@ -201,6 +201,9 @@ rpmSpec newSpec(void)
     spec->flags = RPMSPEC_NONE;
 
     spec->macros = rpmGlobalMacroContext;
+
+    spec->parsePart = PART_NONE;
+    spec->errors = 0;
     
 #ifdef WITH_LUA
     {
diff --git a/rpmio/macro.c b/rpmio/macro.c
index a2778d1..2e5d3d0 100644
--- a/rpmio/macro.c
+++ b/rpmio/macro.c
@@ -69,6 +69,12 @@ typedef struct MacroBuf_s {
     int macro_trace;		/*!< Pre-print macro to expand? */
     int expand_trace;		/*!< Post-print macro expansion? */
     rpmMacroContext mc;
+    const char *fileName;
+    int lineNum;
+    void (*undefined)(const char *fileName, int lineNum,
+	    const char *s, const char *f, const char *fe,
+	    const char *exp, int level, void *arg);
+    void *arg;
 } * MacroBuf;
 
 #define	_MAX_MACRO_DEPTH	16
@@ -1370,10 +1376,19 @@ expandMacro(MacroBuf mb, const char *src, size_t slen)
 		continue;
 	}
 
+	/* Handle user-level undefined macros */
+	if (me == NULL && (risalpha(*f) || *f == '_')) {
+		if (mb->undefined)
+			mb->undefined(mb->fileName, mb->lineNum,
+				s - 1, f, fe, mb->buf, mb->depth, mb->arg);
+		else if (mb->fileName)
+			rpmlog(RPMLOG_WARNING,
+				_("%s:%d: Undefined macro %%%.*s\n"),
+				mb->fileName, mb->lineNum, (int)(fe - f), f);
+	}
+
 	if (me == NULL) {	/* leave unknown %... as is */
-		/* XXX hack to permit non-overloaded %foo to be passed */
-		c = '%';	/* XXX only need to save % */
-		mbAppend(mb, c);
+		mbAppend(mb, '%');
 		continue;
 	}
 
@@ -1414,36 +1429,32 @@ expandMacro(MacroBuf mb, const char *src, size_t slen)
 
 /* =============================================================== */
 
-static int doExpandMacros(rpmMacroContext mc, const char *src, char **target)
+char *rpmExpandMacros(rpmMacroContext mc, const char *src,
+	const char *fileName, int lineNum,
+	void undefined(const char *fileName, int lineNum,
+		const char *s, const char *f, const char *fe,
+		const char *exp, int level, void *arg),
+	void *arg)
 {
-    MacroBuf mb = xcalloc(1, sizeof(*mb));
-    int rc = 0;
-
-    if (mc == NULL) mc = rpmGlobalMacroContext;
+    struct MacroBuf_s mb = { 0 };
 
-    mb->buf = NULL;
-    mb->depth = 0;
-    mb->macro_trace = print_macro_trace;
-    mb->expand_trace = print_expand_trace;
-    mb->mc = mc;
+    mb.macro_trace = print_macro_trace;
+    mb.expand_trace = print_expand_trace;
+    mb.mc = mc ? mc : rpmGlobalMacroContext;
 
-    rc = expandMacro(mb, src, 0);
+    mb.fileName = fileName;
+    mb.lineNum = lineNum;
+    mb.undefined = undefined;
+    mb.arg = arg;
 
-    mb->buf[mb->tpos] = '\0';	/* XXX just in case */
-    /* expanded output is usually much less than alloced buffer, downsize */
-    *target = xrealloc(mb->buf, mb->tpos + 1);
+    int rc = expandMacro(&mb, src, 0);
 
-    _free(mb);
-    return rc;
-}
+    if (rc == 0)
+        /* expanded output is usually much less than alloced buffer, downsize */
+	return xrealloc(mb.buf, mb.tpos + 1);
 
-int expandMacros(void * spec, rpmMacroContext mc, char * sbuf, size_t slen)
-{
-    char *target = NULL;
-    int rc = doExpandMacros(mc, sbuf, &target);
-    rstrlcpy(sbuf, target, slen);
-    free(target);
-    return rc;
+    free(mb.buf);
+    return NULL;
 }
 
 void
@@ -1632,7 +1643,7 @@ rpmExpand(const char *arg, ...)
 	pe = stpcpy(pe, s);
     va_end(ap);
 
-    (void) doExpandMacros(NULL, buf, &ret);
+    ret = rpmExpandMacros(NULL, buf, NULL, 0, NULL, NULL);
 
     free(buf);
 exit:
diff --git a/rpmio/rpmmacro.h b/rpmio/rpmmacro.h
index 765c78c..e11d5f4 100644
--- a/rpmio/rpmmacro.h
+++ b/rpmio/rpmmacro.h
@@ -52,18 +52,26 @@ void	rpmDumpMacroTable	(rpmMacroContext mc,
 					FILE * fp);
 
 /** \ingroup rpmmacro
- * Expand macro into buffer.
- * @deprecated Use rpmExpand().
- * @todo Eliminate from API.
- * @param spec		cookie (unused)
+ * Expand macros (on behalf of user input)
  * @param mc		macro context (NULL uses global context).
- * @retval sbuf		input macro to expand, output expansion
- * @param slen		size of buffer
- * @return		0 on success
+ * @param src		user input to expand
+ * @param fileName	input comes from
+ * @param lineNum	input line number
+ * @param undefined	handler for undefined macros (NULL == warn)
+ * @param s		macro invocation, starting with '%'
+ * @param f		macro name start
+ * @param fe		macro name end
+ * @param exp		already expanded part
+ * @param level		macro recursion level
+ * @param arg		callback argument
+ * @return		macro expansion (malloc'ed), NULL on error
  */
-int	expandMacros	(void * spec, rpmMacroContext mc,
-				char * sbuf,
-				size_t slen);
+char * rpmExpandMacros	(rpmMacroContext mc, const char *src,
+				const char *fileName, int lineNum,
+				void undefined(const char *fileName, int lineNum,
+					const char *s, const char *f, const char *fe,
+					const char *exp, int level, void *arg),
+				void *arg);
 
 /** \ingroup rpmmacro
  * Add macro to context.
-- 
1.8.1



More information about the Rpm-maint mailing list