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 11:10:21 PDT 2010


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/
> >
> > -- 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/
> > >
> > > -- 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/
> > >
> > > -- 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/
> > >
> > > 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/5b1667b9/attachment.html 


More information about the hotspot-runtime-dev mailing list