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

[Mark Doliner]

On Thu, Dec 18, 2008 at 12:21 PM, Mark Schneider <queueram at gmail.com> wrote:
> 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.

Already fixed in revision b4a46d69:
http://pidgin.im/pipermail/commits/2008-December/010948.html

-Mark