pidgin: 594499c2: a patch from eperez that corrects EOF de...

[Mark Schneider]

2008/12/18 Ethan Blanton <elb at pidgin.im>:
> Eduardo Pérez Ureta spake unto us the following wisdom:
>> >> ============================================================
>> >> --- libpurple/protocols/msn/soap.c      a0d0310469dff2036b06d60b54b06db0b11ea595
>> >> +++ libpurple/protocols/msn/soap.c      471109ac485058a9b0dce16f874080883b214c89
>> >> @@ -521,7 +521,7 @@ msn_soap_read_cb(gpointer data, gint fd,
>> >>        /* msn_soap_process could alter errno */
>> >>        msn_soap_process(conn);
>> >>
>> >> -       if (cnt < 0 && perrno != EAGAIN) {
>> >> +       if (cnt < 0 && perrno != EAGAIN || cnt == 0) {
>> >>                /* It's possible msn_soap_process closed the ssl connection */
>> >>                if (conn->ssl) {
>> >>                        purple_ssl_close(conn->ssl);
>> >> ============================================================
>> > <snip>
>> >
>> > Sorry if i'm being picky, but i think it would be more clear if that were:
>> > if (cnt <= 0 && perrno != EAGAIN) {
>>
>> In most cases you should not even check the value of perrno when the
>> returned value (cnt) equals 0 as it is supposed to be undefined.
>>
>> I think my original patch is the most clear.
>
> Eduardo is correct; cnt <= 0 && perrno != EAGAIN is semantically
> different from cnt < 0 && perrno != EAGAIN || cnt == 0.  If you wish
> to clarify it, parenthesizing it may help.  (&& binds more tightly
> than || in C, but one cannot be expected to remember all 15 (or
> whatever) priorities of operation.)
>
> Ethan
>
> --
> The laws that forbid the carrying of arms are laws [that have no remedy
> for evils].  They disarm only those who are neither inclined nor
> determined to commit crimes.
>                -- Cesare Beccaria, "On Crimes and Punishments", 1764
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.6 (GNU/Linux)
>
> iD8DBQFJSpYSr9kA9Ig8HBQRAhxoAJ93V/4QXDze/JpsSaxromGl0muxGgCbBn4v
> ddCYaRawK9cS46wbHhhpv5A=
> =BcsY
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> Devel mailing list
> Devel at pidgin.im
> http://pidgin.im/cgi-bin/mailman/listinfo/devel
>
>

Oops, this never made it to the list:

I see what you mean now; i messed up the logic in my head (i blame the
lack of caffeine in my system at the time).  I recant my original
request and in its place submit a new one, which as Eduardo pointed
out to me was in his original patch submission:
 if ((cnt < 0 && perrno != EAGAIN) || cnt == 0) {

Not really necessary if people like me would just learn to read.

-Mark