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

Ethan Blanton elb at pidgin.im
Thu Dec 18 13:27:30 EST 2008


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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://pidgin.im/pipermail/devel/attachments/20081218/19ff5923/attachment.sig>


More information about the Devel mailing list