[Rpm-maint] [rpm-software-management/rpm] Warn if a text is after %else or %endif (#625)

[Panu Matilainen]

pmatilai commented on this pull request.



> +    { LINE_IFNOS,       LEN_AND_STR("%ifnos")  , 1},
+    { LINE_ELSE,        LEN_AND_STR("%else")   , 0},
+    { LINE_ENDIF,       LEN_AND_STR("%endif")  , 0},
+    { LINE_INCLUDE,     LEN_AND_STR("%include"), 1},
+    { LINE_OTHER,       0, NULL                }
+};
+static const size_t numLineTypes = sizeof(specLineTypes)/sizeof(*specLineTypes);
+
+static int parseLineType(char *line)
+{
+    for (int i=0; i<(numLineTypes-1); i++) {
+	if (specLineTypes[i].withArgs) {
+	    if ( ISMACROWITHARG(line, specLineTypes[i].text, specLineTypes[i].textLen) )
+		return i;
+	} else {
+	    if ( ISMACRO(line, specLineTypes[i].text, specLineTypes[i].textLen) )

Cosmetics: there's an extra space in around those ISMACRO*() "calls", and missing space in the for-loop number comparison.

The name specLineType seems overly generic here, when these are branch-related directives all. I'd suggest walking over the for-loop with a pointer and returning that if found, that way you don't need to separately index specLineTypes a hundred times in both this and the next commit. If you return NULL on not-found, you don't actually need LINE_OTHER type at all.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/625#pullrequestreview-207334724
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20190225/26f6e788/attachment-0001.html>