[Dovecot] [PATCH] fix gcc warnings in 1.0rc26

[Timo Sirainen]

On Thu, 2007-03-15 at 11:46 +0100, Max Kellermann wrote:
> Some of the changes may be arguable, escpecially the const warning
> fixes, example:
> 
>  char *pass = "";
> 
> I changed that to "pass = NULL" and changed the function call:
> 
>  - auth_request_verify_plain(request, pass,
>  + auth_request_verify_plain(request, pass == NULL ? "" : pass,

That's ok, but I'm not sure about bsearch_insert_pos(). It's the way it
is mostly because I wanted to keep bsearch() API. If it can't return
void * then maybe it could be easier to just change the whole API to
something like:

/* If key is found, returns TRUE and sets pos_r to the position where
the key
   was found. If key isn't found, returns FALSE and sets pos_r to the
position
   where the key should be inserted. */
bool bsearch_insert_pos(const void *key, const void *base, unsigned int
nmemb,
			size_t size, int (*cmp)(const void *, const void *),
			unsigned int *pos_r);

Because that's how it's usually used anyway, so it probably makes the
code simpler also. Hmm. And maybe s/pos/idx/ :)

> In other sources, I added "const" to parameters or to the return value
> where it seemed to make sense and didn't cause harm.  Dealing with the
> const problem can be tricky, and sometimes dirty deconstification
> hacks are the only way to go..  however I believe we should always
> turn on "-Wconst" and explicitly add manual deconstitication where we
> assume it's not dangerous.

You mean -Wwrite-strings I guess? Looks like I've added these comments
about various gcc warning options before:

# -Wcast-qual -Wcast-align -Wconversion # too many warnings
# -Wstrict-prototypes -Wredundant-decls # may give warnings in some
systems
# -Wmissing-format-attribute -Wmissing-noreturn -Wwrite-strings # a
couple of warnings

I guess those "warnings in some systems" warnings could be enabled by
default and then disabled on those specific systems.

-Wwrite-strings would be nice as default too..

> I didn't fix all "const" and "missing noreturn" warnings, but my
> patches may be a good start...  if there is further interest in fixing
> all of them, I'd be glad to do it.

I've interest, but I don't think these changes should be made to v1.0
tree anymore. But for CVS HEAD, sure.

I've also enabled -Wstrict-aliasing=2 in CVS HEAD, which is now giving
several warnings. I should either fix those somehow or disable the
warning after all..

I'll see about applying your patches (minus bsearch) to CVS HEAD. Or I
wouldn't mind getting the patches directly against it either :)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://dovecot.org/pipermail/dovecot/attachments/20070315/842cdafe/attachment.pgp