Re: [evolution-patches] Cleanups for evolution/calendar

[Kjartan Maraas <kmaraas broadpark no>]

ons, 16,.08.2006 kl. 13.01 -0400, skrev Joe Shaw:
> Hi,
> 
> On Wed, 2006-08-16 at 18:22 +0200, Kjartan Maraas wrote:
> > > > > On Wed, 2006-08-16 at 16:53 +0200, Kjartan Maraas wrote:
> > > > > > -               d (printf("%s:%d (list_changed_cb) - Removing Calendar %s\n", __FILE__, __LINE__, l->data));            
> > > > > > +               d (printf("%p:%d (list_changed_cb) - Removing Calendar %s\n", __FILE__, __LINE__, l->data));            
> >
> > I'm a bit baffled by the use of %d for all these structs and pointers.
> > If the debug output is supposed to show strings to the user they should
> > all be %s and casted to char * I guess?
> 
> %d is only used for the second value, __LINE__, which is a special
> preprocessor macro set by the compiler.  My understanding is that
> __LINE__ is just an integer value.  Hence the use of %d.
> 
> Likewise, __FILE__ is set in the same way, but it is a string constant.
> Thus, %s.
> 
> The warning is actually for "%s" but references the 4th argument (the
> format string first, __FILE__ second, __LINE__ third, l->data fourth).
> The issue is that l->data is defined as a gpointer, but %s implies char
> *.
> 
> l is an element in a GList or GSList and it was designed to be as
> generic as possible.  That's why the data value is defines as a
> gpointer: you can store a pointer to a string, pointer to a structure,
> even an integer casted to a pointer in there.  But the compiler doesn't
> know exactly what it is and can't automatically cast from a gpointer to
> the appropriate type.

Yeah, I follow that.

> Therefore, it's important for the developer to know what type l->data
> really is.  I presume it's a string since the %s was already in the
> format specifier, so just casting l->data to char * is probably the
> right thing.  Otherwise someone probably would have noticed garbage
> being printed on the console by now.
> 
Looking through the rest of the file almost all debug printouts use %d
and try to print out pointers to structs, strings and so on. I'm not
sure what info is really wanted there so maybe the people who added the
debug statements in the alarm code could chime in and reveal what was
intended here? :-)

Cheers
Kjartan