Re: Patch: PR 16923

[Bryce McKinlay <mckinlay at redhat dot com>]

Thomas Fitzsimmons wrote:

> I've attached an updated patch for this.  There was a problem with the
> last patch that broke gij's -D handling: _Jv_Compiler_Properties was
> initialized to NULL at the start of _Jv_CreateJavaVM.  I've added a new
> variable _Jv_Properties_Count so that _Jv_Compiler_Properties can always
> be allocated dynamically.  "make check" didn't catch the gij -D
> breakage; we should probably add a test for this.
>
> I tested this new patch with "make check" -- it introduces one new pass
> and no new failures.
>
> Tom
>
> 2005-02-20  Thomas Fitzsimmons  <fitzsim@redhat.com>
>
> 	* gij.cc (main): Remove last_D_option.  Dynamically allocate
> 	_Jv_Compiler_Properties.
> 	* jni.cc (JNI_CreateJavaVM): Pass args to _Jv_CreateJavaVM.  Move
> 	argument parsing code ...
> 	* prims.cc (_Jv_CreateJavaVM): ... here.
> 	(_Jv_Compiler_Properties): Initialize to NULL.
> 	(_Jv_Properties_Count): Initialize to 0.
> 	* include/java-props.h (_Jv_Properties_Count): Declare.
> 	* java/lang/natRuntime.cc (insertSystemProperties): Use
> 	_Jv_Properties_Count in for loop exit condition.
> 	* testsuite/libjava.jni/jni.exp
> 	(gcj_invocation_compile_c_to_binary): New procedure.
> 	(gcj_invocation_test_one): Likewise.
> 	(gcj_jni_run): Run JNI invocation API tests.
> 	* testsuite/libjava.jni/invocation/PR16923.c,
> 	testsuite/libjava.jni/invocation/PR16923.java,
> 	testsuite/libjava.jni/invocation/PR16923.out: New test.
>
>  
>
> ------------------------------------------------------------------------
>
> Index: gij.cc
> ===================================================================
> RCS file: /cvs/gcc/gcc/libjava/gij.cc,v
> retrieving revision 1.25
> diff -u -r1.25 gij.cc
> --- gij.cc	18 Feb 2005 20:52:14 -0000	1.25
> +++ gij.cc	20 Feb 2005 07:52:42 -0000
> @@ -57,7 +57,6 @@
> {
>   /* We rearrange ARGV so that all the -D options appear near the
>      beginning.  */
> -  int last_D_option = 0;
>   bool jar_mode = false;
>
>   int i;
> @@ -77,7 +76,12 @@
>
>       if (! strncmp (arg, "-D", 2))
> 	{
> -	  argv[last_D_option++] = arg + 2;
> +	  _Jv_Compiler_Properties = (const char**) realloc
> +	    (_Jv_Compiler_Properties,
> +	     (_Jv_Properties_Count + 1) * sizeof (char*));
> +
> +	  _Jv_Compiler_Properties[_Jv_Properties_Count++] = strdup (arg + 2);
> +
> 	  continue;
> 	}
>  
Shouldn't arguments like -D and -Xms etc be processed by 
_Jv_CreateJavaVM() as well? It makes sense to allow things like heap 
sizes to be set via the invocation API. I'm not sure about --help and 
--version.

> Index: prims.cc
> ===================================================================
> RCS file: /cvs/gcc/gcc/libjava/prims.cc,v
> retrieving revision 1.103
> diff -u -r1.103 prims.cc
> --- prims.cc	16 Feb 2005 04:16:06 -0000	1.103
> +++ prims.cc	20 Feb 2005 07:52:44 -0000
> @@ -80,10 +80,9 @@
> // functions are changed to take a size_t argument instead of jint.
> #define MAX_OBJECT_SIZE ((1<<31) - 1)
>
> -static const char *no_properties[] = { NULL };
> -
> // Properties set at compile time.
> -const char **_Jv_Compiler_Properties = no_properties;
> +const char **_Jv_Compiler_Properties = NULL;
> +int _Jv_Properties_Count = 0;
>
> // The JAR file to add to the beginning of java.class.path.
> const char *_Jv_Jar_Class_Path;
> @@ -910,15 +909,78 @@
> }
>
> jint
> -_Jv_CreateJavaVM (void* /*vm_args*/)
> +_Jv_CreateJavaVM (void* vm_args)
> {
>   using namespace gcj;
> -  
> +
>   if (runtimeInitialized)
>     return -1;
>
>   runtimeInitialized = true;
>
> +  // Parse the arguments.
> +  if (vm_args != NULL)
> +    {
>  

It'd be cleaner to move the argument processing code out into its own 
static function, ie _Jv_ProcessVMArguments() ? This way 
_Jv_CreateJavaVM() is kept fairly clean, which is helpful when tracing 
through the VM startup sequence, etc.

> +      jint version = * (jint *) vm_args;
> +      // We only support 1.2 and 1.4.
> +      if (version != JNI_VERSION_1_2 && version != JNI_VERSION_1_4)
> +	return JNI_EVERSION;
>  

Since _Jv_CreateJavaVM is also called from CNI, we need to either expose 
the JavaVMInitArgs structure in the CNI headers so that it is visible 
from CNI as well, or perhaps have the CNI _Jv_CreateJavaVM translate the 
args to the JNI format before calling _Jv_CreateJavaVM (or vice-versa)

Bryce