Re: [patch] glib -- just call me the human -Werror

[Owen Taylor <otaylor redhat com>]

Darin Adler <darin bentspoon com> writes:

> I compiled glib HEAD and ran into warnings, which I fixed except for
> the one that seems due to an error in gcc's handling of printf format
> strings with positional parameters in them. There are a few important
> ones --
> those local variables named len will create a bit of trouble, and I
> think they should have caused some tests to fail.
> 
> Should I commit? Seems not worth attaching this to a bug report.

Please do, with a few things noted below. except for the shadowing
problems, where I've committed a separate fix.

(Not attaching a patch to a bug report is fine, but then we take 
no responsibility if we forget that you sent it!)

Thansk,
                                        Owen
 
> Index: gobject/gsignal.c
> ===================================================================
> RCS file: /cvs/gnome/glib/gobject/gsignal.c,v
> retrieving revision 1.35
> diff -p -u -r1.35 gsignal.c
> --- gobject/gsignal.c	2001/06/28 17:05:12	1.35
> +++ gobject/gsignal.c	2001/07/11 00:38:04
> @@ -1736,7 +1736,9 @@ g_signal_emitv (const GValue *instance_a
>     const GValue *param_values;
>     gpointer instance;
>     SignalNode *node;
> +#ifndef G_DISABLE_ASSERT
>     guint i;
> +#endif
> 
>     g_return_if_fail (instance_and_params != NULL);
>     instance = g_value_peek_pointer (instance_and_params);


This is the wrong conditional, right? #ifndef G_DISABLE_ASSERT is not
the same as #ifdef G_ENABLE_DEBUG which is the test in the rest
of the function.

> Index: tests/testglib.c
> ===================================================================
> RCS file: /cvs/gnome/glib/tests/testglib.c,v
> retrieving revision 1.51
> diff -p -u -r1.51 testglib.c
> --- tests/testglib.c	2001/06/30 16:54:33	1.51
> +++ tests/testglib.c	2001/07/11 00:38:07
> @@ -385,11 +385,13 @@ main (int   argc,
>     };
>     guint n_skip_root_checks = G_N_ELEMENTS (skip_root_checks);
> 
> +#ifndef G_DISABLE_ASSERT
>     guint16 gu16t1 = 0x44afU, gu16t2 = 0xaf44U;
>     guint32 gu32t1 = 0x02a7f109U, gu32t2 = 0x09f1a702U;
>   #ifdef G_HAVE_GINT64
>     guint64 gu64t1 = G_GINT64_CONSTANT(0x1d636b02300a7aa7U),
>   	  gu64t2 = G_GINT64_CONSTANT(0xa77a0a30026b631dU);
> +#endif
>   #endif
>     const char hello[] = "Hello, World";
>     const int hellolen = sizeof (hello) - 1;

Hmm, I wonder if testglib.c should have #undef G_DISABLE_ASSERT at the
top? It doesn't make much sense to me to run it without asssertion
checkings.