dovecot - Mar 2007 - [PATCH] fix gcc warnings in 1.0rc26

Hi,

I have made a small patchset to fix some of the GCC warnings.  With
these patches, I was able to compile dovecot r_1_0_rc26 with the
following options:

 -Wall -W -Wmissing-prototypes -Wmissing-declarations -Wpointer-arith
 -Wchar-subscripts -Wformat=2 -Wbad-function-cast Wstrict-prototypes
 -Wredundant-decls -Wcast-align -Wwrite-strings Wpacked -Winline
 -Wdisabled-optimization -Wno-long-long -Wundef Wfloat-equal
 -Wsign-compare -Waggregate-return -Werror

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,

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.

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.

Max

-------------- next part --------------
A non-text attachment was scrubbed...
Name: dovecot-imap_bodystructure_write_const.patch
Type: text/x-diff
Size: 1806 bytes
Desc: not available
URL:
<http://dovecot.org/pipermail/dovecot/attachments/20070315/53f1ec71/attachment-0008.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dovecot-gcc_warnings.patch
Type: text/x-diff
Size: 1133 bytes
Desc: not available
URL:
<http://dovecot.org/pipermail/dovecot/attachments/20070315/53f1ec71/attachment-0009.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dovecot-noreturn.patch
Type: text/x-diff
Size: 2235 bytes
Desc: not available
URL:
<http://dovecot.org/pipermail/dovecot/attachments/20070315/53f1ec71/attachment-0010.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dovecot-const_warnings.patch
Type: text/x-diff
Size: 11484 bytes
Desc: not available
URL:
<http://dovecot.org/pipermail/dovecot/attachments/20070315/53f1ec71/attachment-0011.bin>

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: signature.asc
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.bin>