fetchmail-devel

[fetchmail-devel] [PATCHes] Misc. cleanups

[Miloslav T. <mi...@re...>]

Hello,
as usual I have choosen the perfect time for cleanup patches :)

The patches avoid most warnings with gcc -Wall -Wextra:
- fetchmail-signed.patch: Avoid implicit conversions between
  char * and unsigned char *; they are not defined by ISO C and gcc
  currently warns about them.  Most places were changed to use "char *",
  or "void *" where general binary data is handled.
- fetchmail-comparisons.patch: Change variable types or add explicit
  casts to make all casts use types with consistent type signedness,
  for clarity
- fetchmail-clean.patch: Remove or (void) out unused parameters, avoid
  assignments directly in if ().

- fetchmail-lmtp.patch: A real bug detected by the compiler, "," has
  lower priority than "||".

Thanks,
	Mirek

Re: [fetchmail-devel] [PATCHes] Misc. cleanups

[Nico G. <ni...@ng...>]

Hi,
* Miloslav Trmac <mi...@re...> [2006-03-12 01:25]:
[...] 
> -    if (retval = krb5_cc_get_principal(context, ccdef, &client)) {
> +    if ((retval = krb5_cc_get_principal(context, ccdef, &client)) != 0) {

[...] 
What is your point here? I can't see a real reason to do 
this because its the same.
Regards Nico
-- 
Nico Golde - JAB: ni...@ja... | GPG: 0x73647CFF
http://www.ngolde.de | http://www.muttng.org | http://grml.org
Forget about that mouse with 3/4/5 buttons -
gimme a keyboard with 103/104/105 keys!

Re: [fetchmail-devel] [PATCHes] Misc. cleanups

[Miloslav T. <mi...@re...>]

Nico Golde napsal(a):
> Hi,
> * Miloslav Trmac <mi...@re...> [2006-03-12 01:25]:
> [...] 
>> -    if (retval = krb5_cc_get_principal(context, ccdef, &client)) {
>> +    if ((retval = krb5_cc_get_principal(context, ccdef, &client)) != 0) {
> 
> [...] 
> What is your point here? I can't see a real reason to do 
> this because its the same.
No real change, just avoiding gcc warnings to better see the warnings
that detect real bugs.
	Mirek

[fetchmail-devel] Re: Misc. cleanups

[Sunil S. <sh...@bo...>]

Quoting from Nico Golde's mail on Sun, Mar 12, 2006:
> * Miloslav Trmac <mi...@re...> [2006-03-12 01:25]:
> [...] 
> > -    if (retval = krb5_cc_get_principal(context, ccdef, &client)) {
> > +    if ((retval = krb5_cc_get_principal(context, ccdef, &client)) != 0) {
> 
> [...] 
> What is your point here? I can't see a real reason to do 
> this because its the same.

The C construct

    if (var = expression) { ... }

is valid, but is also a common typo. The actual intention may be to
write it as:

    if (var == expression) { ... }

To emphasise that it is not a typo, it is preferred to write the first
expression above in one of the following formats:

    if ((var = expression) != 0) { ... }
    if ((var = expression)) { ... }

So, it's the same, but it's different.

-- 
Sunil Shetye.

Re: [fetchmail-devel] [PATCHes] Misc. cleanups

[Matthias A. <mat...@gm...>]

Nico Golde <ni...@ng...> writes:

> Hi,
> * Miloslav Trmac <mi...@re...> [2006-03-12 01:25]:
> [...] 
>> -    if (retval = krb5_cc_get_principal(context, ccdef, &client)) {
>> +    if ((retval = krb5_cc_get_principal(context, ccdef, &client)) != 0) {
>
> [...] 
> What is your point here? I can't see a real reason to do 
> this because its the same.

Yes, but it causes GCC warnings and who says it shouldn't have been
retval == ... ? I'll see if I take Mirek's patch or just write

       retval = krb5..;
       if (retval) { ... }

when reviewing all of it. 

BTW, Mirek, thanks for your detail work.

The warnings have been annoying me for ages but I didn't have the time
to fix them.

-- 
Matthias Andree

Re: [fetchmail-devel] [PATCHes] Misc. cleanups

[Matthias A. <mat...@gm...>]

Miloslav Trmac <mi...@re...> writes:

> - fetchmail-signed.patch: Avoid implicit conversions between
>   char * and unsigned char *; they are not defined by ISO C and gcc
>   currently warns about them.  Most places were changed to use "char *",
>   or "void *" where general binary data is handled.

The report.c patch appears wrong, it has to be n >= 0 rather than n < 0,
and needs to be changed in both branches of each of the two #ifdef.

What is the reason for the (int32) cast in kerberos.c near line 220?

> - fetchmail-comparisons.patch: Change variable types or add explicit
>   casts to make all casts use types with consistent type signedness,
>   for clarity
> - fetchmail-clean.patch: Remove or (void) out unused parameters, avoid
>   assignments directly in if ().
>
> - fetchmail-lmtp.patch: A real bug detected by the compiler, "," has
>   lower priority than "||".

Other than that, I merged your patches.

Thanks a lot.

-- 
Matthias Andree