RFR-8008118 (original) (raw)

Martin Buchholz martinrb at google.com
Mon Mar 25 21:30:55 UTC 2013

Since we're all on this perfectionist quest for quality here, I noticed that we could replace allocation for path components with a single allocation using NUL as a separator. I think I'll go code that up.

Also note to all: if java VMs are created and destroyed without the process terminating, there is a small leak here (and in many other places in the JDK). There would be a huge amount of work if we ever wanted to get that right (especially if we supported concurrently active JVMs).

On Fri, Mar 22, 2013 at 7:38 AM, John Zavgren <john.zavgren at oracle.com>wrote:

Greetings:

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.) Thanks! John

http://cr.openjdk.java.net/~jzavgren/8008118/webrev.06/ ----- 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 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;

More information about the core-libs-dev mailing list