Hi Costantino
+JNIEXPORT void * JNICALL
+JNU_GetEnv(JavaVM *vm, jint version);
+
+JNIEXPORT void JNICALL
+JNU_ThrowInternalError(JNIEnv *env, const char *msg);
+
+char * strrchr ( const char *, int );
You should probably include the real header files here, like string.h
and jni_util.h, instead of declaring those functions yourself.
+JNIEXPORT void JNICALL Java_sun_awt_X11_GtkFileDialogPeer_quit
+(JNIEnv * env, jobject jpeer)
+{
+ if (dialog != NULL)
+ {
+ fp_gtk_widget_hide (dialog);
+ fp_gtk_widget_destroy (dialog);
+
+ fp_gtk_main_quit ();
+ dialog = NULL;
+ }
+}
Should we put the GDK lock statements around those 3 functions? But if
we should, make sure they aren't taken recursively: when called from
Java the GDK lock isn't taken and we must, but when called from the
handle_response callback function, GTK already holds that lock and we
must not.
You've still got some C++ comments:
$ grep '//' b6913179.patch
+ // no filter, accept all.
+ // We do not implement this method because we
+ // have delegated to FileDialog#setDirectory
+ // We do not implement this method because we
+ // have delegated to FileDialog#setFile
+ // We do not implement this method because we
+ // have delegated to FileDialog#setFilenameFilter
+ // The current GtkFileChooser is available from GTK+ 2.4
+ // The current GtkFileChooser is available from GTK+ 2.4
+ // Init the thread system to use GLib in a thread-safe mode
+ //According the GTK documentation, gdk_threads_init() should be
+ //called before gtk_init() or gtk_init_check()
Also the patch doesn't even compile from me, fpulling the awt forest
to the latest and trying again...
But - lol - even the latest unpatched awt forest no longer compiles:
# Running javac:
/home/damjan/.local/opt/jdk6/bin/java -XX:-PrintVMOptions
-XX:+UnlockDiagnosticVMOptions -XX:-LogVMOutput -client -Xmx896m
-Xms128m -XX:PermSize=32m -XX:MaxPermSize=160m
-Xbootclasspath/p:/openjdk7/awt/build/linux-i586/langtools/dist/bootstrap/lib/javac.jar
-jar /openjdk7/awt/build/linux-i586/langtools/dist/bootstrap/lib/javac.jar
-source 7 -target 7 -encoding ascii
-Xbootclasspath:/openjdk7/awt/build/linux-i586/classes -sourcepath
/openjdk7/awt/build/linux-i586/gensrc:../../../../src/solaris/classes:../../../../src/share/classes
-d /openjdk7/awt/build/linux-i586/classes
@/openjdk7/awt/build/linux-i586/tmp/com/javax.swing.plaf/.classes.list.filtered
../../../../src/share/classes/javax/swing/plaf/nimbus/NimbusLookAndFeel.java:291:
cannot find symbol
defaults.clearOverridesCache(c);
^
symbol: method clearOverridesCache(JComponent)
location: class NimbusDefaults
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
1 error
make[5]: *** [.compile.classlist] Error 1
make[5]: Leaving directory `/openjdk7/awt/jdk/make/javax/swing/plaf'
make[4]: *** [build] Error 1
...
make: *** [build_product_image] Error 2
Uhm, Sun?
Otherwise good work Costantino, thank you and good luck
Damjan
On Wed, Mar 10, 2010 at 1:17 AM, Costantino Cerbo <c.cerbo at gmail.com> wrote:
> Hi Peter, Anthony and Damjan,
>
> here is a new patch (the 4th one!) with these improvements:
> * The copyright in GtkFileDialogPeer is now correctly dated.
> * ANSI C conformity 1: // comments transformed to /* */
> * ANSI C conformity 2: Variable declaration always at the beginning of a block
> * All compiler warnings removed
> * Error checking (in case of failure, JNU_ThrowInternalError(..) is used)
> * Multiple selection mode set only in an OPEN action (otherwise Gtk
> throws a warning when we are saving a file)
>
> As also Peter said, the job is almost done and I hope to see this
> patch soon in the OpenJDK!
>
> Best Regards,
> Costantino
>
> P.S.: About the indentation, I have no idea why it isn't right.
> I set it in my eclipse workspace according the Anthony's suggestions:
> Tab policy: Spaces only
> Indentation size: 4
> Tab site: 4
> Line width: 80
>
>
>
> 2010/3/2 Peter Zhelezniakov <Peter.Zhelezniakov at sun.com>:
>> Hi Costantino,
>>
>> Seems almost done! Changes to GTK code seem right thing to do indeed.
>>
>> I'd agree that we'd better stay away from any C++-isms such as // and
>> declaring variables in the middle of a block. This code is built on a
>> variety of platforms and we don't want to break older ones without a strong
>> reason.
>>
>> Thanks!
>> --
>> Peter | x33066 | location: SPB04 | timezone: GMT+03
>>
>