On Feb 13, 10:38am, enami@sm.sony.co.jp (enami tsugutomo) wrote:
-- Subject: Re: CVS commit: basesrc/bin/sh

| Christos Zoulas <christos@netbsd.org> writes:
| 
| > cvs rdiff -r1.50 -r1.51 basesrc/bin/sh/parser.c
| 
| > Index: basesrc/bin/sh/parser.c
| > diff -u basesrc/bin/sh/parser.c:1.50 basesrc/bin/sh/parser.c:1.51
| > --- basesrc/bin/sh/parser.c:1.50	Tue Feb 12 08:39:10 2002
| > +++ basesrc/bin/sh/parser.c	Tue Feb 12 22:32:35 2002
| > @@ -877,6 +879,21 @@
| >  #define PARSEBACKQNEW()	{oldstyle = 0; goto parsebackq; parsebackq_newreturn:;}
| >  #define	PARSEARITH()	{goto parsearith; parsearith_return:;}
| >  
| > +#define ISDBLQUOTE() ((varnest < 32) ? (dblquote & (1 << varnest)) : \
| > +    (dblquotep[varnest / 32] & (1 << (varnest % 32))))
| 
| 
| >From this definition, dblquotep[0] isn't used even if maxnest is
| greater than 32, but ...
| 
| >  			varnest++;
| > +			if (varnest >= maxnest) {
| > +				maxnest += 32;
| > +				dblquotep = ckrealloc(dblquotep, maxnest / 8);
| > +				if (maxnest == 64)
| > +					*dblquotep = dblquote;
| 
| ...here, dblquotep[0] is initialized with dblquote.  It looks
| inconsistient.

Yes, but I wanted to keep the dblquotep array correct no matter if it is used
or not. It is a good programming practice to keep data consistent.

| > @@ -1082,6 +1115,8 @@
| >  	backquotelist = bqlist;
| >  	grabstackblock(len);
| >  	wordtext = out;
| > +	if (dblquotep != NULL)
| > +	    ckfree(dblquotep);
| >  	return lasttoken = TWORD;
| >  /* end of readtoken routine */
| >  
| 
| There are more return statements in this function.  Is it known to be
| safe without free'ing dblquotep there (it looks like one of them isn't
| compiled tho)?

No, we have to free it everywhere. The problem is actually worse: You can
leave the function without free-ing if the expansion is interrupted by a
signal. On the other hand, it is pretty uncommon to have > 32 nested double
quotes, so it is not a big deal. But if you are certain that the function
can return at other points, we need to add free's there too.

christos