Re: [evolution-patches] One big patch to rule them all (compilation warnings in camel)

[Philip Van Hoof <spam pvanhoof be>]

On Thu, 2006-10-19 at 12:13 +0200, Philip Van Hoof wrote:
> Folks, 

While doing this, I spotted a bug that really shouldn't happen.

An integer isn't always defaulted to 0 after initialisation. The
developer has to explicitly write that. Depending on your platform and C
compiler, a number will be allocated with 0 or a random number on the
stack.

Thus introducing a race condition when creating a flags variable this
way:

CamelStream *
camel_stream_vfs_new_with_uri (const char *name, int flags, mode_t mode)
{
	GnomeVFSResult result;
	GnomeVFSHandle *handle;
	int vfs_flag;

	if (flags & O_WRONLY)
		vfs_flag = vfs_flag | GNOME_VFS_OPEN_WRITE;

By simply looking at the compilation warnings, and there's not going to
be as much warnings as there used to be anymore if people accept my work
on this, this could have been avoided.

It should be avoided because this type of bugs are very hard to find,
yet it's simple to look at compiler warnings (and gcc does warn about it
if you set it to warn about things like this).

This should probably have been "int vfs_vlag = 0". 4 bytes, but an
important, difference.


I hope bugs like this show that fixing warnings *is* important.


> To show my intentions are good, because a lot people seem to actually
> question that, I will create a patch that contains all compilation
> warning fixes for Camel.
> 
> Well, not immediately all of them because some of my work isn't easy to
> extract a patch for (because I have a few significant other changes in
> my copy too).
> 
> I will post this as a enhancement on the bugzilla, here, will put
> Matthew in CC add a ChangeLog, I'll make sure it's tested and feature
> complete (it'll be one big patch).
> 
> I will also fix the problems Jeffrey raised. About the coding style. In
> fact, that is already done (using this script which I ran over the
> existing diffs):
> 
> while (<STDIN>)
> {
>         s/(\*\))([^\s])/$1 $2/g;
>         s/char\*/char \*/g;
>         s/guchar/unsigned gchar/g;
>         s/guint32/unsigned gint32/g;
>         print;
> }
> 
> Hold on tight, it's coming to a mailing list and bugzilla near you.
> 
> 
-- 
Philip Van Hoof, software developer
home: me at pvanhoof dot be
gnome: pvanhoof at gnome dot org
work: vanhoof at x-tend dot be
blog: http://pvanhoof.be/blog