John Zavgren john.zavgren at oracle.com
Fri Mar 22 14:38:29 UTC 2013


I made modifications as per Martin's suggestions: 
1.) free pathv on "abort". 
2.) allocate memory for storing the "cwd" string, and then copy it into the memory location (to make sure that the contents of the pathv[] array don't refer to memory that's from the stack of the procedure that created it.) 


----- Original Message ----- 
From: martinrb at google.com 
To: christos at zoulas.com 
Cc: john.zavgren at oracle.com, core-libs-dev at openjdk.java.net 
Sent: Friday, March 22, 2013 10:19:24 AM GMT -05:00 US/Canada Eastern 
Subject: Re: RFR-8008118 

On Thu, Mar 21, 2013 at 12:11 PM, Christos Zoulas < christos at zoulas.com > wrote: 

On Mar 21, 11:36am, john.zavgren at 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 

+ 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). 


pathv[i] = cwd;

More information about the core-libs-dev mailing list