After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 317804 - More warning cleanup for Pango
More warning cleanup for Pango
Status: RESOLVED FIXED
Product: pango
Classification: Platform
Component: general
1.10.x
Other All
: Low trivial
: ---
Assigned To: pango-maint
pango-maint
Depends on:
Blocks:
 
 
Reported: 2005-10-03 09:04 UTC by Behdad Esfahbod
Modified: 2005-11-04 23:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (82.65 KB, patch)
2005-10-03 09:09 UTC, Behdad Esfahbod
none Details | Review

Description Behdad Esfahbod 2005-10-03 09:04:49 UTC
Attaching a rather huge patch to clean up compiler warnings.  The aim is to make
compiler warnings easier to read such that real errors are spotted.

One of them main changes is adding G_GNUC_UNUSED to unused parameters.  If you
really don't want that, I can prepare another patch without them.  Other than
that, the rest is mostly adding const, initializing all struct members, adding
static in tools, using unsigned int for some loop vars, and adding NORETURN and
PRINTF hints to fail() functions.

In the process I actually spotted a few bugs, none of the critical though.  Like:
Index: tools/gen-script-for-lang.c
===================================================================
RCS file: /cvs/gnome/pango/tools/gen-script-for-lang.c,v
retrieving revision 1.5
diff -u -p -r1.5 gen-script-for-lang.c
--- tools/gen-script-for-lang.c 21 Jul 2005 18:15:45 -0000      1.5
+++ tools/gen-script-for-lang.c 3 Oct 2005 08:56:16 -0000
@@ -172,11 +165,11 @@ scripts_for_file (const char *base_dir,
   GIOStatus status = G_IO_STATUS_NORMAL;

   if (!channel)
-    fail ("Error opening '%s': %s\n", filename, error);
+    fail ("Error opening '%s': %s\n", filename, error->message);

   /* The files have ISO-8859-1 copyright signs in them */
   if (!g_io_channel_set_encoding (channel, "ISO-8859-1", &error))
-      fail ("Cannot set encoding when reading '%s': %s\n", filename, error);
+      fail ("Cannot set encoding when reading '%s': %s\n", filename,
error->message);

   while (status == G_IO_STATUS_NORMAL)
     {
@@ -209,7 +202,7 @@ scripts_for_file (const char *base_dir,
     }

   if (!g_io_channel_shutdown (channel, FALSE, &error))
-    fail ("Error closing '%s': %s\n", channel, error);
+    fail ("Error closing '%s': %s\n", filename, error->message);

   g_free (filename);
 }
@@ -240,7 +233,7 @@ compare_lang (gconstpointer a,
              gconstpointer b)
 {
   const LangInfo *info_a = a;
-  const LangInfo *info_b = a;
+  const LangInfo *info_b = b;

   return strcmp (pango_language_to_string (info_a->lang),
                 pango_language_to_string (info_b->lang));
Comment 1 Behdad Esfahbod 2005-10-03 09:09:31 UTC
Created attachment 52968 [details] [review]
Patch

No comment.
Comment 2 Owen Taylor 2005-10-03 12:09:59 UTC
You are compiling with -W?
Comment 3 Behdad Esfahbod 2005-10-03 12:17:22 UTC
Yes.  Here is the complete list, to go crazy with warning parameters I got from
Carl Worth :-)

-g -O3 -Wall -Wextra -Wfloat-equal \
-Wredundant-decls -Wmissing-noreturn -Wno-shadow -Wendif-labels \
-Wlarger-than-65500 -Wpointer-arith -Wno-cast-qual -Wcast-align -Wwrite-strings \
-Winline -Wformat=2 -Winit-self -Wno-switch-default -Wno-switch-enum \
-Wno-unused-parameter \
-Wmissing-declarations -Wmissing-prototypes -Wstrict-prototypes \
-Wold-style-definition -Wdeclaration-after-statement -Wbad-function-cast"

Of course I can stop using them all!  But these are the warnings I find
extremely helpful in spotting /problems/ in code.

That said, the unused parameter thing is really useless in any decent C code,
but seems like there's not an easy way to turn it off in gcc.  Although I'm
passing -Wno-unused-parameter, but since -Wall and -Wextra are passed, it still
triggers.
Comment 4 Matthias Clasen 2005-11-04 21:18:44 UTC
I'm not enthusiastic about adding G_GNUC_UNUSED to declarations.


+set_glyph (PangoFont *font G_GNUC_UNUSED,
+	   PangoGlyphString *glyphs,
+	   int i,
+	   int offset,
+	   PangoGlyph glyph)

If you want to format it, align the parameter names as well.


+      if (script >= (PangoScript)map->entries->len)

this looks odd, shouldn't you rather case script to some
integral type ?


-main (int argc, char **argv)
+main (void)

No, main always has two arguments...


The rest looks fine to me.
Comment 5 Behdad Esfahbod 2005-11-04 23:44:22 UTC
Removed all G_GNUC_UNUSED and main() stuff, fixed the (PangoScript) cast, committed.

2005-11-04  Behdad Esfahbod  <behdad@gnome.org>

        * configure.in, examples/argcontext.c examples/cairoview.c,
        examples/renderdemo.c, examples/renderdemo.h examples/xftview.c,
        modules/basic/basic-x.c, modules/hangul/hangul-fc.c,
        modules/hebrew/hebrew-shaper.c, modules/hebrew/hebrew-shaper.h,
        modules/indic/indic-fc.c, modules/indic/mprefixups.c,
        modules/syriac/syriac-fc.c, pango/break.c pango/fonts.c,
        pango/modules.c, pango/pango-coverage.c pango/pango-engine.c,
        pango/pango-engine.h, pango/pango-fontmap.c,
        pango/pango-fontset.c, pango/pango-impl-utils.h,
        pango/pango-layout.c, pango/pango-layout.h,
        pango/pango-renderer.c, pango/pango-script.c,
        pango/pango-utils.c, pango/pangocairo-fc.h,
        pango/pangocairo-font.c, pango/pangocairo-fontmap.c,
        pango/pangocairo-private.h, pango/pangofc-decoder.c,
        pango/pangofc-font.c, pango/pangofc-fontmap.c pango/pangoft2.c,
        pango/pangox-fontcache.c, pango/pangox-fontmap.c pango/pangox.c,
        pango/pangoxft-font.c, pango/querymodules.c,
        pango/opentype/ftglue.c, pango/opentype/ftxgpos.c,
        pango/opentype/ftxopen.c, pango/opentype/pango-ot-buffer.c,
        pango/opentype/pango-ot-info.c,
        pango/opentype/pango-ot-ruleset.c, tests/dump-boundaries.c,
        tests/testboundaries.c, tests/testcolor.c tests/testiter.c,
        tests/testscript.c: Turn various gcc warnings off. Adding const,
        adding static, fully initializing structs, match signedness in
        comparisons. (#317804)

        * tests/testscript.c, tools/gen-script-for-lang.c:
        (scripts_for_file): Pass error->message instead of error to fail(),
        which was wrong.
        (compare_lang): Fix typo comparing a and a instead of a and b.