From owner-freebsd-current@FreeBSD.ORG Tue Oct 12 02:51:30 2010 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 9649F106564A for ; Tue, 12 Oct 2010 02:51:30 +0000 (UTC) (envelope-from obrien@NUXI.org) Received: from dragon.nuxi.org (trang.nuxi.org [74.95.12.85]) by mx1.freebsd.org (Postfix) with ESMTP id 565128FC0C for ; Tue, 12 Oct 2010 02:51:30 +0000 (UTC) Received: from dragon.nuxi.org (obrien@localhost [127.0.0.1]) by dragon.nuxi.org (8.14.4/8.14.4) with ESMTP id o9C2JEjf000834 for ; Mon, 11 Oct 2010 19:19:14 -0700 (PDT) (envelope-from obrien@dragon.nuxi.org) Received: (from obrien@localhost) by dragon.nuxi.org (8.14.4/8.14.4/Submit) id o9C2JEfd000829 for freebsd-current@freebsd.org; Mon, 11 Oct 2010 19:19:14 -0700 (PDT) (envelope-from obrien) Date: Mon, 11 Oct 2010 19:19:14 -0700 From: "David O'Brien" To: freebsd-current@freebsd.org Message-ID: <20101012021914.GA72371@dragon.NUXI.org> Mail-Followup-To: obrien@freebsd.org, freebsd-current@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable X-Operating-System: FreeBSD 9.0-CURRENT X-to-the-FBI-CIA-and-NSA: HI! HOW YA DOIN? User-Agent: Mutt/1.5.16 (2007-06-09) Subject: [PATCH] fix shell bug in ${var%pattern} expansion X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: obrien@freebsd.org List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Oct 2010 02:51:30 -0000 At $WORK we hit a bug where ${var%/*} was not producing the correct expansion. There are two patches to fix this. I prefer the first as I feel it is cleaner from an API perspective. I've also added a regression testcase that shows the problem. Thoughts? --=20 -- David (obrien@FreeBSD.org) Commit log: Do not assume in growstackstr() that a "precious" character will be immediately written into the stack after the call. Instead let the caller manage the "space left". Currently growstackstr()'s assumption causes problems with STACKSTRNUL() where we want to be able to turn a stack into a C string, and later pretend the NUL is not there. This fixes a bug in STACKSTRNUL() (that grew the stack) where: 1. STADJUST() called after a STACKSTRNUL() results in an improper adjust. This can be seen in ${var%pattern} and ${var%%pattern} evaluation. 2. Memory leak in STPUTC() called after a STACKSTRNUL(). ----------%<----------%<----------%<----------%<----------%<---------- =20 Index: bin/sh/memalloc.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- bin/sh/memalloc.h (revision 213714) +++ bin/sh/memalloc.h (working copy) @@ -67,9 +67,16 @@ void ungrabstackstr(char *, char *); #define stackblock() stacknxt #define stackblocksize() stacknleft #define STARTSTACKSTR(p) p =3D stackblock(), sstrnleft =3D stackblocksize() -#define STPUTC(c, p) (--sstrnleft >=3D 0? (*p++ =3D (c)) : (p =3D growstac= kstr(), *p++ =3D (c))) +#define STPUTC(c, p) (--sstrnleft >=3D 0? (*p++ =3D (c)) : (p =3D growstac= kstr(), --sstrnleft, *p++ =3D (c))) #define CHECKSTRSPACE(n, p) { if (sstrnleft < n) p =3D makestrspace(); } #define USTPUTC(c, p) (--sstrnleft, *p++ =3D (c)) +/* + * STACKSTRNUL's use is where we want to be able to turn a stack + * (non-sentinel, character counting string) into a C string, + * and later pretend the NUL is not there. + * Note: Because of STACKSTRNUL's semantics, STACKSTRNUL cannot be used + * on a stack that will grabstackstr()ed. + */ #define STACKSTRNUL(p) (sstrnleft =3D=3D 0? (p =3D growstackstr(), *p =3D = '\0') : (*p =3D '\0')) #define STUNPUTC(p) (++sstrnleft, --p) #define STTOPC(p) p[-1] Index: bin/sh/histedit.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- bin/sh/histedit.c (revision 213714) +++ bin/sh/histedit.c (working copy) @@ -418,7 +418,7 @@ fc_replace(const char *s, char *p, char=20 } else STPUTC(*s++, dest); } - STACKSTRNUL(dest); + STPUTC('\0', dest); dest =3D grabstackstr(dest); =20 return (dest); Index: bin/sh/memalloc.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- bin/sh/memalloc.c (revision 213714) +++ bin/sh/memalloc.c (working copy) @@ -295,6 +295,12 @@ grabstackblock(int len) * is space for at least one character. */ =20 +static char * +growstrstackblock(int n) { + growstackblock(); + sstrnleft =3D stackblocksize() - n; + return stackblock() + n; +} =20 char * growstackstr(void) @@ -304,12 +310,10 @@ growstackstr(void) len =3D stackblocksize(); if (herefd >=3D 0 && len >=3D 1024) { xwrite(herefd, stackblock(), len); - sstrnleft =3D len - 1; + sstrnleft =3D len; return stackblock(); } - growstackblock(); - sstrnleft =3D stackblocksize() - len - 1; - return stackblock() + len; + return growstrstackblock(len); } =20 =20 @@ -323,9 +327,7 @@ makestrspace(void) int len; =20 len =3D stackblocksize() - sstrnleft; - growstackblock(); - sstrnleft =3D stackblocksize() - len; - return stackblock() + len; + return growstrstackblock(len); } =20 =20 ----------%<----------%<----------%<----------%<----------%<---------- ALTERNATE PATCH: Index: bin/sh-1/memalloc.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- bin/sh-1/memalloc.h (revision 213696) +++ bin/sh-1/memalloc.h (working copy) @@ -70,7 +70,17 @@ void ungrabstackstr(char *, char *); #define STPUTC(c, p) (--sstrnleft >=3D 0? (*p++ =3D (c)) : (p =3D growstac= kstr(), *p++ =3D (c))) #define CHECKSTRSPACE(n, p) { if (sstrnleft < n) p =3D makestrspace(); } #define USTPUTC(c, p) (--sstrnleft, *p++ =3D (c)) -#define STACKSTRNUL(p) (sstrnleft =3D=3D 0? (p =3D growstackstr(), *p =3D = '\0') : (*p =3D '\0')) +/* + * growstackstr() has the assumption that a "precious" character will be + * immediately written into it, so it sets up 'sstrnleft' appropriately. + * But that is not the case for STACKSTRNUL, where we want to be able to + * turn a stack (non-sentinel, character counting string) into a C string, + * and later pretend the NUL is not there (without adjusting 'sstrnleft'). + * Note: because of STACKSTRNUL's semantics, STACKSTRNUL cannot be used on + * a stack that will grabstackstr()ed. + */ +#define STACKSTRNUL(p) (sstrnleft =3D=3D 0? (p =3D growstackstr(), *p =3D = '\0', ++sstrnleft) : (*p =3D '\0')) +//#define STACKSTRNUL(p) (sstrnleft =3D=3D 0? (p =3D growstackstr(), *p = =3D '\0') : (*p =3D '\0')) #define STUNPUTC(p) (++sstrnleft, --p) #define STTOPC(p) p[-1] #define STADJUST(amount, p) (p +=3D (amount), sstrnleft -=3D (amount)) Index: bin/sh-1/histedit.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- bin/sh-1/histedit.c (revision 213696) +++ bin/sh-1/histedit.c (working copy) @@ -418,7 +418,8 @@ fc_replace(const char *s, char *p, char=20 } else STPUTC(*s++, dest); } - STACKSTRNUL(dest); + //STACKSTRNUL(dest); + STPUTC('\0', dest); dest =3D grabstackstr(dest); =20 return (dest); ----------%<----------%<----------%<----------%<----------%<---------- Index: tools/regression/bin/sh/expansion/trim4.0 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- tools/regression/bin/sh/expansion/trim4.0 (revision 0) +++ tools/regression/bin/sh/expansion/trim4.0 (revision 0) @@ -0,0 +1,12 @@ +# $FreeBSD$ + +J1=3D"/homes/SOME_USER" + +J2=3D"A5678901234567890123456789012345678901234567890123456789012345678901= 234567890123456789012345678911234567891123456789112345678911234567891123456= 789112345678911234567891123456789112345678912345678901234567890123456789012= 34567A567890123456789012345678901234567890123456789012345678901234567890123= 456789012345678901234567891123456789112345678911234567891123456789112345678= 911234567891123456789112345678911234567890123" + +J3=3D"C1234567890123456789012345678901234567890123456789012345678901234567= 890123456789012345678901234567890123456789012345678901234567890123456789012= 345678901234567890123456789012345678901234567890123456789012345678901234567= 890123456789012345678901234567890123456789012345678901234567890123456789012= 345678901234567890123456789012345678901234567890123456789012345678901234567= 890123456789012345678901234567890123456789012345678901234567890123456789012= 345678901234567890123456789012345678901234567890123456789012345678901234567= 890123456789012345678901234567890123456789012345678901234567890123456789012= 345678901234567890123456789012345678901234567890 HI" + +# Trigger bug in VSTRIMRIGHT processing STADJUST() call in expand.c:subeva= lvar() +J4=3D"${J2} ${J1%/*} $J3" + +echo $J4 Property changes on: tools/regression/bin/sh/expansion/trim4.0 ___________________________________________________________________ Added: svn:keywords + FreeBSD=3D%H Index: tools/regression/bin/sh/expansion/trim4.0.stdout =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- tools/regression/bin/sh/expansion/trim4.0.stdout (revision 0) +++ tools/regression/bin/sh/expansion/trim4.0.stdout (revision 0) @@ -0,0 +1 @@ +A5678901234567890123456789012345678901234567890123456789012345678901234567= 890123456789012345678911234567891123456789112345678911234567891123456789112= 34567891123456789112345678911234567891234567890123456789012345678901234567A= 567890123456789012345678901234567890123456789012345678901234567890123456789= 012345678901234567891123456789112345678911234567891123456789112345678911234= 567891123456789112345678911234567890123 /homes C123456789012345678901234567= 890123456789012345678901234567890123456789012345678901234567890123456789012= 345678901234567890123456789012345678901234567890123456789012345678901234567= 890123456789012345678901234567890123456789012345678901234567890123456789012= 345678901234567890123456789012345678901234567890123456789012345678901234567= 890123456789012345678901234567890123456789012345678901234567890123456789012= 345678901234567890123456789012345678901234567890123456789012345678901234567= 890123456789012345678901234567890123456789012345678901234567890123456789012= 345678901234567890123456789012345678901234567890123456789012345678901234567= 8901234567890 HI