Re: Code changes for glib to enhance 64-bit operation



On Wed, 18 Apr 2001, Mark Murnane wrote:

> It has been mentioned in more than one e-mail that Gnome has no problems
> on 64-bit platforms, namely Alpha based systems.  While this may be
> true,
> there are many cases where 64-bit errors could occur but through luck
> have not.  In our own case we found memory alignment problems when using
> a 64-bit version of gmodule.

i'd like to hear more about that, as you didn't significantly patch
gmodule, what exact alignment problems did you run into?

> The bulk of the changes relate to making glib capable of handling 64-bit
> strings.  This may not exactly occur often, but Y2K caught a lot of 
> people off guard :-)
> 
> This move makes sense for another reason.  On both the Solaris and Linux
> platforms, many of the string library functions are specified as 
> returning size_t values, or taking them as parameters.  It makes sense 
> then that glib use the equivalent value rather than relying on the fact 
> that longs, pointer and ints are all basically the same thing on a 
> 32-bit platform.

the string/memory/file size changes you made should definitely go
into glib.
you didn't seem to be overly consistent in applying gulong vs. gsize,
sizes which are currently 32bit should probably just become g[s]size
and not longs instead.

> Another significant change we had to make was to change the use of all 
> GPOINTER_TO_[U]INT macros.  We introduced a GPOINTER_TO_[U]LONG macro
> wherever the integer version was used.  While we can't change every 
> application that makes use of these macros, we would encourage it.  
> It'll make apps more reliable as 64-bit computing becomes more 
> widespread.

the basic intention of the GPOINTER_TO_[u]INT macros is to provide
users with a compiler independant way of storing an integer in a pointer,
and it provides the guarrantee that GPOINTER_TO_INT (GINT_TO_POINTER (i))
is a conversion without loss.
on some systems, sizeof (gpointer) < sizeof (glong) which is why we
can't make that guarantee there and would effectively fool users with
providing lossage-bound casting macors for longs.

> A change which we would also like to make concerns the definition of
> gsize and gssize.  These are currently defined to be 64-bit values 
> when on a 64-bit platform.  I'd like to come up with some way to
> redefine this as something like size_t and ssize_t, but I found that
> not every platform know what ssize_t is.  Your input on this would
> be appreciated.

this is essentially what glib does. to quote configure.in:
        case x$glib_size_t in
        x2) echo "typedef gint16  gssize;" >> $outfile
            echo "typedef guint16 gsize;"  >> $outfile
            ;;
        x4) echo "typedef gint32  gssize;" >> $outfile
            echo "typedef guint32 gsize;"  >> $outfile
            ;;
        x8) echo "typedef gint64  gssize;" >> $outfile
            echo "typedef guint64 gsize;"  >> $outfile
            ;;
        *)  echo "#error size of size_t is unknown" >> $outfile
            ;;
        esac
because some systems don't define ssize_t, but every system we currently
run on at least provides size_t, we make the assumption that
sizeof(size_t)==sizeof(ssize_t) and based on that always provide gsize and
gssize with the same size as sizeof(size_t).

> A final change we made was to redefine GHashFunc to return a glong. 
> This was done due to the use of functions like g_direct_hash, which 
> basically return a pointer. This causes the same loss of data that the
> GPOINTER_TO_[U]INT macros cause.

as i stated earlier, the intention behind GPOINTER_TO_INT() and friends
is to provide int->pointer->int conversion without lossage, mostly
because we have a couple of places where users get to plug a
gpointer user_data; item, but often just require an int.
we don't make that same guarantee for pointer->int->pointer conversions
and so the macros don't need changing towards longs.
while you're right that a hash-function
guint hash (gpointer p) { return GPOINTER_TO_UINT (p); }
provides a hash value that doesn't carry all of the information of
a 64bit pointer, this is not an case that has to be fixed for 64bit
systems. simply because hash value generation is by definition a
lossage-based index-hint generation to store values in hash buckets.
(and we're very unlikely to require hash-tables with more than
2^32 buckets in the near future).
to avoid hash-collisions for hashtables indexed by pointers in the
case where you store 64bit pointers that only differ in the upper
32bits (though i'd expect that to be a very rare case scenario), the
only change i could imagine you'd want to be doing is:
guint
g_direct_hash (gconstpointer v)
{
#if GLIB_SIZEOF_VOID_P > 4
  gulong lp = (gulong) v;
  return (lp >> 32) | (lp & 0xffffffff);
#else
  return GPOINTER_TO_UINT (v);
#endif
}

similarly, we shouldn't redefine GQuark from the guint32 which
it currently is to glong. there is a 1:1 mapping between quarks and
the elements of a unique string list.
in order to exceed the value range of a GQuark, you need sufficient
space to store at least 2^32 different string, plus their pointer
and zero termination. that requires a machine that can spare more
than sizeof (gpointer) * (4 + 1) * 2^32 = 8 * 5 * 2^32 = 163.8GB
for quark lists per process. i personally think we're unlikely to reach
that limit any time soon.


> Similarly, GCompareFunc was modified.  The API does not specify what
> the return value is.  People may try to interpret this value and if
> the are dealing with strings, large numbers or pointers, they may
> lose data.

GCompareFunc should actually just return {-1,0,1}, therefore with the
new binary searchable arrays in glib 2.0, we introduced a macro
that would return a valid comparision value for types beyond 32bit:

#define G_BSEARCH_ARRAY_CMP(v1,v2) ((v1) < (v2) ? -1 : (v1) > (v2) ? 1 : 0)

it might be a good idea to put that into a BSearchArray-independant
namespace though, say:

#define G_COMPARE_VALUES(v1,v2) ((v1) < (v2) ? -1 : (v1) > (v2) ? 1 : 0)


> Index: configure.in
> ===================================================================
> RCS file: /cvs/gnome/glib/configure.in,v
> retrieving revision 1.190
> diff -u -r1.190 configure.in
> --- configure.in	2001/04/17 00:55:34	1.190
> +++ configure.in	2001/04/18 18:06:16
> @@ -1597,6 +1597,12 @@
>  
>  #define GINT_TO_POINTER(i)	((gpointer) ${glib_gpi_cast} (i))
>  #define GUINT_TO_POINTER(u)	((gpointer) ${glib_gpui_cast} (u))
> +
> +#define GPOINTER_TO_LONG(p)  ((glong) (p))
> +#define GPOINTER_TO_ULONG(p) ((gulong) (p))
> +
> +#define GLONG_TO_POINTER(l)  ((gpointer) (l))
> +#define GULONG_TO_POINTER(u) ((gpointer) (u))

we shouldn't provide these.

> diff -u -r1.4 gasyncqueue.c
> --- gasyncqueue.c	2000/11/02 14:54:51	1.4
> +++ gasyncqueue.c	2001/04/18 18:06:16
> @@ -303,7 +303,7 @@
>  gpointer
>  g_async_queue_try_pop (GAsyncQueue* queue)
>  {
> -  gpointer retval;
> +  gpointer retval;     
>  
>    g_return_val_if_fail (queue, NULL);
>    g_return_val_if_fail (queue->ref_count > 0, NULL);
> @@ -399,7 +399,7 @@
>  gint
>  g_async_queue_length (GAsyncQueue* queue)
>  {
> -  glong retval;
> +  gint retval;     /* 64-bit change: glong->gint */

we don't need to have comments for changes like this, so please
back out the "/* 64-bit change: ..:" comments in general (except
when you feel that 64bit could provide a bad trap to tap in to).

> @@ -353,10 +353,10 @@
>    gchar *outp;
>    const gchar *insert_str = NULL;
>    const gchar *p;
> -  int inbytes_remaining;
> +  glong inbytes_remaining;    /* 64-bit change: gint -> glong */
>    const gchar *save_p = NULL;
>    size_t save_inbytes = 0;
> -  size_t outbytes_remaining;
> +  size_t outbytes_remaining; 
>    size_t err;
>    GIConv cd;
>    size_t outbuf_size;

though not your fault, the glib code here should actually use gsize
right away instead of size_t (esp. since ssize_t isn't always present).

> @@ -564,9 +564,9 @@
>   **/
>  gchar *
>  g_locale_to_utf8 (const gchar  *opsysstring,
> -		  gint          len,
> -		  gint         *bytes_read,
> -		  gint         *bytes_written,
> +		  glong         len,            /* 64-bit change: gint -> glong */
> +		  glong        *bytes_read,     /* 64-bit change: gint* -> glong* */
> +		  glong        *bytes_written,  /* 64-bit change: gint* -> glong* */
>  		  GError      **error)

shouldn't these (and similar functions) use gssize types
instead of longs?


> @@ -869,7 +869,7 @@
>  
>    if (bytes_read || bytes_written)
>      {
> -      gint len = strlen (opsysstring);
> +      gsize len = strlen (opsysstring);   /* 64-bit change: gint -> gsize */
>  
>        if (bytes_read)
>  	*bytes_read = len;

since you apply a gsize here?

> Index: gdataset.c
> ===================================================================
> RCS file: /cvs/gnome/glib/gdataset.c,v
> retrieving revision 1.17
> diff -u -r1.17 gdataset.c
> --- gdataset.c	2001/02/17 23:30:46	1.17
> +++ gdataset.c	2001/04/18 18:06:17
> @@ -543,7 +543,7 @@
>    
>    G_LOCK (g_quark_global);
>    if (g_quark_ht)
> -    quark = GPOINTER_TO_UINT (g_hash_table_lookup (g_quark_ht, string));
> +      quark = (GQuark)g_hash_table_lookup (g_quark_ht, string);  /* 64-bit change */

GQuarks should stay guint32, so this isn't necessary.

>    G_UNLOCK (g_quark_global);
>    
>    return quark;

> Index: ghash.c
> ===================================================================
> RCS file: /cvs/gnome/glib/ghash.c,v
> retrieving revision 1.24
> diff -u -r1.24 ghash.c
> --- ghash.c	2001/03/30 18:14:40	1.24
> +++ ghash.c	2001/04/18 18:06:17
> @@ -604,7 +604,7 @@
>    GHashNode *node;
>    GHashNode *next;
>    gfloat nodes_per_list;
> -  guint hash_val;
> +  gulong hash_val;    /* 64-bit change:  guint -> gulong */

hashes should stay a guint also.

>    gint new_size;
>    gint i;

> Index: ghash.h
> ===================================================================
> RCS file: /cvs/gnome/glib/ghash.h,v
> retrieving revision 1.4
> diff -u -r1.4 ghash.h
> --- ghash.h	2001/03/30 18:14:40	1.4
> +++ ghash.h	2001/04/18 18:06:17
> @@ -80,24 +80,26 @@
>  
>  /* Hash Functions
>   */
> -gboolean g_str_equal (gconstpointer  v,
> -                      gconstpointer  v2);
> -guint    g_str_hash  (gconstpointer  v);
>  
> -gboolean g_int_equal (gconstpointer  v,
> -                      gconstpointer  v2) G_GNUC_CONST;
> -guint    g_int_hash  (gconstpointer  v) G_GNUC_CONST;
> +gboolean g_str_equal (gconstpointer   v,
> +                      gconstpointer   v2);
> +gulong   g_str_hash  (gconstpointer   v);    /* 64-bit change */
>  
> +gboolean g_int_equal (gconstpointer   v,
> +                      gconstpointer   v2) G_GNUC_CONST;
> +gulong   g_int_hash  (gconstpointer   v) G_GNUC_CONST;    /* 64-bit change */
> +
> +
[...]

none of theese need changing either if hashes stay in size.

> Index: ghook.c
> ===================================================================
> RCS file: /cvs/gnome/glib/ghook.c,v
> retrieving revision 1.18
> diff -u -r1.18 ghook.c
> --- ghook.c	2001/03/08 16:34:57	1.18
> +++ ghook.c	2001/04/18 18:06:17
> @@ -631,5 +631,5 @@
>  g_hook_compare_ids (GHook *new_hook,
>  		    GHook *sibling)
>  {
> -  return ((glong) new_hook->hook_id) - ((glong) sibling->hook_id);
> +  return ((glong) new_hook->hook_id) - ((glong) sibling->hook_id);    
>  }

this should probably just do G_COMPARE_VALUES() since
g_hook_compare_ids() returns an int (and hook ids are gulongs).

> Index: gmain.c
> ===================================================================
> RCS file: /cvs/gnome/glib/gmain.c,v
> retrieving revision 1.57
> diff -u -r1.57 gmain.c
> --- gmain.c	2001/04/02 16:34:08	1.57
> +++ gmain.c	2001/04/18 18:06:18
> @@ -197,13 +197,13 @@
>  static void g_main_context_wakeup               (GMainContext *context);
>  
>  static gboolean g_timeout_prepare  (GSource     *source,
> -				    gint        *timeout);
> +				    glong        *timeout);    /* 64-bit change: gint* -> glong* */

i'm not too sure about this, and would like to get owen's word on it.
though since struct timeval uses longs for tv_sec and tv_usec, i'd
probably vote for longs here.

> Index: gmem.c
> ===================================================================
> RCS file: /cvs/gnome/glib/gmem.c,v
> retrieving revision 1.28
> diff -u -r1.28 gmem.c
> --- gmem.c	2001/02/17 23:30:46	1.28
> +++ gmem.c	2001/04/18 18:06:19
> @@ -58,9 +58,9 @@
>  #  define IN_MEM_CHUNK_ROUTINE()	FALSE
>  #else	/* !G_DISABLE_CHECKS */
>  static GPrivate* mem_chunk_recursion = NULL;
> -#  define MEM_CHUNK_ROUTINE_COUNT()	GPOINTER_TO_UINT (g_private_get (mem_chunk_recursion))
> -#  define ENTER_MEM_CHUNK_ROUTINE()	g_private_set (mem_chunk_recursion, GUINT_TO_POINTER (MEM_CHUNK_ROUTINE_COUNT () + 1))
> -#  define LEAVE_MEM_CHUNK_ROUTINE()	g_private_set (mem_chunk_recursion, GUINT_TO_POINTER (MEM_CHUNK_ROUTINE_COUNT () - 1))
> +#  define MEM_CHUNK_ROUTINE_COUNT()	GPOINTER_TO_ULONG (g_private_get (mem_chunk_recursion))
> +#  define ENTER_MEM_CHUNK_ROUTINE()	g_private_set (mem_chunk_recursion, GULONG_TO_POINTER (MEM_CHUNK_ROUTINE_COUNT () + 1))
> +#  define LEAVE_MEM_CHUNK_ROUTINE()	g_private_set (mem_chunk_recursion, GULONG_TO_POINTER (MEM_CHUNK_ROUTINE_COUNT () - 1))
>  #endif	/* !G_DISABLE_CHECKS */

apart from use not introducing GPOINTER_TO_ULONG(), there'd still
be no need change this here. MEM_CHUNK_ROUTINE_COUNT() counts recursion
depths of GMemChunk routines and is unlikely to exceed 2.

>  #ifndef	REALLOC_0_WORKS
> @@ -101,7 +101,7 @@
>  static void
>  standard_free (gpointer mem)
>  {
> -  return free (mem);
> +  /*return*/ free (mem);

thanks, i've cought that in my tree.
in case you came across this because your compiler gave a warning,
that might indicate that our configure.in needs fixing (assuming
you have a system with sane malloc() etc. prototypes).
could you check whether

dnl ok, here we try to check whether the systems prototypes for
dnl malloc and friends actually match the prototypes provided
dnl by gmem.h (keep in sync). i currently only know how to check
dnl this reliably with gcc (-Werror), improvements for other
dnl compilers are apprechiated.
SANE_MALLOC_PROTOS=no
AC_MSG_CHECKING([if malloc() and friends prototypes are gmem.h compatible])
glib_save_CFLAGS=$CFLAGS
if test "x$GCC" = "xyes"; then
  CFLAGS="$CFLAGS -Werror"
  AC_TRY_COMPILE([#include <stdlib.h>], [
    void* (*my_calloc_p)  (size_t, size_t) = calloc;
    void* (*my_malloc_p)  (size_t)         = malloc;
    void  (*my_free_p)    (void*)          = free;
    void* (*my_realloc_p) (void*, size_t)  = realloc;
    my_calloc_p = 0;
    my_malloc_p = 0;
    my_free_p = 0;
    my_realloc_p = 0;
  ],
    AC_DEFINE(SANE_MALLOC_PROTOS)
    SANE_MALLOC_PROTOS=yes)
fi
AC_MSG_RESULT($SANE_MALLOC_PROTOS)
CFLAGS=$glib_save_CFLAGS

passes for you?


>  }
>  static gpointer
>  standard_calloc (gsize n_blocks,
> @@ -403,9 +403,10 @@
>  {
>    gulong *p;
>  
> -#ifdef  G_ENABLE_DEBUG
> -  if (g_trap_malloc_size == n_bytes)
> +#ifdef  G_ENABLE_DEBUG 
> +  if (g_trap_malloc_size == n_bytes) {
>      G_BREAKPOINT ();
> +  }
>  #endif  /* G_ENABLE_DEBUG */

these braces aren't required, and btw, we put opening
braces after newlines only.

>    p = standard_malloc (sizeof (gulong) * 2 + n_bytes);
> @@ -979,6 +980,7 @@
>  	    }
>  	}
>      }
> +
>    LEAVE_MEM_CHUNK_ROUTINE ();
>  }


> Index: gmessages.c
> ===================================================================
> RCS file: /cvs/gnome/glib/gmessages.c,v
> retrieving revision 1.25
> diff -u -r1.25 gmessages.c
> --- gmessages.c	2001/03/09 21:23:33	1.25
> +++ gmessages.c	2001/04/18 18:06:19
> @@ -66,7 +66,8 @@
>  
>  
>  /* --- prototypes --- */
> -static inline guint printf_string_upper_bound (const gchar *format,
> +/* 64-bit change:  Return type - guint -> gsize */
> +static inline gsize printf_string_upper_bound (const gchar *format,  
>  					       gboolean     may_warn,
>  					       va_list      args);
>  
> @@ -401,7 +402,7 @@
>        test_level = 1 << i;
>        if (log_level & test_level)
>  	{
> -	  guint depth = GPOINTER_TO_UINT (g_private_get (g_log_depth));
> +	  gulong depth = GPOINTER_TO_ULONG (g_private_get (g_log_depth));

there's no need to use longs here, log level handlers abort after
a recursion depth of 2.

> ===================================================================
> RCS file: /cvs/gnome/glib/grel.c,v
> retrieving revision 1.9
> diff -u -r1.9 grel.c
> --- grel.c	2000/10/30 14:34:52	1.9
> +++ grel.c	2001/04/18 18:06:19
> @@ -63,7 +63,7 @@
>    return a[0] == b[0] && a[1] == b[1];
>  }
>  
> -static guint
> +static gulong    /* 64-bit change: guint -> gulong */
>  tuple_hash_2 (gconstpointer v_a)
>  {
>    gpointer* a = (gpointer*) v_a;
> @@ -279,7 +279,8 @@
>  {
>    gpointer    *tuple = (gpointer*) tuple_value;
>    GRealTuples *tuples = (GRealTuples*) user_data;
> -  gint stride = sizeof (gpointer) * tuples->width;
> +  /* 64-bit change: gint -> glong */
> +  glong stride = sizeof (gpointer) * tuples->width;   
>    
>    g_assert (tuple_key == tuple_value);

there's no need to do this change, while it certainly wouldn't
hurt, you don't expect the stride used in the following memcpy(,,stride)
to really exceed 2^32, do you?

> @@ -188,7 +188,7 @@
>  gchar*
>  g_strconcat (const gchar *string1, ...)
>  {
> -  guint	  l;
> +  gsize	  l;     /* 64-bit change: guint -> gsize */
>    va_list args;
>    gchar	  *s;
>    gchar	  *concat;
> @@ -862,10 +862,14 @@
>  	   const gchar *src,
>  	   gsize        dest_size)
>  {
> +  gsize retval;
> +
>    g_return_val_if_fail (dest != NULL, 0);
>    g_return_val_if_fail (src  != NULL, 0);
>    
> -  return strlcat (dest, src, dest_size);
> +  retval = strlcat (dest, src, dest_size);
> +
> +  return retval;
>  }

i don't see why you had to do this, g_strlcat() already
returns a gsize value.

> Index: gstrfuncs.h
> ===================================================================
> RCS file: /cvs/gnome/glib/gstrfuncs.h,v
> retrieving revision 1.6
> diff -u -r1.6 gstrfuncs.h
> --- gstrfuncs.h	2001/04/16 20:05:23	1.6
> +++ gstrfuncs.h	2001/04/18 18:06:21
> @@ -46,11 +46,11 @@
>  					gchar	    **endptr);
>  G_CONST_RETURN gchar* g_strerror       (gint	      errnum) G_GNUC_CONST;
>  G_CONST_RETURN gchar* g_strsignal      (gint	      signum) G_GNUC_CONST;
> -gint	              g_strcasecmp     (const gchar *s1,
> +glong              g_strcasecmp     (const gchar *s1,  /* 64-bit change */
>  					const gchar *s2);
> -gint	              g_strncasecmp    (const gchar *s1,
> +glong              g_strncasecmp    (const gchar *s1,  /* 64-bit change */
>  					const gchar *s2,
> -					guint        n);
> +					gsize        n);    /* 64-bit change */
>  gchar*	              g_strdown	       (gchar	     *string);
>  gchar*	              g_strup	       (gchar	     *string);
>  gchar*	              g_strreverse     (gchar	     *string);
> @@ -76,8 +76,8 @@
>  gchar*	              g_strdup_vprintf (const gchar *format,
>  					va_list      args);
>  gchar*	              g_strndup	       (const gchar *str,
> -					guint        n);
> -gchar*	              g_strnfill       (guint        length,
> +					gsize        n);  /* 64-bit change */
> +gchar*	              g_strnfill       (gsize        length,  /* 64-bit change */
>  					gchar        fill_char);
>  gchar*	              g_strconcat      (const gchar *string1,
>  					...); /* NULL terminated */

i think the longs here also should be gsize.

> @@ -77,12 +77,12 @@
>    return strcmp (string1, string2) == 0;
>  }
>  
> -/* 31 bit hash function */
> -guint
> +/* 31/63 bit hash function */
> +gulong    /* 64-bit change:  guint -> gulong */
>  g_str_hash (gconstpointer key)
>  {
>    const char *p = key;
> -  guint h = *p;
> +  gulong h = *p;    /* 64-bit change: guint -> gulong */
>  
>    if (h)
>      for (p += 1; *p != '\0'; p++)

this is not necessary as dicussed above. but apart from that,
the comment is misleading, the 31 refers to h = (h << 5) - h,
i.e. 2^5-1 and correlates with the amount of significant bits
in a character to generate string hashes.

> @@ -193,10 +193,10 @@
>  
>  /* Strings.
>   */
> -static inline gint
> -nearest_power (gint num)
> +static inline glong    /* 64-bit change: gint -> glong */
> +nearest_power (glong num)    /* 64-bit change: gint -> glong */
>  {
> -  gint n = 1;
> +  glong n = 1;    /* 64-bit change: gint -> glong */
>  
>    while (n < num)
>      n <<= 1;

this reminds me, for 64bit systems, we might want to adapt:

G_INLINE_FUNC guint
g_bit_storage (guint number)
{
  register guint n_bits = 0;

  do
    {
      n_bits++;
      number >>= 1;
    }
  while (number);
  return n_bits;
}

but simply doing s/guint/gulong/ is probably a bad idea, since even
on 32bit platforms one might want to know the number of used bits
for 64bit numbers. so the right thing to do here (and for nearest_power
variants) is probably to operate with guint64 values.

> Index: gstring.h
> ===================================================================
> RCS file: /cvs/gnome/glib/gstring.h,v
> retrieving revision 1.3
> diff -u -r1.3 gstring.h
> --- gstring.h	2001/03/07 14:46:41	1.3
> +++ gstring.h	2001/04/18 18:06:21
> @@ -37,12 +37,12 @@
>  struct _GString
>  {
>    gchar *str;
> -  gint len;
> +  glong len;    /* 64-bit change: gint -> glong */
>  };

like in many other places, this should probably be gssize not glong.

> Index: gthread.h
> ===================================================================
> RCS file: /cvs/gnome/glib/gthread.h,v
> retrieving revision 1.11
> diff -u -r1.11 gthread.h
> --- gthread.h	2001/04/02 16:34:08	1.11
> +++ gthread.h	2001/04/18 18:06:21
> @@ -221,7 +221,7 @@
>  
>  struct _GStaticPrivate
>  {
> -  guint index;
> +  gulong index;    /* 64-bit change */

this is not required, we're very unlikely to attach more than 2^32
private data pointers to a thread.

>  };
>  #define G_STATIC_PRIVATE_INIT { 0 }
>  void     g_static_private_init           (GStaticPrivate   *private_key);

> Index: gtypes.h
> ===================================================================
> RCS file: /cvs/gnome/glib/gtypes.h,v
> retrieving revision 1.9
> diff -u -r1.9 gtypes.h
> --- gtypes.h	2001/03/18 04:44:35	1.9
> +++ gtypes.h	2001/04/18 18:06:21
> @@ -66,9 +66,11 @@
>  typedef void* gpointer;
>  typedef const void *gconstpointer;
>  
> -typedef gint            (*GCompareFunc)         (gconstpointer  a,
> +/* 64-bit change */
> +typedef glong           (*GCompareFunc)         (gconstpointer  a,
>                                                   gconstpointer  b);
> -typedef gint            (*GCompareDataFunc)     (gconstpointer  a,
> +/* 64-bit change */
> +typedef glong           (*GCompareDataFunc)     (gconstpointer  a,
>                                                   gconstpointer  b,
>  						 gpointer       user_data);

see above on GCompareFunc.

>  typedef gboolean        (*GEqualFunc)           (gconstpointer  a,
> @@ -76,7 +78,8 @@
>  typedef void            (*GDestroyNotify)       (gpointer       data);
>  typedef void            (*GFunc)                (gpointer       data,
>                                                   gpointer       user_data);
> -typedef guint           (*GHashFunc)            (gconstpointer  key);
> +/* 64-bit change */
> +typedef gulong          (*GHashFunc)            (gconstpointer  key);

not required.

>  typedef void            (*GHFunc)               (gpointer       key,
>                                                   gpointer       value,
>                                                   gpointer       user_data);

> Index: gutils.c
> ===================================================================
> RCS file: /cvs/gnome/glib/gutils.c,v
> retrieving revision 1.92
> diff -u -r1.92 gutils.c
> --- gutils.c	2001/04/16 15:04:17	1.92
> +++ gutils.c	2001/04/18 18:06:22
> @@ -335,7 +335,7 @@
>    return NULL;
>  }
>  
> -gint
> +glong    /* 64-bit change - gint -> glong */
>  g_snprintf (gchar	*str,
>  	    gulong	 n,
>  	    gchar const *fmt,
> @@ -343,7 +343,7 @@

at least on my system (glibc2.2) printf still returns an int,
are there libraries that actually return longs from printf and
friends?

> @@ -858,7 +858,7 @@
>          struct passwd pwd;
>  #    ifdef _SC_GETPW_R_SIZE_MAX  
>  	/* This reurns the maximum length */
> -        guint bufsize = sysconf (_SC_GETPW_R_SIZE_MAX);
> +        guint bufsize = (guint)sysconf (_SC_GETPW_R_SIZE_MAX);  /* 64-bit cast */

you don't need a cast to go from int->long, though here using
glong for bufsize probably wouldn't hurt as that's what
sysconf() returns.

> @@ -1050,10 +1050,10 @@
>    G_UNLOCK (g_utils_global);
>  }
>  
> -guint
> +gulong    /* 64-bit change:  guint -> gulong */
>  g_direct_hash (gconstpointer v)
>  {
> -  return GPOINTER_TO_UINT (v);
> +  return GPOINTER_TO_ULONG (v);   /* 64-bit change:  Macro change */

see above discussion.

>  }
>  
>  gboolean

> Index: testglib.c
> ===================================================================
> RCS file: /cvs/gnome/glib/testglib.c,v
> retrieving revision 1.49
> diff -u -r1.49 testglib.c
> --- testglib.c	2001/03/09 21:31:21	1.49
> +++ testglib.c	2001/04/18 18:06:22
> @@ -46,7 +46,7 @@
>  #include <io.h>			/* For read(), write() etc */
>  #endif
>  
> -static int array[10000];
> +static int array[10000];    /* 64-bit change: int -> long */

you didn't do what the comment says, and actually don't need to
because "int" is just an arbitrary type for hash table elements
here.

>  static gboolean failed = FALSE;
>  
>  #define	TEST(m,cond)	G_STMT_START { failed = !(cond); \

> Index: gobject/gclosure.c
> ===================================================================
> RCS file: /cvs/gnome/glib/gobject/gclosure.c,v
> retrieving revision 1.11
> diff -u -r1.11 gclosure.c
> --- gobject/gclosure.c	2001/04/09 17:03:55	1.11
> +++ gobject/gclosure.c	2001/04/18 18:06:23
> @@ -410,6 +410,7 @@
>  		  gpointer        invocation_hint)
>  {
>    g_return_if_fail (closure != NULL);
> +  g_return_if_fail (closure->marshal || closure->meta_marshal);
>  
>    if (!closure->is_invalid)
>      {
> @@ -417,8 +418,6 @@
>        gpointer marshal_data;
>        gboolean in_marshal = closure->in_marshal;
>  
> -      g_return_if_fail (closure->marshal || closure->meta_marshal);
> -

you seem to be diffing with non-recent version of your tree against
CVS HEAD.

> @@ -504,8 +503,8 @@
>  {
>    GTypeClass *class;
>    gpointer callback;
> -  /* GType itype = GPOINTER_TO_UINT (closure->data); */
> -  guint offset = GPOINTER_TO_UINT (marshal_data);
> +  /* GType itype = GPOINTER_TO_ULONG(closure->data); */
> +  guint offset = GPOINTER_TO_ULONG(marshal_data);

we put spaces before openeing paranthesis, and this change
isn't required.

> Index: gobject/genums.c
> ===================================================================
> RCS file: /cvs/gnome/glib/gobject/genums.c,v
> retrieving revision 1.7
> diff -u -r1.7 genums.c
> --- gobject/genums.c	2001/03/07 14:46:45	1.7
> +++ gobject/genums.c	2001/04/18 18:06:24
> @@ -127,7 +127,7 @@
>  			      GTypeCValue  *collect_values,
>  			      guint         collect_flags)
>  {
> -  gint *int_p = collect_values[0].v_pointer;
> +  glong *int_p = collect_values[0].v_pointer;

this actually breaks the API and existing user code, as people
will now possibly get their memory space overriden in queries
such as:

gint enum_value;
g_object_get (object,
              "an_enum_property", &enum_value,
              NULL);

basically enums are stored in integers and flags are stored in
unsigned integers as far as the type system and GValue APIs
are concerned, we need to keep those constrains.

>    if (!int_p)
>      return g_strdup_printf ("value location for `%s' passed as NULL", G_VALUE_TYPE_NAME (value));

> Index: gobject/gtype.h
> ===================================================================
> RCS file: /cvs/gnome/glib/gobject/gtype.h,v
> retrieving revision 1.25
> diff -u -r1.25 gtype.h
> --- gobject/gtype.h	2001/04/01 04:04:46	1.25
> +++ gobject/gtype.h	2001/04/18 18:06:26
> @@ -120,7 +120,7 @@
>  typedef struct _GTypeInterface          GTypeInterface;
>  typedef struct _GTypeInstance           GTypeInstance;
>  typedef struct _GTypeInfo               GTypeInfo;
> -typedef struct _GTypeFundamentalInfo    GTypeFundamentalInfo;
> +typedef union  _GTypeFundamentalInfo    GTypeFundamentalInfo;
>  typedef struct _GInterfaceInfo          GInterfaceInfo;
>  typedef struct _GTypeValueTable         GTypeValueTable;
>  typedef struct _GTypeQuery		GTypeQuery;
> @@ -263,9 +263,10 @@
>    /* value handling */
>    const GTypeValueTable	*value_table;
>  };
> -struct _GTypeFundamentalInfo
> +union _GTypeFundamentalInfo
>  {
>    GTypeFundamentalFlags  type_flags;
> +  gsize                  ForAlignment;
>  };
>  struct _GInterfaceInfo
>  {

it's a bad idea to try to fix the gtype.c internal alignment
probelm in the public header file. in current CVS, i did:

#define SIZEOF_FUNDAMENTAL_INFO    ((gssize) MAX (MAX (sizeof (GTypeFundamentalInfo), \
                                                       sizeof (gpointer)), \
                                                  sizeof (glong)))
and am just using that throughout gtype.c instead of
sizeof(GTypeFundamentalInfo).

> 
> Thanks,
> 
> Mark
> 

---
ciaoTJ





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