Enhance file_analysis preprocessor a little more, use in idempotent test.

tools/ccanlint/ccanlint.h

@@ -108,11 +108,16 @@ enum line_compiled {
 	MAYBE_COMPILED,
 };
 
-/* Simple evaluator: if this pre-processor symbol is defined to this
- * value, is this line compiled? (Other symbols assumed undefined) */
+/* Simple evaluator.  If symbols are set this way, is this condition true?
+ * NULL values mean undefined, NULL symbol terminates. */
 enum line_compiled get_ccan_line_pp(struct pp_conditions *cond,
 				    const char *symbol,
-				    unsigned int value);
+				    const unsigned int *value, ...);
+
+/* Get token if it's equal to token. */
+bool get_token(const char **line, const char *token);
+/* Talloc copy of symbol token, or NULL.  Increment line. */
+char *get_symbol_token(void *ctx, const char **line);
 
 /* Similarly for ->doc_sections */
 struct list_head *get_ccan_file_docs(struct ccan_file *f);

tools/ccanlint/file_analysis.c

@@ -13,6 +13,7 @@
 #include <errno.h>
 #include <dirent.h>
 #include <ctype.h>
+#include <stdarg.h>
 
 char **get_ccan_file_lines(struct ccan_file *f)
 {
@@ -57,6 +58,7 @@ static void add_files(struct manifest *m, const char *dir)
 
 		f = talloc(m, struct ccan_file);
 		f->lines = NULL;
+		f->line_info = NULL;
 		f->doc_sections = NULL;
 		f->name = talloc_asprintf(f, "%s%s", dir, ent->d_name);
 		if (lstat(f->name, &st) != 0)
@@ -242,7 +244,7 @@ static bool continues(const char *line)
 }
 
 /* Get token if it's equal to token. */
-static bool get_token(const char **line, const char *token)
+bool get_token(const char **line, const char *token)
 {
 	unsigned int toklen;
 
@@ -261,7 +263,7 @@ static bool get_token(const char **line, const char *token)
 	return false;
 }
 
-static char *get_symbol_token(void *ctx, const char **line)
+char *get_symbol_token(void *ctx, const char **line)
 {
 	unsigned int toklen;
 	char *ret;
@@ -421,47 +423,102 @@ struct line_info *get_ccan_line_info(struct ccan_file *f)
 	return f->line_info;
 }
 
-enum line_compiled get_ccan_line_pp(struct pp_conditions *cond,
-				    const char *symbol,
-				    unsigned int value)
+struct symbol {
+	struct list_node list;
+	const char *name;
+	const unsigned int *value;
+};
+
+static struct symbol *find_symbol(struct list_head *syms, const char *sym)
 {
-	enum line_compiled ret;
+	struct symbol *i;
+
+	list_for_each(syms, i, list)
+		if (streq(sym, i->name))
+			return i;
+	return NULL;
+}
+
+static enum line_compiled get_pp(struct pp_conditions *cond,
+				 struct list_head *syms)
+{
+	struct symbol *sym;
+	unsigned int val;
+	enum line_compiled parent, ret;
 
 	/* No conditions?  Easy. */
 	if (!cond)
 		return COMPILED;
 
 	/* Check we get here at all. */
-	ret = get_ccan_line_pp(cond->parent, symbol, value);
-	if (ret != COMPILED)
-		return ret;
+	parent = get_pp(cond->parent, syms);
+	if (parent == NOT_COMPILED)
+		return NOT_COMPILED;
+
+	if (cond->type == PP_COND_UNKNOWN)
+		return MAYBE_COMPILED;
+
+	sym = find_symbol(syms, cond->symbol);
+	if (!sym)
+		return MAYBE_COMPILED;
 
 	switch (cond->type) {
 	case PP_COND_IF:
-		if (streq(cond->symbol, symbol)) {
-			if (!value == cond->inverse)
-				return COMPILED;
-			else
-				return NOT_COMPILED;
-		}
-		/* Unknown symbol, will be 0. */
-		if (cond->inverse)
-			return COMPILED;
-		return NOT_COMPILED;
+		/* Undefined is 0. */
+		val = sym->value ? *sym->value : 0;
+		if (!val == cond->inverse)
+			ret = COMPILED;
+		else
+			ret = NOT_COMPILED;
+		break;
 
 	case PP_COND_IFDEF:
-		if (streq(cond->symbol, symbol)) {
-			if (cond->inverse)
-				return NOT_COMPILED;
-			else
-				return COMPILED;
-		}
-		/* Unknown symbol, assume undefined. */
-		if (cond->inverse)
-			return COMPILED;
-		return NOT_COMPILED;
-		
-	default: /* Unknown. */
-		return MAYBE_COMPILED;
+		if (cond->inverse == !sym->value)
+			ret = COMPILED;
+		else
+			ret = NOT_COMPILED;
+		break;
+
+	default:
+		abort();
 	}
+
+	/* If parent didn't know, NO == NO, but YES == MAYBE. */
+	if (parent == MAYBE_COMPILED && ret == COMPILED)
+		ret = MAYBE_COMPILED;
+	return ret;
 }
+
+static void add_symbol(struct list_head *head,
+		       const char *symbol, const unsigned int *value)
+{
+	struct symbol *sym = talloc(head, struct symbol);
+	sym->name = symbol;
+	sym->value = value;
+	list_add(head, &sym->list);
+}
+	
+enum line_compiled get_ccan_line_pp(struct pp_conditions *cond,
+				    const char *symbol,
+				    const unsigned int *value,
+				    ...)
+{
+	enum line_compiled ret;
+	struct list_head *head;
+	va_list ap;
+
+	head = talloc(NULL, struct list_head);
+	list_head_init(head);
+
+	va_start(ap, value);
+	add_symbol(head, symbol, value);
+
+	while ((symbol = va_arg(ap, const char *)) != NULL) {
+		value = va_arg(ap, const unsigned int *);
+		add_symbol(head, symbol, value);
+	}
+	ret = get_pp(cond, head);
+	talloc_free(head);
+	return ret;
+}
+

tools/ccanlint/idempotent.c

@@ -22,81 +22,92 @@ static const char explain[]
   "...\n"
   "#endif /* MY_HEADER_H */\n";
 
-static char *get_ifndef_sym(char *line)
+static char *report_idem(struct ccan_file *f, char *sofar)
 {
-	line += strspn(line, SPACE_CHARS);
-	if (line[0] == '#')
-	{
-		line++;
-		line += strspn(line, SPACE_CHARS);
-		if (strstarts(line, "ifndef") && isspace(line[6]))
-			return line+6+strspn(line+6, SPACE_CHARS);
-		else if (strstarts(line, "if"))
-		{
-			line += 2;
-			line += strspn(line, SPACE_CHARS);
-			if (line[0] == '!')
-			{
-				line++;
-				line += strspn(line, SPACE_CHARS);
-				if (strstarts(line, "defined"))
-				{
-					line += 7;
-					line += strspn(line, SPACE_CHARS);
-					if (line[0] == '(')
-					{
-						line++;
-						line += strspn(line,
-							SPACE_CHARS);
-					}
-					return line;
-				}
-			}
-		}
+	struct line_info *line_info;
+	unsigned int i, first_preproc_line;
+	const char *line, *sym;
+
+	line_info = get_ccan_line_info(f);
+	if (f->num_lines < 3)
+		/* FIXME: We assume small headers probably uninteresting. */
+		return sofar;
+
+	for (i = 0; i < f->num_lines; i++) {
+		if (line_info[i].type == DOC_LINE
+		    || line_info[i].type == COMMENT_LINE)
+			continue;
+		if (line_info[i].type == CODE_LINE)
+			return talloc_asprintf_append(sofar,
+			      "%s:%u:expect first non-comment line to be #ifndef.\n", f->name, i+1);
+		else if (line_info[i].type == PREPROC_LINE)
+			break;
 	}
-	return NULL;
-}
 
-static int is_define(char *line, char *id, size_t id_len)
-{
-	line += strspn(line, SPACE_CHARS);
-	if (line[0] == '#')
-	{
-		line++;
-		line += strspn(line, SPACE_CHARS);
-		if (strstarts(line, "define") && isspace(line[6]))
-		{
-			line += 6;
-			line += strspn(line, SPACE_CHARS);
-			if (strspn(line, IDENT_CHARS) == id_len &&
-			    memcmp(id, line, id_len) == 0)
-				return 1;
-		}
+	/* No code at all?  Don't complain. */
+	if (i == f->num_lines)
+		return sofar;
+
+	first_preproc_line = i;
+	for (i = first_preproc_line+1; i < f->num_lines; i++) {
+		if (line_info[i].type == DOC_LINE
+		    || line_info[i].type == COMMENT_LINE)
+			continue;
+		if (line_info[i].type == CODE_LINE)
+			return talloc_asprintf_append(sofar,
+			      "%s:%u:expect second line to be #define.\n", f->name, i+1);
+		else if (line_info[i].type == PREPROC_LINE)
+			break;
 	}
-	return 0;
-}
 
-static char *report_idem(struct ccan_file *f, char *sofar)
-{
-	char **lines;
-	char *id;
-	size_t id_len;
+	/* No code at all?  Weird. */
+	if (i == f->num_lines)
+		return sofar;
 
-	lines = get_ccan_file_lines(f);
-	if (f->num_lines < 3)
-		/* FIXME: We assume small headers probably uninteresting. */
-		return NULL;
+	/* We expect a condition on this line. */
+	if (!line_info[i].cond) {
+		return talloc_asprintf_append(sofar,
+					      "%s:%u:expected #ifndef.\n",
+					      f->name, first_preproc_line+1);
+	}
+
+	line = f->lines[i];
 
-	id = get_ifndef_sym(lines[0]);
-	if (!id)
+	/* We expect the condition to be ! IFDEF <symbol>. */
+	if (line_info[i].cond->type != PP_COND_IFDEF
+	    || !line_info[i].cond->inverse) {
 		return talloc_asprintf_append(sofar,
-			"%s:1:expect first line to be #ifndef.\n", f->name);
-	id_len = strspn(id, IDENT_CHARS);
+					      "%s:%u:expected #ifndef.\n",
+					      f->name, first_preproc_line+1);
+	}
 
-	if (!is_define(lines[1], id, id_len))
+	/* And this to be #define <symbol> */
+	if (!get_token(&line, "#"))
+		abort();
+	if (!get_token(&line, "define")) {
 		return talloc_asprintf_append(sofar,
-			"%s:2:expect second line to be '#define %.*s'.\n",
-			f->name, (int)id_len, id);
+			      "%s:%u:expected '#define %s'.\n",
+			      f->name, i+1, line_info[i].cond->symbol);
+	}
+	sym = get_symbol_token(f, &line);
+	if (!sym || !streq(sym, line_info[i].cond->symbol)) {
+		return talloc_asprintf_append(sofar,
+			      "%s:%u:expected '#define %s'.\n",
+			      f->name, i+1, line_info[i].cond->symbol);
+	}
+
+	/* Rest of code should all be covered by that conditional. */
+	for (i++; i < f->num_lines; i++) {
+		unsigned int val = 0;
+		if (line_info[i].type == DOC_LINE
+		    || line_info[i].type == COMMENT_LINE)
+			continue;
+		if (get_ccan_line_pp(line_info[i].cond, sym, &val)
+		    != NOT_COMPILED)
+			return talloc_asprintf_append(sofar,
+			      "%s:%u:code outside idempotent region.\n",
+			      f->name, i+1);
+	}
 
 	return sofar;
 }

tools/ccanlint/test/run-file_analysis.c

@@ -100,7 +100,7 @@ int main(int argc, char *argv[])
 	struct line_info *line_info;
 	struct ccan_file *f = talloc(NULL, struct ccan_file);
 
-	plan_tests(NUM_LINES * 2 + 2 + 66);
+	plan_tests(NUM_LINES * 2 + 2 + 86);
 
 	f->num_lines = NUM_LINES;
 	f->line_info = NULL;
@@ -176,26 +176,55 @@ int main(int argc, char *argv[])
 
 	/* Now check using interface. */
 	for (i = 0; i < f->num_lines; i++) {
+		unsigned int val = 1;
 		if (streq(testfile[i].line, "BAR")) {
-			ok1(get_ccan_line_pp(line_info[i].cond, "BAR", 1)
-			    == COMPILED);
-			ok1(get_ccan_line_pp(line_info[i].cond, "FOO", 1)
-			    == NOT_COMPILED);
+			/* If we don't know if the TEST_H was undefined,
+			 * best we get is a MAYBE. */
+			ok1(get_ccan_line_pp(line_info[i].cond, "BAR", &val,
+					     NULL) == MAYBE_COMPILED);
+			ok1(get_ccan_line_pp(line_info[i].cond, "BAR", NULL,
+					     NULL) == NOT_COMPILED);
+			ok1(get_ccan_line_pp(line_info[i].cond, "BAR", &val,
+					     "TEST_H", NULL,
+					     NULL) == COMPILED);
+			ok1(get_ccan_line_pp(line_info[i].cond, "BAR", NULL,
+					     "TEST_H", NULL,
+					     NULL) == NOT_COMPILED);
 		} else if (streq(testfile[i].line, "!BAR")) {
-			ok1(get_ccan_line_pp(line_info[i].cond, "BAR", 1)
-			    == NOT_COMPILED);
-			ok1(get_ccan_line_pp(line_info[i].cond, "FOO", 1)
-			    == COMPILED);
+			ok1(get_ccan_line_pp(line_info[i].cond, "BAR", &val,
+					     NULL) == NOT_COMPILED);
+			ok1(get_ccan_line_pp(line_info[i].cond, "BAR", NULL,
+					     NULL) == MAYBE_COMPILED);
+			ok1(get_ccan_line_pp(line_info[i].cond, "BAR", &val,
+					     "TEST_H", NULL,
+					     NULL) == NOT_COMPILED);
+			ok1(get_ccan_line_pp(line_info[i].cond, "BAR", NULL,
+					     "TEST_H", NULL,
+					     NULL) == COMPILED);
 		} else if (streq(testfile[i].line, "HAVE_BAR")) {
-			ok1(get_ccan_line_pp(line_info[i].cond, "HAVE_BAR", 1)
-			    == COMPILED);
-			ok1(get_ccan_line_pp(line_info[i].cond, "HAVE_BAR", 0)
-			    == NOT_COMPILED);
+			ok1(get_ccan_line_pp(line_info[i].cond, "HAVE_BAR",
+					     &val, NULL) == MAYBE_COMPILED);
+			ok1(get_ccan_line_pp(line_info[i].cond, "HAVE_BAR",
+					     &val, "TEST_H", NULL,
+					     NULL) == COMPILED);
+			val = 0;
+			ok1(get_ccan_line_pp(line_info[i].cond, "HAVE_BAR",
+					     &val, NULL) == NOT_COMPILED);
+			ok1(get_ccan_line_pp(line_info[i].cond, "HAVE_BAR",
+					     &val, "TEST_H", NULL,
+					     NULL) == NOT_COMPILED);
 		} else if (streq(testfile[i].line, "HAVE_FOO")) {
-			ok1(get_ccan_line_pp(line_info[i].cond, "HAVE_FOO", 1)
-			    == COMPILED);
-			ok1(get_ccan_line_pp(line_info[i].cond, "HAVE_FOO", 0)
-			    == NOT_COMPILED);
+			ok1(get_ccan_line_pp(line_info[i].cond, "HAVE_FOO",
+					     &val, NULL) == MAYBE_COMPILED);
+			ok1(get_ccan_line_pp(line_info[i].cond, "HAVE_FOO",
+					     &val, "TEST_H", NULL,
+					     NULL) == COMPILED);
+			val = 0;
+			ok1(get_ccan_line_pp(line_info[i].cond, "HAVE_FOO",
+					     &val, NULL) == NOT_COMPILED);
+			ok1(get_ccan_line_pp(line_info[i].cond, "HAVE_FOO",
+					     &val, "TEST_H", NULL,
+					     NULL) == NOT_COMPILED);
 		}
 	}