Re: changes to make(1)

2002-10-31 Thread Ruslan Ermilov
On Thu, Oct 31, 2002 at 04:37:32AM -0500, AlanE wrote:
 On Thu, Oct 31, 2002 at 11:20:26AM +0200, Ruslan Ermilov wrote:
 On Thu, Oct 31, 2002 at 01:12:46AM -0500, AlanE wrote:
  On Wed, Oct 30, 2002 at 10:08:20PM -0800, David O'Brien wrote:
  On Thu, Oct 31, 2002 at 01:03:20AM -0500, AlanE wrote:
   There is also an expression parser bug that I would like to fix; it is
   a confusing error that causes make(1) to reject perfectly legitimate
   conditional expressions. We'll see about that one when I get to it; I
   suspect it should wait until after 5.0-RELEASE.
  
  If make release isn't broken, it should 100% wait until after
  5.0-RELEASE.
  
  I tend to agree here. No existing Makefiles are broken, so it can 
  wait. (Besides, I haven't even begun to investigate the problem...)
  
 What is this problem you're talking about?  I might have already
 looked at it recently.
 
 This code works correctly.
 
 .if ${PERL_MAJOR}  5 || ${PERL_MAJOR} == 5  ${PERL_MINOR}  8
 ${ECHO_CMD} This port might need the p5-File-Spec port.
 .endif # ${PERL_MAJOR}  5 || ${PERL_MAJOR} == 5  ${PERL_MINOR}  8
 
 This code, differing only in the use of ()'s to group the expressions,
 causes make(1) to think it is doing *string* comparisons, and then
 complain very loudly about only using == and != in said comparison.
 
 .if (${PERL_MAJOR}  5) || (${PERL_MAJOR} == 5  ${PERL_MINOR}  8)
 ${ECHO_CMD} This port might need the p5-File-Spec port.
 .endif # (${PERL_MAJOR}  5) || (${PERL_MAJOR} == 5  ${PERL_MINOR}  8)
 
Heh, I wondered about that too when I first these strange lines in
bsd.port.mk with spaces surronding parentheses -- it also works if
you insert spaces after digits before right parentheses:

.if (${PERL_MAJOR}  5 ) || (${PERL_MAJOR} == 5  ${PERL_MINOR}  8 )

Here is the fix:

%%%
Index: cond.c
===
RCS file: /home/ncvs/src/usr.bin/make/cond.c,v
retrieving revision 1.25
diff -u -p -r1.25 cond.c
--- cond.c  23 Oct 2002 23:16:42 -  1.25
+++ cond.c  31 Oct 2002 13:10:13 -
@@ -688,14 +688,15 @@ do_string_compare:
}
} else {
char *c = CondCvtArg(rhs, right);
-   if (*c != '\0'  !isspace((unsigned char) *c))
+   if (*c != '\0'  *c != ')' 
+   !isspace((unsigned char) *c))
goto do_string_compare;
if (rhs == condExpr) {
/*
 * Skip over the right-hand side
 */
while(!isspace((unsigned char) *condExpr) 
- (*condExpr != '\0')) {
+ *condExpr != ')'  *condExpr != '\0') {
condExpr++;
}
}
%%%

ports/devel/pmake is also vulnerable to this (even the latest beta),
and unless I'm dreadfully mistaken, OpenBSD's make(1) too.


Cheers,
-- 
Ruslan Ermilov  Sysadmin and DBA,
[EMAIL PROTECTED]   Sunbay Software AG,
[EMAIL PROTECTED]  FreeBSD committer,
+380.652.512.251Simferopol, Ukraine

http://www.FreeBSD.org  The Power To Serve
http://www.oracle.com   Enabling The Information Age



msg45776/pgp0.pgp
Description: PGP signature


Re: changes to make(1)

2002-10-31 Thread Ruslan Ermilov
On Thu, Oct 31, 2002 at 06:21:25AM -0800, Juli Mallett wrote:
 * De: Ruslan Ermilov [EMAIL PROTECTED] [ Data: 2002-10-31 ]
   [ Subjecte: Re: changes to make(1) ]
 
 Interesting.  It seems to me we really need a 'CondIsExpressionTerminator'
 or something.  ')' should only be a terminator if we are inside a '('. are
 both of those cases in '(' code?  If so then everything else is bogus, so
 I'd bet not :(
 
')' should either be escaped or quoted to be recognized properly here.

With or without my patch (make -r -dc):

STR=foo

.if ${STR} == foo)  == lhs = foo, rhs = foo, op = == (malformed conditional)
.if ${STR} == foo\) == lhs = foo, rhs = foo), op = ==
.if ${STR} == foo)== lhs = foo, rhs = foo), op = ==

You'll get the similar results with the NUM=1 and .if ${NUM} == 1
except here:

.if ${NUM} == 1)

old version will== lhs = 1, rhs = 1, op = ==
new version will== left = 1.00, right = 1.00, op = ==
(both will fail with malformed conditional)

If we go further and back out the change in rev. 1.9, we can
even make the following code work (no surrounding spaces for ):

NUM=1
num:
.if (${NUM}==1${NUM}==1)
@echo OK.
.endif

But this will cost us the following:

.if ${NUM}  0z
@echo OK.
.endif

make(1) won't now complain about z.  A small improvement gives us:

%%%
Index: cond.c
===
RCS file: /home/ncvs/src/usr.bin/make/cond.c,v
retrieving revision 1.25
diff -u -p -u -r1.25 cond.c
--- cond.c  23 Oct 2002 23:16:42 -  1.25
+++ cond.c  31 Oct 2002 15:26:16 -
@@ -688,16 +688,13 @@ do_string_compare:
}
} else {
char *c = CondCvtArg(rhs, right);
-   if (*c != '\0'  !isspace((unsigned char) *c))
+   if (c == rhs)
goto do_string_compare;
if (rhs == condExpr) {
/*
 * Skip over the right-hand side
 */
-   while(!isspace((unsigned char) *condExpr) 
- (*condExpr != '\0')) {
-   condExpr++;
-   }
+   condExpr = c;
}
}
 
%%%

  %%%
  Index: cond.c
  ===
  RCS file: /home/ncvs/src/usr.bin/make/cond.c,v
  retrieving revision 1.25
  diff -u -p -r1.25 cond.c
  --- cond.c  23 Oct 2002 23:16:42 -  1.25
  +++ cond.c  31 Oct 2002 13:10:13 -
  @@ -688,14 +688,15 @@ do_string_compare:
  }
  } else {
  char *c = CondCvtArg(rhs, right);
  -   if (*c != '\0'  !isspace((unsigned char) *c))
  +   if (*c != '\0'  *c != ')' 
  +   !isspace((unsigned char) *c))
  goto do_string_compare;
  if (rhs == condExpr) {
  /*
   * Skip over the right-hand side
   */
  while(!isspace((unsigned char) *condExpr) 
  - (*condExpr != '\0')) {
  + *condExpr != ')'  *condExpr != '\0') {
  condExpr++;
  }
  }
  %%%
  
  ports/devel/pmake is also vulnerable to this (even the latest beta),
  and unless I'm dreadfully mistaken, OpenBSD's make(1) too.


Cheers,
-- 
Ruslan Ermilov  Sysadmin and DBA,
[EMAIL PROTECTED]   Sunbay Software AG,
[EMAIL PROTECTED]  FreeBSD committer,
+380.652.512.251Simferopol, Ukraine

http://www.FreeBSD.org  The Power To Serve
http://www.oracle.com   Enabling The Information Age



msg45792/pgp0.pgp
Description: PGP signature