Re: [evolution-patches] Sparse warning cleanups for e-d-s



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]