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 598994 - Fix compilation warnings
Fix compilation warnings
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: .General
2.19.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on: 601234 601337 611736
Blocks:
 
 
Reported: 2009-10-19 22:45 UTC by Javier Jardón (IRC: jjardon)
Modified: 2010-03-09 16:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix compilation warnings (6.12 KB, patch)
2009-10-19 22:45 UTC, Javier Jardón (IRC: jjardon)
none Details | Review
Remove some unused variables (1.49 KB, patch)
2009-10-20 02:14 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Use g_get_current_dir() instead getcwd() (1.40 KB, patch)
2009-10-20 02:15 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Catch the returned value by write() (1019 bytes, patch)
2009-10-20 02:16 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Use parenthesis in a expression with "&" operator (823 bytes, patch)
2009-10-20 02:16 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Reorder the function so the "lookup_context" variable can't be used uninitialized (1.97 KB, patch)
2009-10-20 02:17 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Added some default cases and assert if reached (1.74 KB, patch)
2009-10-20 02:17 UTC, Javier Jardón (IRC: jjardon)
needs-work Details | Review
Initialize variable to NULL (888 bytes, patch)
2009-10-20 02:19 UTC, Javier Jardón (IRC: jjardon)
needs-work Details | Review
Added some default cases and assert if reached.v2 (2.97 KB, patch)
2009-10-20 19:55 UTC, Javier Jardón (IRC: jjardon)
none Details | Review
Added some default cases and assert if reached.v3 (1.87 KB, patch)
2009-10-20 23:48 UTC, Javier Jardón (IRC: jjardon)
none Details | Review
Added some default cases and assert if reached.v4 (1.89 KB, patch)
2009-10-21 00:09 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Use switch instead if's to get a default value (3.64 KB, patch)
2009-10-21 00:41 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
use G_GSIZE_FORMAT instead %i (1.22 KB, patch)
2009-10-21 03:22 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Remove an unused function (1.85 KB, patch)
2009-10-21 03:50 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Cast to guchar* (706 bytes, patch)
2009-10-21 03:51 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Cast to gchar* (1.06 KB, patch)
2009-10-21 21:55 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Cast to gchar* (840 bytes, patch)
2009-10-21 22:01 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Cast to gchar* to fix a compilation warning in io-pnm (868 bytes, patch)
2009-10-24 15:12 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Cast to gchar* in io-gif (1.31 KB, patch)
2009-10-24 15:13 UTC, Javier Jardón (IRC: jjardon)
rejected Details | Review
Cast to gchar* in io-gif (1.29 KB, patch)
2009-10-25 21:20 UTC, Javier Jardón (IRC: jjardon)
none Details | Review
Cast to gchar* in io-gif (1.27 KB, patch)
2009-10-25 21:48 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Initialize to false the returned variable (809 bytes, patch)
2009-10-29 04:12 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
[gdk/x11] Cast to gchar* (785 bytes, patch)
2009-10-29 04:13 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
[gdk/x11] Fix warning: cast to guint* (1.31 KB, patch)
2009-10-29 04:14 UTC, Javier Jardón (IRC: jjardon)
needs-work Details | Review
[gtk/gtkcolorsel] Fix warning: cast to gchar* (774 bytes, patch)
2009-10-29 04:15 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
[gtk/gtkmain] Remove statement with no effect (658 bytes, patch)
2009-10-29 04:16 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
[gtk/gtkobject] Fix warning: cast to gint* (658 bytes, patch)
2009-10-29 04:17 UTC, Javier Jardón (IRC: jjardon)
rejected Details | Review
[gtk/gtktext] Fix warning: some casts to gchar* (1.48 KB, patch)
2009-10-29 04:18 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
[gtk/gtkclist] Fix warning: cast to gint8* (2.27 KB, patch)
2009-10-29 04:19 UTC, Javier Jardón (IRC: jjardon)
needs-work Details | Review
[gdk/x11/gdkkeys-x11] Fix some compilation warnings (1.71 KB, patch)
2009-10-30 03:16 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
[gtk/gtkobject] Fix warning: cast to gint* (1.35 KB, patch)
2009-10-30 03:24 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
[gtk/gtkclist] Use proper gint8 array variable instead a string (2.90 KB, patch)
2009-10-30 03:51 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Use gpointer (782 bytes, patch)
2009-11-02 02:36 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Cast to guint8* (6.29 KB, patch)
2009-11-02 02:37 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Cast to guchar* (1.25 KB, patch)
2009-11-09 03:10 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Cast to gchar* (882 bytes, patch)
2009-11-09 03:10 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Use const gchar (785 bytes, patch)
2009-11-09 03:11 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Remove unused gtk_text_btree_node_invalidate_downwar() function (2.20 KB, patch)
2009-11-09 03:14 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Use GINT_TO_POINTER() and GPOINTER_TO_INT() (1.31 KB, patch)
2009-11-09 03:15 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Use the proper G*_TO_POINTER macro (1.33 KB, patch)
2009-11-09 03:16 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Initialize variable to 0 (845 bytes, patch)
2009-11-09 05:46 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Disable test until it works (841 bytes, patch)
2009-11-09 05:46 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Initialize variable to NO_MATCH (1.66 KB, patch)
2009-11-09 05:47 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Use parenthesis in a expression with '&&' and '||' (1.73 KB, patch)
2009-11-09 05:48 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Use G_POINTER_TO_UINT (1007 bytes, patch)
2009-11-09 19:42 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Use G_GNUC_UNUSED macro (796 bytes, patch)
2009-11-09 20:52 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Use a variable to store the return value (802 bytes, patch)
2009-11-09 21:42 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Remove useless line (677 bytes, patch)
2009-12-02 08:29 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review
Use {} in empty if..else blocks (877 bytes, patch)
2009-12-02 08:30 UTC, Javier Jardón (IRC: jjardon)
committed Details | Review

Description Javier Jardón (IRC: jjardon) 2009-10-19 22:45:09 UTC
Created attachment 145812 [details] [review]
Fix compilation warnings

Here a little patch to fix some compilation warnings
Comment 1 Javier Jardón (IRC: jjardon) 2009-10-20 02:14:47 UTC
Created attachment 145817 [details] [review]
Remove some unused variables
Comment 2 Javier Jardón (IRC: jjardon) 2009-10-20 02:15:27 UTC
Created attachment 145818 [details] [review]
Use g_get_current_dir() instead getcwd()
Comment 3 Javier Jardón (IRC: jjardon) 2009-10-20 02:16:03 UTC
Created attachment 145819 [details] [review]
Catch the returned value by write()
Comment 4 Javier Jardón (IRC: jjardon) 2009-10-20 02:16:35 UTC
Created attachment 145820 [details] [review]
Use parenthesis in a expression with "&" operator
Comment 5 Javier Jardón (IRC: jjardon) 2009-10-20 02:17:11 UTC
Created attachment 145821 [details] [review]
Reorder the function so the "lookup_context" variable can't be used uninitialized
Comment 6 Javier Jardón (IRC: jjardon) 2009-10-20 02:17:49 UTC
Created attachment 145822 [details] [review]
Added some default cases and assert if reached
Comment 7 Javier Jardón (IRC: jjardon) 2009-10-20 02:19:05 UTC
Created attachment 145823 [details] [review]
Initialize variable to NULL

The function will assert if the variable == NULL after the if's block to assign its value
Comment 8 Benjamin Otte (Company) 2009-10-20 18:21:50 UTC
Comment on attachment 145819 [details] [review]
Catch the returned value by write()

You probably shouldn't initialize the n_bytes variable to -1. With that fixed, go ahead and commit.
Comment 9 Benjamin Otte (Company) 2009-10-20 18:24:12 UTC
Comment on attachment 145822 [details] [review]
Added some default cases and assert if reached

Could you still initialize the variables to their default values here? Otherwise compilation with -DG_DISABLE_ASSERT=1 will give you the warnings back.

Currently you don't get the warnings because g_assert_not_reached() aborts the program, but that can be disabled, and then they're back.
Comment 10 Benjamin Otte (Company) 2009-10-20 18:30:09 UTC
Comment on attachment 145823 [details] [review]
Initialize variable to NULL

I think you want to switch (layout) in the dialog_get_pages_per_sheet(dialog)== 2 part here. And don't forget the default case. That should get rid of the need to initialize enum_value to NULL.
Comment 11 Javier Jardón (IRC: jjardon) 2009-10-20 18:46:10 UTC
Comment on attachment 145820 [details] [review]
Use parenthesis in a expression with "&" operator

commit 8e70bc988b5fc7589ee949be2f8f768bfd1a173d
Comment 12 Javier Jardón (IRC: jjardon) 2009-10-20 18:46:54 UTC
Comment on attachment 145819 [details] [review]
Catch the returned value by write()

committed without initialize n_bytes to -1

commit 33f53e1d0460b26f5dd4713555cc553dbef4847a
Comment 13 Javier Jardón (IRC: jjardon) 2009-10-20 18:47:21 UTC
Comment on attachment 145818 [details] [review]
Use g_get_current_dir() instead getcwd()

commit 232fca6e9587041c12d9b81e19d721c661c24207
Comment 14 Javier Jardón (IRC: jjardon) 2009-10-20 18:47:47 UTC
Comment on attachment 145817 [details] [review]
Remove some unused variables

commit d3155bb1eaf81a6175dff0347d7e6e146735a4d4
Comment 15 Javier Jardón (IRC: jjardon) 2009-10-20 19:55:40 UTC
Created attachment 145890 [details] [review]
Added some default cases and assert if reached.v2

Initialize variables to default values
Comment 16 Javier Jardón (IRC: jjardon) 2009-10-20 23:48:31 UTC
Created attachment 145907 [details] [review]
Added some default cases and assert if reached.v3
Comment 17 Javier Jardón (IRC: jjardon) 2009-10-21 00:09:28 UTC
Created attachment 145908 [details] [review]
Added some default cases and assert if reached.v4
Comment 18 Javier Jardón (IRC: jjardon) 2009-10-21 00:10:04 UTC
Comment on attachment 145908 [details] [review]
Added some default cases and assert if reached.v4

commit 670e141b3a23c9578dcffe7b04d6a0ff84404bec
Comment 19 Benjamin Otte (Company) 2009-10-21 00:30:12 UTC
Comment on attachment 145821 [details] [review]
Reorder the function so the "lookup_context" variable can't be used uninitialized

Seems I forget to mark this.
Comment 20 Benjamin Otte (Company) 2009-10-21 00:30:53 UTC
Comment on attachment 145908 [details] [review]
Added some default cases and assert if reached.v4

You forgot to set the lower rectangle to 0, too.
Comment 21 Javier Jardón (IRC: jjardon) 2009-10-21 00:41:12 UTC
Created attachment 145910 [details] [review]
Use switch instead if's to get a default value
Comment 22 Javier Jardón (IRC: jjardon) 2009-10-21 00:45:40 UTC
Comment on attachment 145910 [details] [review]
Use switch instead if's to get a default value

commit 69773763ee7fbd2c494a5b8c5ff13093715fb788
Comment 23 Benjamin Otte (Company) 2009-10-21 00:46:40 UTC
Comment on attachment 145910 [details] [review]
Use switch instead if's to get a default value

If you group the switch statements like:
case GTK_NUMBER_UP_LAYOUT_LEFT_TO_RIGHT_TOP_TO_BOTTOM:
case GTK_NUMBER_UP_LAYOUT_TOP_TO_BOTTOM_LEFT_TO_RIGHT:
  enum_value = g_enum_get_value (enum_class, GTK_NUMBER_UP_LAYOUT_LEFT_TO_RIGHT_TOP_TO_BOTTOM);
  break;
I'll be perfectly happy.
Comment 24 Javier Jardón (IRC: jjardon) 2009-10-21 00:48:47 UTC
Comment on attachment 145821 [details] [review]
Reorder the function so the "lookup_context" variable can't be used uninitialized

commit 69773763ee7fbd2c494a5b8c5ff13093715fb788
Comment 25 Javier Jardón (IRC: jjardon) 2009-10-21 00:59:41 UTC
Comment on attachment 145910 [details] [review]
Use switch instead if's to get a default value

commited a version with the switch statements grouped

commit 6f6746389c5d08e1534a7eb04f02e00c5b409e50
Comment 26 Javier Jardón (IRC: jjardon) 2009-10-21 03:22:16 UTC
Created attachment 145915 [details] [review]
use G_GSIZE_FORMAT instead %i
Comment 27 Javier Jardón (IRC: jjardon) 2009-10-21 03:50:43 UTC
Created attachment 145917 [details] [review]
Remove an unused function
Comment 28 Javier Jardón (IRC: jjardon) 2009-10-21 03:51:14 UTC
Created attachment 145918 [details] [review]
Cast to guchar*
Comment 29 Benjamin Otte (Company) 2009-10-21 16:43:07 UTC
Comment on attachment 145918 [details] [review]
Cast to guchar*

If you want to, you can use NULL here instead of (guchar *) ""
Looks a bit better and does the same thing.
Comment 30 Javier Jardón (IRC: jjardon) 2009-10-21 17:39:02 UTC
Comment on attachment 145915 [details] [review]
use G_GSIZE_FORMAT instead %i

commit d5f1b79fabe73345968b6b1f6035a9ba149adc38
Comment 31 Javier Jardón (IRC: jjardon) 2009-10-21 17:40:33 UTC
Comment on attachment 145917 [details] [review]
Remove an unused function

commit d020cb4aa34e786a15a71be72679f37b6164507e
Comment 32 Javier Jardón (IRC: jjardon) 2009-10-21 17:47:18 UTC
Comment on attachment 145918 [details] [review]
Cast to guchar*

committed a version using NULL

commit c43a3acac3d56d7fb67c996450d3d1ca9f7668e7
Comment 33 Javier Jardón (IRC: jjardon) 2009-10-21 21:55:12 UTC
Created attachment 145994 [details] [review]
Cast to gchar*
Comment 34 Javier Jardón (IRC: jjardon) 2009-10-21 22:01:54 UTC
Created attachment 145995 [details] [review]
Cast to gchar*
Comment 35 Benjamin Otte (Company) 2009-10-21 22:03:56 UTC
Review of attachment 145995 [details] [review]:

asdf
Comment 36 Javier Jardón (IRC: jjardon) 2009-10-21 22:11:51 UTC
Comment on attachment 145995 [details] [review]
Cast to gchar*

commit 8e945e0e3f1c7df4cb0873cc248181e03dcdc9ce
Comment 37 Javier Jardón (IRC: jjardon) 2009-10-24 15:12:46 UTC
Created attachment 146175 [details] [review]
Cast to gchar* to fix a compilation warning in io-pnm
Comment 38 Javier Jardón (IRC: jjardon) 2009-10-24 15:13:36 UTC
Created attachment 146176 [details] [review]
Cast to gchar* in io-gif
Comment 39 Benjamin Otte (Company) 2009-10-25 11:55:12 UTC
Comment on attachment 146176 [details] [review]
Cast to gchar* in io-gif

I'm pretty sure this patch is wrong. strcmp() and friends compare the trailing 0 byte to make sure the string is terminated properly, strncmp() doesn't do that. So with this patch the code compares one byte more than previously and this will probably cause it to do bad things.

Also, there is no need to use strcmp0() here as both strings are guaranteed to not be NULL, and that's the only reason to use this function.
Comment 40 Javier Jardón (IRC: jjardon) 2009-10-25 21:14:46 UTC
Comment on attachment 146175 [details] [review]
Cast to gchar* to fix a compilation warning in io-pnm

commit e21355495a056488fdea98372de2754530b3c86a
Comment 41 Javier Jardón (IRC: jjardon) 2009-10-25 21:20:41 UTC
Created attachment 146226 [details] [review]
Cast to gchar* in io-gif
Comment 42 Javier Jardón (IRC: jjardon) 2009-10-25 21:48:34 UTC
Created attachment 146227 [details] [review]
Cast to gchar* in io-gif
Comment 43 Javier Jardón (IRC: jjardon) 2009-10-26 14:15:49 UTC
Comment on attachment 146227 [details] [review]
Cast to gchar* in io-gif

commit 567b23229f78e2ca4d651c3ab60054fda5857d1b
Comment 44 Javier Jardón (IRC: jjardon) 2009-10-29 04:12:32 UTC
Created attachment 146472 [details] [review]
Initialize to false the returned variable
Comment 45 Javier Jardón (IRC: jjardon) 2009-10-29 04:13:33 UTC
Created attachment 146473 [details] [review]
[gdk/x11] Cast to gchar*
Comment 46 Javier Jardón (IRC: jjardon) 2009-10-29 04:14:19 UTC
Created attachment 146474 [details] [review]
[gdk/x11] Fix warning: cast to guint*
Comment 47 Javier Jardón (IRC: jjardon) 2009-10-29 04:15:04 UTC
Created attachment 146475 [details] [review]
[gtk/gtkcolorsel] Fix warning: cast to gchar*
Comment 48 Javier Jardón (IRC: jjardon) 2009-10-29 04:16:07 UTC
Created attachment 146476 [details] [review]
[gtk/gtkmain] Remove statement with no effect
Comment 49 Javier Jardón (IRC: jjardon) 2009-10-29 04:17:07 UTC
Created attachment 146477 [details] [review]
[gtk/gtkobject] Fix warning: cast to gint*
Comment 50 Javier Jardón (IRC: jjardon) 2009-10-29 04:18:07 UTC
Created attachment 146478 [details] [review]
[gtk/gtktext] Fix warning: some casts to gchar*
Comment 51 Javier Jardón (IRC: jjardon) 2009-10-29 04:19:12 UTC
Created attachment 146479 [details] [review]
[gtk/gtkclist] Fix warning: cast to gint8*
Comment 52 Benjamin Otte (Company) 2009-10-29 07:51:48 UTC
Comment on attachment 146472 [details] [review]
Initialize to false the returned variable

I believe the correct initial return value would be TRUE here, no?

Also, could you fix the indentation of that function while you're at it?
Comment 53 Benjamin Otte (Company) 2009-10-29 07:59:19 UTC
Comment on attachment 146474 [details] [review]
[gdk/x11] Fix warning: cast to guint*

I'd much prefer if you fixed the prototypes of MyEnhancedXkbTranslateKeyCode()and translate_keysym() to take a gint* because those functions treat these arguments as an int anyway. And they're only called from here, too. So we get around the need to cast.
Comment 54 Benjamin Otte (Company) 2009-10-29 08:03:59 UTC
Comment on attachment 146475 [details] [review]
[gtk/gtkcolorsel] Fix warning: cast to gchar*

Could you add a patch that removes the workaround below where the dash-offset gets made positive?
This is fixed since Cairo 0.9.3, and Gtk requires Cairo 1.6 these days.
Comment 55 Benjamin Otte (Company) 2009-10-29 08:06:27 UTC
Comment on attachment 146477 [details] [review]
[gtk/gtkobject] Fix warning: cast to gint*

This patch doesn't match the description. In fact, it's the previous patch again.

If you used git-bz here, please point Owen to this bug.
Comment 56 Benjamin Otte (Company) 2009-10-29 08:08:35 UTC
Comment on attachment 146479 [details] [review]
[gtk/gtkclist] Fix warning: cast to gint8*

can you instead create do something like
gint8 dashes[] = { 4, 4 };
at the beginning of the if and then call
gdk_gc_set_dashes (..., dashes, G_N_ELEMENTS (dashes));

That should be clearer.
Comment 57 Javier Jardón (IRC: jjardon) 2009-10-30 03:05:19 UTC
Comment on attachment 146473 [details] [review]
[gdk/x11] Cast to gchar*

commit d76287c94b3b6a111eb6bf84315ba9c10ff3eb7f
Comment 58 Javier Jardón (IRC: jjardon) 2009-10-30 03:16:23 UTC
Created attachment 146544 [details] [review]
[gdk/x11/gdkkeys-x11] Fix some compilation warnings
Comment 59 Javier Jardón (IRC: jjardon) 2009-10-30 03:18:06 UTC
Comment on attachment 146475 [details] [review]
[gtk/gtkcolorsel] Fix warning: cast to gchar*

commit cbba733b47424cfe44f7a358317db14a406d2374
Comment 60 Javier Jardón (IRC: jjardon) 2009-10-30 03:19:37 UTC
Comment on attachment 146476 [details] [review]
[gtk/gtkmain] Remove statement with no effect

commit 07236ee745c06c4dbb210d239bf188cb8dc3550a
Comment 61 Javier Jardón (IRC: jjardon) 2009-10-30 03:24:08 UTC
Created attachment 146546 [details] [review]
[gtk/gtkobject] Fix warning: cast to gint*

Sorry, here the correct patch
Comment 62 Javier Jardón (IRC: jjardon) 2009-10-30 03:28:28 UTC
Comment on attachment 146478 [details] [review]
[gtk/gtktext] Fix warning: some casts to gchar*

commit 0a9843c1b9d7b58adb2690630d704ad524011a3c
Comment 63 Javier Jardón (IRC: jjardon) 2009-10-30 03:51:34 UTC
Created attachment 146548 [details] [review]
[gtk/gtkclist] Use proper gint8 array variable instead a string
Comment 64 Javier Jardón (IRC: jjardon) 2009-10-30 17:34:02 UTC
Comment on attachment 146544 [details] [review]
[gdk/x11/gdkkeys-x11] Fix some compilation warnings

commit 3dbfc08a7a219db290b049a0788db519fcbcb35d
Comment 65 Javier Jardón (IRC: jjardon) 2009-10-30 17:35:17 UTC
Comment on attachment 146546 [details] [review]
[gtk/gtkobject] Fix warning: cast to gint*

commit 8763551b6b0035d6ecc762368c838507dccbbbeb
Comment 66 Javier Jardón (IRC: jjardon) 2009-10-30 17:36:34 UTC
Comment on attachment 146548 [details] [review]
[gtk/gtkclist] Use proper gint8 array variable instead a string

commit 550796d897380d64675373d763275d69eefbcad1
Comment 67 Javier Jardón (IRC: jjardon) 2009-10-30 18:24:53 UTC
Comment on attachment 146472 [details] [review]
Initialize to false the returned variable

commit 141a7dac000a5cc4c103e614291021f0b292660c
Comment 68 Javier Jardón (IRC: jjardon) 2009-11-02 02:36:35 UTC
Created attachment 146711 [details] [review]
Use gpointer
Comment 69 Javier Jardón (IRC: jjardon) 2009-11-02 02:37:16 UTC
Created attachment 146712 [details] [review]
Cast to guint8*
Comment 70 Benjamin Otte (Company) 2009-11-02 09:02:19 UTC
Comment on attachment 146712 [details] [review]
Cast to guint8*

If this wasn't deprecated code, I'd have asked to check if it wasn't better to use int variables instead of guint for the last two chunks of the patch, but now I won't care.

The guint8* casts are perfectly fine in any case.
Comment 71 Javier Jardón (IRC: jjardon) 2009-11-02 22:49:29 UTC
Comment on attachment 146711 [details] [review]
Use gpointer

commit 1a02671437369f8b07c7eb0bfaa3de98f41f1bfc
Comment 72 Javier Jardón (IRC: jjardon) 2009-11-02 23:13:59 UTC
Comment on attachment 146712 [details] [review]
Cast to guint8*

Comitted a modified version of this patch: I use guint directly instead do a cast

commit 8dfac859a15ea90c3015c510247dd3113c14dc26
Comment 73 Javier Jardón (IRC: jjardon) 2009-11-09 03:10:14 UTC
Created attachment 147236 [details] [review]
Cast to guchar*
Comment 74 Javier Jardón (IRC: jjardon) 2009-11-09 03:10:56 UTC
Created attachment 147237 [details] [review]
Cast to gchar*
Comment 75 Javier Jardón (IRC: jjardon) 2009-11-09 03:11:59 UTC
Created attachment 147238 [details] [review]
Use const gchar
Comment 76 Javier Jardón (IRC: jjardon) 2009-11-09 03:14:42 UTC
Created attachment 147239 [details] [review]
Remove unused gtk_text_btree_node_invalidate_downwar() function
Comment 77 Javier Jardón (IRC: jjardon) 2009-11-09 03:15:35 UTC
Created attachment 147240 [details] [review]
Use GINT_TO_POINTER() and GPOINTER_TO_INT()
Comment 78 Javier Jardón (IRC: jjardon) 2009-11-09 03:16:19 UTC
Created attachment 147241 [details] [review]
Use the proper G*_TO_POINTER macro
Comment 79 Javier Jardón (IRC: jjardon) 2009-11-09 05:46:08 UTC
Created attachment 147242 [details] [review]
Initialize variable to 0
Comment 80 Javier Jardón (IRC: jjardon) 2009-11-09 05:46:53 UTC
Created attachment 147243 [details] [review]
Disable test until it works
Comment 81 Javier Jardón (IRC: jjardon) 2009-11-09 05:47:29 UTC
Created attachment 147244 [details] [review]
Initialize variable to NO_MATCH
Comment 82 Javier Jardón (IRC: jjardon) 2009-11-09 05:48:37 UTC
Created attachment 147245 [details] [review]
Use parenthesis in a expression with '&&' and '||'
Comment 83 Benjamin Otte (Company) 2009-11-09 08:22:26 UTC
Comment on attachment 147244 [details] [review]
Initialize variable to NO_MATCH

First the easy comment: I would omit the first chunk of the patch as it's much clearer to say "return NO_MATCH" than "return result", even if you know that result == NO_MATCH.

Now for the second part: I'm pretty sure the logic here is not completely right. If we get a common_prefix and it is_complete_not_unique, you set result = COMPLETED unconditionally, but the old code keeps a value of COMPLETE_BUT_NOT_UNIQUE.

I like your idea of unifying the have_result and result variables, but I'm not sure how to best do that in this convoluted code. So I would probably just initialize the variable to NO_MATCH and be done with it.
Comment 84 Benjamin Otte (Company) 2009-11-09 08:24:40 UTC
Comment on attachment 147239 [details] [review]
Remove unused gtk_text_btree_node_invalidate_downwar() function

You should ask Kris if it's ok to delete the code. If Kris isn't there, just #if 0 the code. Oftentimes people keep code in because they think it might be needed later and then they'll be surprised when its gone.
Comment 85 Javier Jardón (IRC: jjardon) 2009-11-09 08:44:16 UTC
Comment on attachment 147236 [details] [review]
Cast to guchar*

commit 73cfcf94953359dc781f30f94eef15593000a2e5
Comment 86 Javier Jardón (IRC: jjardon) 2009-11-09 08:45:00 UTC
Comment on attachment 147242 [details] [review]
Initialize variable to 0

commit e70a46c7e3266e38294ac49e97646d99ab4c83a9
Comment 87 Javier Jardón (IRC: jjardon) 2009-11-09 08:45:49 UTC
Comment on attachment 147243 [details] [review]
Disable test until it works

commit c7a5ddf8a7250ae549995508dda894544435f2eb
Comment 88 Javier Jardón (IRC: jjardon) 2009-11-09 08:47:03 UTC
Comment on attachment 147237 [details] [review]
Cast to gchar*

commit ef414cc6bf01adf667e036ec1d00e2ad15d98a74
Comment 89 Javier Jardón (IRC: jjardon) 2009-11-09 08:47:40 UTC
Comment on attachment 147238 [details] [review]
Use const gchar

commit d8f5ce4b545250a5c8dd50f21d031999b348a8e3
Comment 90 Javier Jardón (IRC: jjardon) 2009-11-09 08:48:36 UTC
Comment on attachment 147245 [details] [review]
Use parenthesis in a expression with '&&' and '||'

commit 8831a4490264d9ac6ecac8014a066fabd2fbdc6e
Comment 91 Javier Jardón (IRC: jjardon) 2009-11-09 08:49:18 UTC
Comment on attachment 147240 [details] [review]
Use GINT_TO_POINTER() and GPOINTER_TO_INT()

commit 20f123531a70bce7c6b96b788aa22d7d1551d392
Comment 92 Javier Jardón (IRC: jjardon) 2009-11-09 08:51:06 UTC
Comment on attachment 147244 [details] [review]
Initialize variable to NO_MATCH

Committed a version of this patch only with this change, as you requested:

-  CommonPrefixResult result;
+  CommonPrefixResult result = NO_MATCH;

commit 1919f55647007d1ca1f9fd38b5108e0adb54ea30
Comment 93 Benjamin Otte (Company) 2009-11-09 09:10:11 UTC
Comment on attachment 147241 [details] [review]
Use the proper G*_TO_POINTER macro

if you remove the (void *) casts, it'll be perfect.
Comment 94 Javier Jardón (IRC: jjardon) 2009-11-09 09:29:02 UTC
Comment on attachment 147241 [details] [review]
Use the proper G*_TO_POINTER macro

commit f1f6d76078dccd1c7d7c305a45c270451ead6612
Comment 95 Javier Jardón (IRC: jjardon) 2009-11-09 19:42:44 UTC
Created attachment 147317 [details] [review]
Use G_POINTER_TO_UINT
Comment 96 Benjamin Otte (Company) 2009-11-09 20:10:43 UTC
Review of attachment 147317 [details] [review]:

Dear Splinter, please accept empty reviews when one changes the patch status. Thank you.
Comment 97 Javier Jardón (IRC: jjardon) 2009-11-09 20:11:34 UTC
Comment on attachment 147239 [details] [review]
Remove unused gtk_text_btree_node_invalidate_downwar() function

Use #if 0 instead remove the code

commit 3143a344a76ebaf9e931c00efd8b797074d56542
Comment 98 Javier Jardón (IRC: jjardon) 2009-11-09 20:19:04 UTC
Comment on attachment 147317 [details] [review]
Use G_POINTER_TO_UINT

commit 78151dc7d41069e876fe165527e68c238ef7fd4a
Comment 99 Javier Jardón (IRC: jjardon) 2009-11-09 20:52:59 UTC
Created attachment 147322 [details] [review]
Use G_GNUC_UNUSED macro
Comment 100 Benjamin Otte (Company) 2009-11-09 20:55:41 UTC
Review of attachment 147322 [details] [review]:

looks good
Comment 101 Javier Jardón (IRC: jjardon) 2009-11-09 20:59:17 UTC
Comment on attachment 147322 [details] [review]
Use G_GNUC_UNUSED macro

commit 21f6e1841ae1546cf109ed83ff0fbb705212b29d
Comment 102 Javier Jardón (IRC: jjardon) 2009-11-09 21:42:58 UTC
Created attachment 147328 [details] [review]
Use a variable to store the return value
Comment 103 Benjamin Otte (Company) 2009-11-09 21:52:00 UTC
Review of attachment 147328 [details] [review]:

r=me
Comment 104 Javier Jardón (IRC: jjardon) 2009-11-09 21:55:06 UTC
Comment on attachment 147328 [details] [review]
Use a variable to store the return value

commit cb301e6dbbe8c4017c7022995976b577796cf903
Comment 105 Javier Jardón (IRC: jjardon) 2009-12-02 08:29:46 UTC
Created attachment 148881 [details] [review]
Remove useless line
Comment 106 Javier Jardón (IRC: jjardon) 2009-12-02 08:30:32 UTC
Created attachment 148882 [details] [review]
Use {} in empty if..else blocks
Comment 107 Benjamin Otte (Company) 2009-12-02 08:37:04 UTC
Comment on attachment 148882 [details] [review]
Use {} in empty if..else blocks

Could you close this bug when you've committed the fixes? This bug is getting crowded.
Feel free to open a new one for the next sets of compiler warnings and cc me there.
(It will increase our bugzilla score, too!)
Comment 108 Javier Jardón (IRC: jjardon) 2009-12-02 08:39:12 UTC
Comment on attachment 148881 [details] [review]
Remove useless line

commit f35c581de43532c493439c05ad6c1b19942e000e
Comment 109 Javier Jardón (IRC: jjardon) 2009-12-02 08:39:52 UTC
Comment on attachment 148882 [details] [review]
Use {} in empty if..else blocks

commit 62bfbe91d89bf7c4c91bd4384649c01dccde2b74
Comment 110 Javier Jardón (IRC: jjardon) 2009-12-02 08:48:20 UTC
(In reply to comment #107)
> (From update of attachment 148882 [details] [review])
> Could you close this bug when you've committed the fixes? This bug is getting
> crowded.
> Feel free to open a new one for the next sets of compiler warnings and cc me
> there.
> (It will increase our bugzilla score, too!)

Sure, 

I'll open a new bug If I catch more compilation warnings, but I'd like to keep this open until the gtk/gtkimcontextsimpleseqs.h downcasting problem was solved.

As always, thank you again for your reviews ;)
Comment 111 Javier Jardón (IRC: jjardon) 2010-03-09 16:17:32 UTC
As bug #601234 is now  fixed, we can close this bug because there is not compilation warnings in GTK+ now :)