Review request: 6378314: Bad warning message when agent library not found. local directory is not searched.

[Coleen Phillimore]

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 <mailto: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 <mailto: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 <mailto: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 <mailto:John.Coomes at oracle.com>> wrote:
>     > > Chuck Rasbold (rasbold at google.com <mailto: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
>     <mailto: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/badc7d9c/attachment-0001.html