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 699147 - Fixes signedness mismatches.
Fixes signedness mismatches.
Status: RESOLVED FIXED
Product: seahorse
Classification: Applications
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: Seahorse Maintainer
Seahorse Maintainer
Depends on:
Blocks:
 
 
Reported: 2013-04-28 15:07 UTC by Alban Browaeys
Modified: 2013-05-27 09:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fixes signedness mismatches. (13.56 KB, patch)
2013-04-28 15:07 UTC, Alban Browaeys
needs-work Details | Review
Fixes signedness mismatches and length check. (13.21 KB, patch)
2013-05-25 09:28 UTC, Alban Browaeys
committed Details | Review

Description Alban Browaeys 2013-04-28 15:07:05 UTC
Created attachment 242721 [details] [review]
Fixes signedness mismatches.

The previous fix for CFLAGS enabled various warning flags . Those are fixes for all the signedness mismatches that were revealed..
Comment 1 Stef Walter 2013-04-28 18:03:32 UTC
Review of attachment 242721 [details] [review]:

Thanks for the patch. Could you make the following changes and fixes before it goes in?

::: libseahorse/seahorse-object-model.c
@@ +312,3 @@
     GtkTreePath *path;
     GtkTreePath *ipath;
+    guint i;

This change makes the (i--) code below rely on rolling over from zero to G_MAXUINT and then back again. Is this defined behavior (on all platforms, ie: in the C standard)?

::: libseahorse/seahorse-util.c
@@ +225,3 @@
 seahorse_util_read_to_memory (GInputStream *input, guint *len)
 {
+	guint size = 128;

Shouldn't this be a gsize?

::: pgp/seahorse-gpgme-generate.c
@@ +299,3 @@
     sel = gtk_combo_box_get_active (GTK_COMBO_BOX (widget));
+    num_algorithms = G_N_ELEMENTS(available_algorithms);
+    g_assert (sel <= num_algorithms);

This seems rather pointless extra variable. Why not just use a cast?

::: pgp/seahorse-gpgme-photos.c
@@ +93,1 @@
     int written;

Shouldn't written be a gssize? And again, why are we using another variable here?

::: pgp/seahorse-pgp-key-properties.c
@@ +1531,3 @@
     SeahorseWidget *swidget = SEAHORSE_WIDGET (user_data);
     GObject *object;
+    gint trust;

I think this should be a SeahorseValidity

::: ssh/seahorse-ssh-key-data.c
@@ +309,2 @@
 SeahorseSSHKeyData*
+seahorse_ssh_key_data_parse_line (const gchar *line, gint length)

Can we use gssize and then not have to use the temporary variable?

::: ssh/seahorse-ssh-operation.c
@@ +412,3 @@
+		  closure->sin = g_string_new_len (input, strlen(input));
+                else
+		  closure->sin = g_string_new_len (input, length);

Ditto about gssize.
Comment 2 Alban Browaeys 2013-05-25 09:28:33 UTC
Created attachment 245287 [details] [review]
Fixes signedness mismatches and length check.

> :: libseahorse/seahorse-object-model.c
> @@ +312,3 @@
>      GtkTreePath *path;
>      GtkTreePath *ipath;
> +    guint i;
> 
> This change makes the (i--) code below rely on rolling over from zero to
> G_MAXUINT and then back again. Is this defined behavior (on all platforms, ie:
> in the C standard)?

g_ptr_array_index expects a guint . so even if there is i-- in libseahorse/seahorse-object-model.c line 353 I guess it was not intended to go below zero.

Other points I have applied and compiled. Indeed better. Thanks.
Comment 3 Stef Walter 2013-05-27 09:16:07 UTC
Attachment 245287 [details] pushed as 0688d47 - Fixes signedness mismatches and length check.