Review request: 6378314: Bad warning message when agent library not found. local directory is not searched.
Chuck Rasbold
rasbold at google.com
Thu Aug 5 15:24:02 PDT 2010
Thanks, Coleen.
Agreed that vm_exit_during_initialization makes dead code out of the calls
to FREE_C_HEAP_ARRAY.
Just trying to be a good citizen in case that vm_exit call turns into
something else.
-- Chuck
On Thu, Aug 5, 2010 at 11:33 AM, Coleen Phillimore <
coleen.phillimore at oracle.com> wrote:
>
> Tom and Chuck, I can push it since it fixed my bug under kernel, it's the
> least I can do. And the code looks fine except vm_exit_during_initialization
> doesn't return so I don't think these things aren't actually freed. It
> doens't matter because vm_exit_during_initialization() currently exits.
> Thanks,
> Coleen
>
>
> On 08/05/10 14:10, Chuck Rasbold wrote:
>
> I haven't had any other volunteers yet.
>
> Thanks for making it happen.
>
> On Thu, Aug 5, 2010 at 10:36 AM, Tom Rodriguez <tom.rodriguez at oracle.com>wrote:
>
>> I hadn't considered that. That seems fine then. If no one else has
>> volunteered to push this, I will take care of it.
>>
>> tom
>>
>> On Aug 5, 2010, at 8:01 AM, Chuck Rasbold wrote:
>>
>> > I think it is too soon in VM startup to use resource allocation. I've
>> switched to C_HEAP allocation, instead.
>> >
>> > I applied changes to the KERNEL chunk of code to make it consistent. I
>> can't/won't test that code. Let me know if you'd rather me leave it alone.
>> >
>> > Webrev updated.
>> >
>> > http://cr.openjdk.java.net/~rasbold/6378314/<http://cr.openjdk.java.net/%7Erasbold/6378314/>
>> >
>> > -- Chuck
>> >
>> > On Wed, Aug 4, 2010 at 4:28 PM, Tom Rodriguez <tom.rodriguez at oracle.com>
>> wrote:
>> > I think simply allocating using NEW_RESOURCE_ARRAY(char, len) will be
>> fine/better. The use of AllocateHeap in the ifdef KERNEL piece is kind of
>> odd to me. Maybe they are being paranoid about what might happen in
>> fork_and_exec. Actually there's a minor bug in that code because of the use
>> of AllocateHeap.
>> >
>> > FreeHeap(cmd);
>> > if (status == -1) {
>> > warning(cmd);
>> >
>> > tom
>> >
>> > On Aug 4, 2010, at 3:01 PM, Chuck Rasbold wrote:
>> >
>> > > Internally, we did discuss the "char buf[len]" construct, but forgot
>> about the SunStudio compilers. Sorry about that.
>> > >
>> > > I've changed the new code to use AllocateHeap() and FreeHeap(), as
>> does the currently existing code within the #ifdef KERNEL.
>> > >
>> > > And then I put a fix and a clean-up within that #ifdef, so the code is
>> more consistent throughout.
>> > >
>> > > http://cr.openjdk.java.net/~rasbold/6378314/<http://cr.openjdk.java.net/%7Erasbold/6378314/>
>> > >
>> > > -- Chuck
>> > >
>> > >
>> > > On Wed, Aug 4, 2010 at 1:28 PM, Paul Hohensee <
>> paul.hohensee at oracle.com> wrote:
>> > > No 'new' and 'delete' in the hotspot source code please. Use
>> > > NEW_RESOURCE_ARRAY and FREE_RESOURCE_ARRAY.
>> > >
>> > > Paul
>> > >
>> > >
>> > > On 8/4/10 4:15 PM, Kelly O'Hair wrote:
>> > >
>> > > On Aug 4, 2010, at 11:43 AM, Tom Rodriguez wrote:
>> > >
>> > > The SunStudio compiler doesn't like this construct:
>> > >
>> > > + size_t len = strlen(msg) + strlen(name) + strlen(sub_msg) +
>> strlen(ebuf) + 1;
>> > > + char buf[len];
>> > >
>> > >
>> > > I wondered about that.
>> > >
>> > > so I think you'll have to use another fixed size buffer.
>> > >
>> > > or char * buf = new char[len];
>> > >
>> > > and a delete [] buf
>> > > ???
>> > >
>> > > -kto
>> > >
>> > >
>> > > tom
>> > >
>> > > On Aug 4, 2010, at 11:20 AM, Chuck Rasbold wrote:
>> > >
>> > > Thanks, John.
>> > >
>> > > The nits have been fixed and the webrev updated.
>> > >
>> > > http://cr.openjdk.java.net/~rasbold/6378314/<http://cr.openjdk.java.net/%7Erasbold/6378314/>
>> > >
>> > > -- Chuck
>> > >
>> > > On Tue, Aug 3, 2010 at 4:36 PM, John Coomes <John.Coomes at oracle.com>
>> wrote:
>> > > Chuck Rasbold (rasbold at google.com) wrote:
>> > > http://cr.openjdk.java.net/~rasbold/6378314/<http://cr.openjdk.java.net/%7Erasbold/6378314/>
>> > >
>> > > Fixed: 6378314: Bad warning message when agent library not found.
>> local
>> > > directory is not searched.
>> > > Contributed-by: jeremymanson at google.com
>> > >
>> > > Print a more detailed error message for agent library load failure.
>> > >
>> > > Hi Chuck & Jeremy,
>> > >
>> > > Looks ok to me, aside from a couple of nits: use jio_snprintf instead
>> > > of sprintf, and strlen returns a size_t, so use that to declare len.
>> > >
>> > > -John
>> > >
>> > >
>> > >
>> > >
>> > >
>> >
>> >
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20100805/8dc7286a/attachment.html
More information about the hotspot-runtime-dev
mailing list