Re: jvmti patch for mingw32

Re: jvmti patch for mingw32

[David Daney]

Patches go to java-patches@gcc.gnu.org

David Daney.


Marco Trudel wrote:
> Building for mingw32 is currently broken at multiple places. This is the 
> first patch. I'll see if I can fix the others as well...
> 
> Ok? I have no commiting rights...
> 
> 
> Marco
> 
> 
> ------------------------------------------------------------------------
> 
> --- jvmti.cc	2006-09-22 21:14:53.000000000 +0200
> +++ jvmti.cc	2006-09-22 21:13:24.000000000 +0200
> @@ -966,7 +966,7 @@
>    return JVMTI_ERROR_NONE;
>  }
>  
> -jvmtiError
> +jvmtiError JNICALL
>  _Jv_JVMTI_GetErrorName (MAYBE_UNUSED jvmtiEnv *env, jvmtiError error,
>  			char **name_ptr)
>  {

Re: jvmti patch for mingw32

[Mohan Embar]

Hi Marco,

> Building for mingw32 is currently broken at multiple places. This is the 
> first patch. I'll see if I can fix the others as well...

Are the other broken places similar missing JNICALL declarations
or other issues? If they are similar missing declarations, can you
combine them into one patch? Thanks for looking into this. If you
create a proper ChangeLog, I'll check this in if no one else does.

-- Mohan
http://www.thisiscool.com/
http://www.animalsong.org/

Re: jvmti patch for mingw32

[Marco Trudel]

[-- Attachment #1: Type: text/plain, Size: 884 bytes --]

Mohan Embar wrote:
> Hi Marco,
> 
>> Building for mingw32 is currently broken at multiple places. This is the 
>> first patch. I'll see if I can fix the others as well...
> 
> Are the other broken places similar missing JNICALL declarations
> or other issues? If they are similar missing declarations, can you
> combine them into one patch?

It's the only missing JNICALL. The second problem is that

natInetAddress.cc uses checkConnect():
    java::lang::SecurityException *ex = checkConnect (host);
What gives:
    error: 'checkConnect' was not declared in this scope

But I'm unable to find the defining header... I sent a mail to Gary 
Benson. He desperately searched a win32 tester once he checked that in.

The rest compiled flawless...


> Thanks for looking into this. If you
> create a proper ChangeLog, I'll check this in if no one else does.

Attached... Thanks...


Marco

[-- Attachment #2: changelog-patch.txt --]
[-- Type: text/plain, Size: 346 bytes --]

--- ChangeLog	2006-09-22 22:40:49.000000000 +0200
+++ ChangeLog	2006-09-22 22:40:18.000000000 +0200
@@ -1,3 +1,7 @@
+2006-09-22 Marco Trudel <mtrudel@gmx.ch>
+
+	* jvmti.cc (_Jv_JVMTI_GetErrorName): Added missing "JNICALL" specifier
+
 2006-09-21  Keith Seitz  <keiths@redhat.com>
 
 	* jvmti.cc (_Jv_JVMTI_DisposeEnvironment): Check for enabled

Re: jvmti patch for mingw32

[Tom Tromey]

>>>>> "Mohan" == Mohan Embar <gnustuff@thisiscool.com> writes:

>> Building for mingw32 is currently broken at multiple places. This is the 
>> first patch. I'll see if I can fix the others as well...

Mohan> Are the other broken places similar missing JNICALL declarations
Mohan> or other issues? If they are similar missing declarations, can you
Mohan> combine them into one patch? Thanks for looking into this. If you
Mohan> create a proper ChangeLog, I'll check this in if no one else does.

I'm doing a build here with that patch in place.  I also made that
function 'static'.  I'll commit when the build is done.

Tom

Re: jvmti patch for mingw32

[Tom Tromey]

>>>>> "Marco" == Marco Trudel <mtrudel@gmx.ch> writes:

Marco> natInetAddress.cc uses checkConnect():
Marco>     java::lang::SecurityException *ex = checkConnect (host);
Marco> What gives:
Marco>     error: 'checkConnect' was not declared in this scope

Marco> But I'm unable to find the defining header... I sent a mail to Gary
Marco> Benson. He desperately searched a win32 tester once he checked that in.

The code is in java/net/natInetAddressWin32.cc.
We make a symlink named natInetAddress.cc during configure.

The headers you want are generated at build time from the .class
files.  So look in your build tree for InetAddress.h, etc.

Tom

Re: jvmti patch for mingw32

[Mohan Embar]

Hi Marco,

I don't mean to be annoying, but could you submit the
patch exactly like I've done here?:

http://gcc.gnu.org/ml/java-patches/2006-q3/msg00194.html

- put the patch inline in the body of the email message and not
  as an attachment
- the ChangeLog entry shouldn't be in diff format
- the jvmti.cc should be generated via "svn diff jvmti.cc"
  when you're in the libjava directory. (I'm assuming you've
  been working with svn a lot.)

The reason I'm being so annoying is so that you learn to
do this properly if you submit other patches in the future.

Also, I'm assuming this patch is okay to apply on your behalf
because I applied a similar patch under the "obvious" rule.
For more substantial patches, the GCC Higher Powers are
going to tell you that you need copyright assignment papers:

http://www.gnu.org/licenses/why-assign.html

-- Mohan
http://www.thisiscool.com/
http://www.animalsong.org/

Re: jvmti patch for mingw32

[Mohan Embar]

Hi Tom,

>Mohan> Are the other broken places similar missing JNICALL declarations
>Mohan> or other issues? If they are similar missing declarations, can you
>Mohan> combine them into one patch? Thanks for looking into this. If you
>Mohan> create a proper ChangeLog, I'll check this in if no one else does.

>I'm doing a build here with that patch in place.  I also made that
>function 'static'.  I'll commit when the build is done.

Thanks for this.

Marco: just for fun, if you want to redo the patch like I said
and either send it to me personally or post it to this list, I
or others can review it for correct form. It would be nice to have
more helping hands here....

-- Mohan
http://www.thisiscool.com/
http://www.animalsong.org/

Re: jvmti patch for mingw32

[Tom Tromey]

>>>>> "Mohan" == Mohan Embar <gnustuff@thisiscool.com> writes:

Mohan> - put the patch inline in the body of the email message and not
Mohan>   as an attachment

Either way is ok by gcc standards, fwiw.

Mohan> Also, I'm assuming this patch is okay to apply on your behalf
Mohan> because I applied a similar patch under the "obvious" rule.

Yeah, this one is ok.  But I already checked it in :)

Mohan> For more substantial patches, the GCC Higher Powers are
Mohan> going to tell you that you need copyright assignment papers:
Mohan> http://www.gnu.org/licenses/why-assign.html

Yup.  Marco, send me email and I can get you started on the process.
It is pretty easy.

Tom

Re: jvmti patch for mingw32

[Mohan Embar]

>Mohan> - put the patch inline in the body of the email message and not
>Mohan>   as an attachment
>
>Either way is ok by gcc standards, fwiw.

I thought I remember someone complaining about
this but maybe I was just imagining it. Nice to know.

-- Mohan
http://www.thisiscool.com/
http://www.animalsong.org/

Re: jvmti patch for mingw32

[David Daney]

Mohan Embar wrote:
>>Mohan> - put the patch inline in the body of the email message and not
>>Mohan>   as an attachment
>>
>>Either way is ok by gcc standards, fwiw.
> 
> 
> I thought I remember someone complaining about
> this but maybe I was just imagining it. Nice to know.
> 

The important thing is that the patch does not get mangled.  If you use 
a mail client that does not mangle things, then in-line is fine. 
However if you use something like Thunderbird, your only easy option is 
to attach the patches.

David Daney

Re: jvmti patch for mingw32

[Tom Tromey]

>>>>> "David" == David Daney <ddaney@avtrex.com> writes:

>>> Either way is ok by gcc standards, fwiw.
>> I thought I remember someone complaining about
>> this but maybe I was just imagining it. Nice to know.

David> The important thing is that the patch does not get mangled.

Yeah.

The official policy is here:

http://gcc.gnu.org/contribute.html#patches

Tom

Re: jvmti patch for mingw32

[Marco Trudel]

Tom Tromey wrote:
>>>>>> "Marco" == Marco Trudel <mtrudel@gmx.ch> writes:
> 
> Marco> natInetAddress.cc uses checkConnect():
> Marco>     java::lang::SecurityException *ex = checkConnect (host);
> Marco> What gives:
> Marco>     error: 'checkConnect' was not declared in this scope
> 
> Marco> But I'm unable to find the defining header... I sent a mail to Gary
> Marco> Benson. He desperately searched a win32 tester once he checked that in.
> 
> The code is in java/net/natInetAddressWin32.cc.
> We make a symlink named natInetAddress.cc during configure.
> 
> The headers you want are generated at build time from the .class
> files.  So look in your build tree for InetAddress.h, etc.


Actually I checked all header files in the gcc build and gcc source dir. 
The only header file defining checkConnect() is 
java/lang/SecurityManager.h and the signature is wrong.
So either I'm blind, used the search tool wrong, or there's more than a 
missing header... I'll wait what Gary says about it...


Marco

Re: jvmti patch for mingw32

[Marco Trudel]

Mohan Embar wrote:
> Hi Marco,
> 
> I don't mean to be annoying, but could you submit the
> patch exactly like I've done here?:

Actually you're not annoying. I'm glad you take the time to tell me 
these things. At last, I'm not here to make things more complicated than 
they have to be...


> - put the patch inline in the body of the email message and not
>   as an attachment

Following the later messages, I will stay with attached patches. I use 
thunderbird :-( Actually I attached them because I once tried to use an 
inlined patch from the list, but it just didn't work because Thunderbird 
broke it or I copied it wrong...


> - the ChangeLog entry shouldn't be in diff format

But you meant the ChangeLog file in the dir? Or did you mean the log for 
committing into subversion?
So I guess you just expected a line of text describing the changes?


> - the jvmti.cc should be generated via "svn diff jvmti.cc"
>   when you're in the libjava directory. (I'm assuming you've
>   been working with svn a lot.)

That will work...
Is the svn diff using the same style as "diff -Naur"?


> The reason I'm being so annoying is so that you learn to
> do this properly if you submit other patches in the future.

That's ok...


Marco

Re: jvmti patch for mingw32

[Marco Trudel]

Mohan Embar wrote:
> Hi Tom,
> 
>> Mohan> Are the other broken places similar missing JNICALL declarations
>> Mohan> or other issues? If they are similar missing declarations, can you
>> Mohan> combine them into one patch? Thanks for looking into this. If you
>> Mohan> create a proper ChangeLog, I'll check this in if no one else does.
> 
>> I'm doing a build here with that patch in place.  I also made that
>> function 'static'.  I'll commit when the build is done.
> 
> Thanks for this.
> 
> Marco: just for fun, if you want to redo the patch like I said
> and either send it to me personally or post it to this list, I
> or others can review it for correct form.

I think that won't be necessary. I will keep your comments as further 
references and ask on the next patch if everything is fine.


thanks
Marco

Re: jvmti patch for mingw32

[Mohan Embar]

Hi Marco,

Keep in mind that I know less about this stuff than
most here. I'll attempt to answer these - others: jump
in if I say something incorrect or that can be clarified.

>> - the ChangeLog entry shouldn't be in diff format
>
>But you meant the ChangeLog file in the dir? Or did you mean the log for 
>committing into subversion?

The ChangeLog file in the directory. The procedure for
checking in a change tells you that you're not supposed
to use "patch" to update a ChangeLog because things might
have changed between the time you've prepared the patch and
the time you're committing it. You're supposed to update
the ChangeLog directly before committing the patch. As for
the SVN log entry, I simply cut and paste that from the
ChangeLog:

http://gcc.gnu.org/svnwrite.html

>Is the svn diff using the same style as "diff -Naur"?

Not sure. I just learn the minimal of what works for me. :)

Looking forward to seeing you more involved in this.
It's the Next Step from simply building the compiler.

-- Mohan
http://www.thisiscool.com/
http://www.animalsong.org/

Re: jvmti patch for mingw32

[Tom Tromey]

>>>>> "Marco" == Marco Trudel <mtrudel@gmx.ch> writes:

Marco> Actually I checked all header files in the gcc build and gcc source
Marco> dir. The only header file defining checkConnect() is
Marco> java/lang/SecurityManager.h and the signature is wrong.
Marco> So either I'm blind, used the search tool wrong, or there's more than
Marco> a missing header... I'll wait what Gary says about it...

Ok, I see what you meant now.  Probably what happened is that
checkConnect was removed by some earlier patch.  I suggest reading
Gary's last few commits in this area (either with viewcvs or in the
java-patches archive) to see what the code looked like previously;
probably whatever changes were applied to the posix port can be
brought over to the win32 port fairly easily.

Tom