GNOME Bugzilla – Bug 641838
More compiler warning fixes
Last modified: 2013-09-14 16:54:01 UTC
Created attachment 180386 [details] [review] patch for warnings Attaching a patch
Thanks for a bug report and patch. I'm basically for it, fixing return values is fine, only do not replace e_return_error_if_fail with g_return_val_if_fail in e-book.c. Please keep there that e_return_error_if_fail. In e-data-cal-view.c I suppose the GList was meant to not have the comma in the initializer, in that case the whole structure is filled with zeros, which is what is supposed to happen. Could you change these two things and commit, please? If you do not have time then just let me know and I'll do it for you (I only do not have similar environment as you, to ensure my proposed changes to your patch will address all the warnings the same way). Thanks in advance.
The problem with e_return_error_if_fail in e-book.c:2751 is that the function is supposed to return a pointer, but instead returns an error code as an enum. Should it be G_INT_TO_POINTER(E_BOOK_ERROR_INVALID_ARG) for example? And changing the GList initializer to GList l = {0} still complains about using an integer in place of a pointer. e-data-cal-view.c:640:20: warning: Using plain integer as NULL pointer
The warning about e_return_error_if_fail is here: e-book.c:3291:9: warning: Using plain integer as NULL pointer
I took care of the e-book.c issues in: http://git.gnome.org/browse/evolution-data-server/commit/?id=3f8c77e4d8ac24adb677441dc8b16c2a258dd018
(In reply to comment #2) > The problem with e_return_error_if_fail in e-book.c:2751 is that the function > is supposed to return a pointer, but instead returns an error code as an enum. > Should it be G_INT_TO_POINTER(E_BOOK_ERROR_INVALID_ARG) for example? And > changing the GList initializer to GList l = {0} still complains about using an > integer in place of a pointer. > > e-data-cal-view.c:640:20: warning: Using plain integer as NULL pointer Strange, I expected it as a structure initializer. As we depend on a stack-allocated GList, then you may name all members of it pretty safely, I believe. The only thing I do not like on the previous version is that the structure initialized ends with a comma. I remember you've been fixing similar things in enums, so having it here and not there is pretty confusing to me.
Review of attachment 180386 [details] [review]: --- a/camel/camel-file-utils.c +++ b/camel/camel-file-utils.c @@ -321,7 +321,7 @@ camel_file_util_decode_string (FILE *in, gchar **str) gint camel_file_util_encode_fixed_string (FILE *out, const gchar *str, gsize len) { - gchar buf[len]; + gchar buf[sizeof(gsize)]; /* Don't allow empty strings to be written */ if (len < 1) This part is wrong, but it revealed a larger issue which I fixed in a separate commit: http://git.gnome.org/browse/evolution-data-server/commit/?id=af5ee3533a3ff45c7ac30d58968e0499c72a9e04
Committed the remaining patch hunks for 2.91.92: http://git.gnome.org/browse/evolution-data-server/commit/?id=38dceb9e2b3238b87db705d1af2d773f4c0c0f52
landed on inbound http://hg.mozilla.org/integration/mozilla-inbound/rev/1140e223915f
(In reply to comment #8) > landed on inbound > http://hg.mozilla.org/integration/mozilla-inbound/rev/1140e223915f sorry about that, wrong bugzilla / bug obviously