[cp-patches] Re: [gui] GdkPixbufDecoder JNIEnv fix proposal (PR/16824)

[Mark Wielaard]

Hi,

On Sun, 2004-08-08 at 19:17, Thomas Fitzsimmons wrote:
> On Sun, 2004-08-08 at 11:30, Mark Wielaard wrote:
> > 2004-08-08  Mark Wielaard  <address@hidden>
> > 
> >         * native/jni/gtk-peer/gnu_java_awt_peer_gtk_GdkPixbufDecoder.c:
> >         #include gdk.h, not gtk.h. #include jni.h, native_state.h, string.h
> >         and stdlib.h, not gtkpeer.h.
> >         (*vm): New static variable.
> >         (areaPreparedID): Make static.
> >         (areaUpdatedID): Likewise.
> >         (area_prepared): Get and use JNIEnv through stored JavaVM *vm.
> >         (area_prepared): Likewise.
> >         (area_updated): Likewise.
> >         (closed): Likewise.
> >         (initStaticState): Initialize *vm javaVM.
> >         (pumpBytes): Use given env, not global gdk_env.
[ ... ]
> > The only issue is that gcc 3+ complains loudly about the following:
> > 
> > ../../../../classpath/native/jni/gtk-peer/gnu_java_awt_peer_gtk_GdkPixbufDecoder.c:
> >  In function `area_prepared':
> > ../../../../classpath/native/jni/gtk-peer/gnu_java_awt_peer_gtk_GdkPixbufDecoder.c:86:
> >  warning: dereferencing type-punned pointer will break strict-aliasing rules
> > ../../../../classpath/native/jni/gtk-peer/gnu_java_awt_peer_gtk_GdkPixbufDecoder.c:
> >  In function `area_updated':
> > ../../../../classpath/native/jni/gtk-peer/gnu_java_awt_peer_gtk_GdkPixbufDecoder.c:124:
> >  warning: dereferencing type-punned pointer will break strict-aliasing rules
> > ../../../../classpath/native/jni/gtk-peer/gnu_java_awt_peer_gtk_GdkPixbufDecoder.c:
> >  In function `closed':
> > ../../../../classpath/native/jni/gtk-peer/gnu_java_awt_peer_gtk_GdkPixbufDecoder.c:167:
> >  warning: dereferencing type-punned pointer will break strict-aliasing rules
> > 
> > Does anybody know what the correct way to solve this is?
> 
> Yes, I found an explanation of this warning here:
> 
> http://lists.freebsd.org/pipermail/freebsd-current/2003-July/007561.html
> 
> Type-punning using a union is explicitly allowed by C99.  For example,
> see this patch to gthread-jni.c; it eliminates type-punned warnings on
> 64-bit architectures:
> 
> http://gcc.gnu.org/bugzilla/attachment.cgi?id=6843&action=view
> 
> > Any comments on the general idea of the patch?
>
> The general idea seems fine to me.

Thanks. I added that workaround and committed to GNU Classpath CVS and
libgcj gui branchs as attached.

Cheers,

Mark

Index: native/jni/gtk-peer/gnu_java_awt_peer_gtk_GdkPixbufDecoder.c
===================================================================
RCS file: 
/cvsroot/classpath/classpath/native/jni/gtk-peer/gnu_java_awt_peer_gtk_GdkPixbufDecoder.c,v
retrieving revision 1.5
diff -u -r1.5 gnu_java_awt_peer_gtk_GdkPixbufDecoder.c
--- native/jni/gtk-peer/gnu_java_awt_peer_gtk_GdkPixbufDecoder.c        26 Jun 
2004 16:07:03 -0000      1.5
+++ native/jni/gtk-peer/gnu_java_awt_peer_gtk_GdkPixbufDecoder.c        8 Aug 
2004 19:57:20 -0000
@@ -1,5 +1,5 @@
 /* gdkpixbufdecoder.c
-   Copyright (C) 1999, 2003 Free Software Foundation, Inc.
+   Copyright (C) 1999, 2003, 2004 Free Software Foundation, Inc.
 
    This file is part of GNU Classpath.
    
@@ -35,14 +35,17 @@
    obligated to do so.  If you do not wish to do so, delete this
    exception statement from your version. */
 
-
-#include <gtk/gtk.h>
+#include <gdk/gdk.h>
 #include <gdk-pixbuf/gdk-pixbuf.h>
 #include <gdk-pixbuf/gdk-pixbuf-loader.h>
 
-#include "gtkpeer.h"
+#include <jni.h>
+#include "native_state.h"
 #include "gnu_java_awt_peer_gtk_GdkPixbufDecoder.h"
 
+#include <string.h>
+#include <stdlib.h>
+
 struct state_table *native_pixbufdecoder_state_table;
 
 #define NSA_PB_INIT(env, clazz) \
@@ -57,14 +60,24 @@
 #define NSA_DEL_PB_PTR(env, obj) \
   remove_state_slot (env, obj, native_pixbufdecoder_state_table)
 
+/* Union used for type punning. */
+union env_union
+{
+  void **void_env;
+  JNIEnv **jni_env;
+};
+
+static JavaVM *vm;
 
-jmethodID areaPreparedID;
-jmethodID areaUpdatedID;
+static jmethodID areaPreparedID;
+static jmethodID areaUpdatedID;
 
 static void
 area_prepared (GdkPixbufLoader *loader, 
               jobject *decoder)
 {
+  JNIEnv *env;
+  union env_union e;
   jint width, height;
 
   GdkPixbuf *pixbuf = gdk_pixbuf_loader_get_pixbuf (loader);
@@ -78,11 +91,13 @@
 
   g_assert (decoder != NULL);
 
-  (*gdk_env)->CallVoidMethod (gdk_env,
-                             *decoder,
-                             areaPreparedID,
-                             width, height);
-
+  e.jni_env = &env;
+  (*vm)->GetEnv (vm, e.void_env, JNI_VERSION_1_1);
+  (*env)->CallVoidMethod (env,
+                         *decoder,
+                         areaPreparedID,
+                         width, height);
+  
   gdk_threads_enter ();
 }
 
@@ -92,6 +107,8 @@
              gint width, gint height,
              jobject *decoder)
 {
+  JNIEnv *env;
+  union env_union e;
   jint stride_bytes, stride_pixels, n_channels, n_pixels;
   int i, px;
   jintArray jpixels;  
@@ -114,8 +131,10 @@
   n_pixels = height * stride_pixels;
   gdk_pixels = gdk_pixbuf_get_pixels (pixbuf);
 
-  jpixels = (*gdk_env)->NewIntArray (gdk_env, n_pixels);
-  java_pixels = (*gdk_env)->GetIntArrayElements (gdk_env, jpixels, NULL);
+  e.jni_env = &env;
+  (*vm)->GetEnv (vm, e.void_env, JNI_VERSION_1_1);
+  jpixels = (*env)->NewIntArray (env, n_pixels);
+  java_pixels = (*env)->GetIntArrayElements (env, jpixels, NULL);
 
   memcpy (java_pixels, 
          gdk_pixels + (y * stride_bytes), 
@@ -141,22 +160,27 @@
 
   gdk_threads_leave ();
 
-  (*gdk_env)->ReleaseIntArrayElements (gdk_env, jpixels, java_pixels, 0);
-  (*gdk_env)->CallVoidMethod (gdk_env, 
-                             *decoder, 
-                             areaUpdatedID,
-                             (jint) x, (jint) y,
-                             (jint) width, (jint) height,
-                             jpixels,
-                             stride_pixels);
+  (*env)->ReleaseIntArrayElements (env, jpixels, java_pixels, 0);
+  (*env)->CallVoidMethod (env, 
+                         *decoder, 
+                         areaUpdatedID,
+                         (jint) x, (jint) y,
+                         (jint) width, (jint) height,
+                         jpixels,
+                         stride_pixels);
   gdk_threads_enter ();
 }
 
 static void
 closed (GdkPixbufLoader *loader __attribute__((unused)), jobject *decoder)
 {
+  JNIEnv *env;
+  union env_union e;
+  e.jni_env = &env;
+  (*vm)->GetEnv (vm, e.void_env, JNI_VERSION_1_1);
+
   gdk_threads_leave ();
-  (*gdk_env)->DeleteGlobalRef (gdk_env, *decoder); 
+  (*env)->DeleteGlobalRef (env, *decoder); 
   free (decoder);
   gdk_threads_enter ();
 }
@@ -187,6 +211,8 @@
 JNIEXPORT void JNICALL 
Java_gnu_java_awt_peer_gtk_GdkPixbufDecoder_initStaticState 
   (JNIEnv *env, jclass clazz)
 {
+  (*env)->GetJavaVM(env, &vm);
+
   areaPreparedID = (*env)->GetMethodID (env, clazz, 
                                        "areaPrepared", 
                                        "(II)V");
@@ -223,14 +249,14 @@
   if (len < 1)
     return;
 
-  bytes = (*gdk_env)->GetByteArrayElements (gdk_env, jarr, NULL);
+  bytes = (*env)->GetByteArrayElements (env, jarr, NULL);
   g_assert (bytes != NULL);
   loader = (GdkPixbufLoader *)NSA_GET_PB_PTR (env, obj);
   g_assert (loader != NULL);
 
   gdk_threads_enter ();
-  gdk_pixbuf_loader_write (loader, bytes, len, NULL);
+  gdk_pixbuf_loader_write (loader, (const guchar *) bytes, len, NULL);
   gdk_threads_leave ();
 
-  (*gdk_env)->ReleaseByteArrayElements (gdk_env, jarr, bytes, 0);
+  (*env)->ReleaseByteArrayElements (env, jarr, bytes, 0);
 }

signature.asc
Description: This is a digitally signed message part