On Thu, Mar 21, 2013 at 12:11 PM, Christos Zoulas <chris...@zoulas.com>wrote:
> On Mar 21, 11:36am, john.zavg...@oracle.com (John Zavgren) wrote: > -- Subject: Re: RFR-8008118 > > | All: > | > | How does this look? > | 1.) I reverted the for statement formatting change. > > Not exactly. Now it is indented incorrectly. > > Agreed. Really revert. > | 2.) I removed the goto statement and "inlined" some code instead. > > I prefer to write simpler code that works with both signed and unsigned > indexes: > > + while (i > 0) > + if (pathv[--i] != cwd) > + free(pathv[i]); > + > > Christos' suggestion looks pretty good. As Mark noted, need to add missing free of pathv itself. > | 3.) I checked to make sure that we're not freeing memory that we didn't > act= > | ually allocate. (Path vector elements that are empty.) > > You are still using the "./" string literal constant in the code so you'll > end up freeing it (the compiler will prolly merge the two instances and > you'll get lucky but it is semantically incorrect). > Agreed, pathv[i] = cwd;
