Re: [evolution-patches] Sparse warning cleanups for e-d-s
- From: Matthew Barnes <mbarnes redhat com>
- To: evolution-patches gnome org
- Subject: Re: [evolution-patches] Sparse warning cleanups for e-d-s
- Date: Mon, 07 May 2007 12:31:45 -0400
On Fri, 2007-04-20 at 16:25 +0200, Kjartan Maraas wrote:
> Patch attached.
>
> - rename variables shadowing others
> - uninitialized vars
> - missing declarations
> - static markers
> - remove some unused bits
> - const correctness
> - mixing enums
> - if vs ifdef
> - add comments for some issues that should be discussed
Thanks Kjartan!
I reviewed this in detail and committed it as revision 7728.
Here's what I did to address the issues you flagged:
* camel/camel-stream-vfs.c
/* Is this going to work? The handle is a struct */
if (stream_vfs->handle != -1)
gnome_vfs_close (stream_vfs->handle);
It looks like -1 is being used as a special bit pattern to
tell
whether the "handle" pointer was touched during the object's
lifetime. Having a separate "handle_needs_closed" boolean
might
be better but would break ABI, so I just wrapped the -1 in a
GINT_TO_POINTER() macro both here and in the constructor.
* addressbook/libebook/e-book.c
/* Move this to e-book.h so it's declared properly? */
GMainContext *_ebook_context;
I'd say no because the underscore indicates this variable is
not
meant to be exported. I left it alone, but a better idea
might be
to start a e-book-private.h file for sharing stuff like this.
* addressbook/libebook/e-book.c
/* This returns FALSE not NULL. Open code this instead? */
e_return_error_if_fail (uri, E_BOOK_ERROR_INVALID_ARG);
Two cases of this programming error. I changed the macro to
take
a third "return value" argument, and then adjusted all the
other
places where this macro is used.
Matthew Barnes
[Date Prev][
Date Next] [Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]