[Rpm-maint] [rpm-software-management/rpm] Warn if %else is after %else (#649)

[Panu Matilainen]

pmatilai commented on this pull request.



> @@ -480,6 +480,13 @@ int readLine(rpmSpec spec, int strip)
 			ofi->fileName, ofi->lineNum);
 	    return PART_ERROR;
 	}
+	if ((spec->readStack->ifStage == LINE_ELSE)) {
+	    /* Got an else after %else ! */
+	    rpmlog(RPMLOG_ERR,
+			_("%s:%d: Got a %%else after %%else\n"),
+			ofi->fileName, ofi->lineNum);

The comment here is useless and redundant, the error message says the same thing, and the rpmlog() call is needlessly spread over three lines when it easily fits two. While those aren't actual errors,  along those two you also copy-pasted the grammar error from the preceding check ;) 

Something being in the codebase doesn't necessarily mean it's correct and good to copy around, especially in code this old (1998-2001). I'd rather see the superfluous comment removed and grammar error fixed in the older message too than add more.

Another issue is that the %if-%else-%endif sanity checking now looks even more ad-hoc because some things look at spec->redStack->next and others at spec->readStack->ifStage. I

-- 
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/649#pullrequestreview-224904957
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20190410/0f6d04fe/attachment.html>