[RFC]: make IcedTea-web to be compatible with RHEL5 libraries
Dr Andrew John Hughes
ahughes at redhat.com
Tue Aug 30 20:02:21 PDT 2011
On 13:03 Mon 29 Aug , Jiri Vanek wrote:
snip...
> >
> > This would be a cleaner patch if the test was inverted i.e. #ifndef LEGACY_GLIB
>
> Although I do not see the point why it is cleaner - done.
The point was that you wouldn't be changing the original code, just adding #ifndef etc.
around it, but the point is negated now the function is separated out anyway.
snip...
>
> The patch have shrunk a lot, and now I'm not absolutely sure that I did not broke something. My intentions for this were to touch the code I do not understand full as seldom as possible, but due to reviewing issues i made much more changes.
> Please note that I have touched and reused $DEFS, and moved block of code in IcedTeaNPPlugin.cc into separate method. Although I have tested those binaries on rhel5,6 and f13 and everything seems to be working, I'm really not 100% sure.
>
Better to do it right IMHO.
Comments below.
> Best Regards
> J.
> diff -r 36270c76a533 Makefile.am
> --- a/Makefile.am Wed Aug 24 15:17:46 2011 -0400
> +++ b/Makefile.am Mon Aug 29 12:43:24 2011 +0200
> @@ -210,6 +210,7 @@
> mkdir -p $(PLUGIN_DIR) && \
> cd $(PLUGIN_DIR) && \
> $(CXX) $(CXXFLAGS) \
> + $(DEFS) \
> -DJDK_UPDATE_VERSION="\"$(JDK_UPDATE_VERSION)\"" \
> -DPLUGIN_NAME="\"IcedTea-Web Plugin\"" \
> -DPLUGIN_VERSION="\"$(PLUGIN_VERSION)\"" \
> diff -r 36270c76a533 acinclude.m4
> --- a/acinclude.m4 Wed Aug 24 15:17:46 2011 -0400
> +++ b/acinclude.m4 Mon Aug 29 12:43:24 2011 +0200
> @@ -484,6 +484,15 @@
> AC_SUBST(PKGVERSION)
> ])
>
> +AC_DEFUN_ONCE([IT_GET_GLIBVERSION],
> +[
> +PKG_CHECK_MODULES([GLIB2_V_216],[glib-2.0 >= 2.16],
> +[],[
> +AC_DEFINE([GLIB214])
> +])
> +]
> +)
> +
You can merge this all into much less space now:
AC_DEFUN_ONCE([IT_GET_GLIBVERSION], [
PKG_CHECK_MODULES([GLIB2_V_216],[glib-2.0 >= 2.16],[],[AC_DEFINE([GLIB214])])
])
I think IT_CHECK_GLIB_VERSION would be a more accurate name.
Also, the name GLIB214 makes no sense. As mentioned before, LEGACY_GLIB would be better.
> AC_DEFUN([IT_CHECK_WITH_GCJ],
> [
> AC_MSG_CHECKING([whether to compile ecj natively])
> diff -r 36270c76a533 configure.ac
> --- a/configure.ac Wed Aug 24 15:17:46 2011 -0400
> +++ b/configure.ac Mon Aug 29 12:43:24 2011 +0200
> @@ -80,6 +80,7 @@
> IT_CHECK_FOR_CLASS(COM_SUN_JNDI_TOOLKIT_URL_URLUTIL, [com.sun.jndi.toolkit.url.UrlUtil])
> IT_CHECK_FOR_CLASS(SUN_APPLET_APPLETIMAGEREF, [sun.applet.AppletImageRef])
> IT_CHECK_FOR_APPLETVIEWERPANEL_HOLE
> +IT_GET_GLIBVERSION
>
> #
> # Find optional depedencies
> diff -r 36270c76a533 plugin/icedteanp/IcedTeaNPPlugin.cc
> --- a/plugin/icedteanp/IcedTeaNPPlugin.cc Wed Aug 24 15:17:46 2011 -0400
> +++ b/plugin/icedteanp/IcedTeaNPPlugin.cc Mon Aug 29 12:43:24 2011 +0200
> @@ -869,6 +869,31 @@
> PLUGIN_DEBUG ("ITNP_URLNotify return\n");
> }
>
> +#ifdef GLIB214
> +// Returns key from first item stored in hashtable
> +gboolean
> +find_first_item_in_hash_table(gpointer key, gpointer value, gpointer user_data)
> +{
> + user_data = key;
> + return (gboolean)TRUE;
> +}
> +#endif
> +
> +#if MOZILLA_VERSION_COLLAPSED >= 1090100
> +gpointer getFirtsTableInstance()
> +{
> + gpointer id, instance;
> + #ifndef GLIB214
> + GHashTableIter iter;
> + g_hash_table_iter_init (&iter, instance_to_id_map);
> + g_hash_table_iter_next (&iter, &instance, &id);
> + #else
> + g_hash_table_find(instance_to_id_map, (GHRFunc)find_first_item_in_hash_table, &instance);
> + #endif
> + return instance;
> +}
> +#endif
> +
Typo here: it should be getFirstTableInstance.
I expected it to take arguments but I guess it isn't necessary.
> NPError
> get_cookie_info(const char* siteAddr, char** cookieString, uint32_t* len)
> {
> @@ -912,15 +937,9 @@
> // Fortunately, XULRunner does not care about the instance as long as it is
> // valid. So we just pick the first valid one and use it. Proxy/Cookie
> // information is not instance specific anyway, it is URL specific.
> -
> if (browser_functions.getvalueforurl)
> {
> - GHashTableIter iter;
> - gpointer id, instance;
> -
> - g_hash_table_iter_init (&iter, instance_to_id_map);
> - g_hash_table_iter_next (&iter, &instance, &id);
> -
> + gpointer instance=getFirtsTableInstance();
> return browser_functions.getvalueforurl((NPP) instance, NPNURLVCookie, siteAddr, cookieString, len);
> } else
> {
> @@ -1363,21 +1382,19 @@
>
> #else
>
> +
> if (browser_functions.getvalueforurl)
> {
>
> // As in get_cookie_info, we use the first active instance
> - GHashTableIter iter;
> - gpointer id, instance;
> -
> - g_hash_table_iter_init (&iter, instance_to_id_map);
> - g_hash_table_iter_next (&iter, &instance, &id);
> -
> + gpointer instance=getFirtsTableInstance();
> browser_functions.getvalueforurl((NPP) instance, NPNURLVProxy, siteAddr, proxy, len);
> } else
> {
> return NPERR_GENERIC_ERROR;
> }
> +
> +
> #endif
>
> return NPERR_NO_ERROR;
> @@ -1403,6 +1420,17 @@
> return FALSE;
> }
>
> +#ifdef GLIB214
> +int
> +g_strcmp0(char *str1, char *str2)
> +{
> + if (str1 != NULL)
> + return str2 != NULL ? strcmp(str1, str2) : 1;
> + else // str1 == NULL
> + return str2 != NULL ? 1 : 0;
> +}
> +#endif
> +
This and the other #ifdef LEGACY_GLIB block would be better placed together
at the beginning of the file, out of the way of the functions that do the
real work. You could use one #ifdef then too.
Deleting below this as it looks like duplication. Please post the next
version in a new mail.
Thanks,
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D 0698 0713 C3ED F586 2A37
More information about the distro-pkg-dev
mailing list