Re: RFC: GConf Preference Backend for IcedTea

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Mario Torre wrote:
hehe, it seems like I'm going back in time with this :)

I've just finished to port the GConf backend to IcedTea, so sorry for
cross posting, but I think it's a good news for both Classpath and
IcedTea.

I'm asking for comment because I'm not a configure expert, so may very
well be that this is not the perfect way to handle things.

I've tested with few applications and all seems correct, the code is
pretty much the same as the Classpath version. I've not tested it as
concern the security model. I say this because I've seen that the
FileBasedPreferences in openjdk does a little more checks on the
SecurityManager than what we do. I want to check this carefully, but
other than that, I think it's ok as is.

This thing works that way, if the user issues a --disable-gconf-peer to
configure, nothing happens and the backend is not compiled. Otherwise,
the backend is compiled and a shared object is created and then copied
in the jre tree (much like we do with gcjwebplugin). Also, a patch is
applied to call this backend on Linux and Solaris.

I'm waiting for comments, thanks,

+	if test "x${ENABLE_GCONF_PEER}" = xyes; then \
+	  cp -af $(GCONF_PEER_LIB_DIR)/libgconfpeer.so \
+	    $(BUILD_OUTPUT_DIR)/j2sdk-image/jre/lib/$(INSTALL_ARCH_DIR) ; \
+	  cp -af $(GCONF_PEER_LIB_DIR)/libgconfpeer.so  \
+	    $(BUILD_OUTPUT_DIR)/j2re-image/lib/$(INSTALL_ARCH_DIR) ; \
+	fi

Makefile.am sections that test for ${ENABLE_GCONF_PEER} should use the
CREATE_GCONF_PEER_LIBRARIES conditional instead.

@@ -221,6 +240,12 @@ icedtea-debug: stamps/bootstrap-director
 	  $(BUILD_OUTPUT_DIR)-debug/j2sdk-image/jre/lib/$(INSTALL_ARCH_DIR)
 	cp -af gcjwebplugin.so \
 	  $(BUILD_OUTPUT_DIR)-debug/j2re-image/lib/$(INSTALL_ARCH_DIR)
+	if test "x${ENABLE_GCONF_PEER}" = xyes; then \
+	  cp -af $(GCONF_PEER_LIB_DIR)/libgconfpeer.so \
+	    $(BUILD_OUTPUT_DIR)/j2sdk-image/jre/lib/$(INSTALL_ARCH_DIR) ; \
+	  cp -af $(GCONF_PEER_LIB_DIR)/libgconfpeer.so \
+	    $(BUILD_OUTPUT_DIR)/j2re-image/lib/$(INSTALL_ARCH_DIR) ; \
+	fi

Copy-n-paste errors: you want $(BUILD_OUTPUT_DIR)-debug here.

+	if test "x${ENABLE_GCONF_PEER}" = xyes; then \
+          if [ ! -d "rt/gnu" ]; then \
+            mkdir -p rt/gnu ; \
+          fi ;\
+          cp -arf gconf-prefs-peer/gnu/* rt/gnu ;\
+        fi ; \

You could just commit the new Java source files under rt.

+AC_CONFIG_FILES([Makefile
+gconf-prefs-peer/Makefile
+gconf-prefs-peer/native/Makefile])

These files aren't included in the patch.  Can they be inlined in
the top-level Makefile.am?

+AC_ARG_ENABLE([gconf-peer],
...

You could just assume that GConf2 is installed in the system,
like we already do for similar dependencies.

+static void
+throw_exception_by_name (JNIEnv * env, const char *name, const char *msg);

The function name shouldn't be at the start of a line in a
declaration, only in a definition.

+ * Factory object that generates a Preferences nodes that are read from a GConf
+ * daemon.

Typo: "a Preferences nodes".

Some general comments:

In the future, try not to include generated files in patch
submissions.

I'd like the security situation cleared up before this patch goes
in.  For example, can untrusted applets read or write GConf2
preferences with the patch as-is?

Have you tested with the Swing GTK PLAF to ensure your use of the
GDK lock doesn't interfere?  Have you investigated retrieving the
default GConfBackendVTable and using its lock/unlock functions
instead?

Tom


[Index of Archives]     [Linux Kernel]     [Linux Cryptography]     [Fedora]     [Fedora Directory]     [Red Hat Development]

  Powered by Linux