martinrb at google.com
Wed Mar 27 01:13:33 UTC 2013
does not appear to be public - could we fix that?
Here is my alternative set of perfectionist changes:
# Cleanup declaration of "environ"
# Fix PATH handling
that could be rolled into one.
I adopted Christos' suggestion of using
This version only does two allocations.
On Mon, Mar 25, 2013 at 2:30 PM, Martin Buchholz <martinrb at google.com>wrote:
> 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:
>> 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
>> ----- 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
>>> 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