Re: [coldsync-hackers] 3.0-pre3: serial comm problems - FIXED

[Christophe Beauregard <christophe dot beauregard at sympatico.ca>]

On Saturday 17 May 2003 07:15, Alessandro Zummo wrote:
> Christophe Beauregard <christophe.beauregard@sympatico.ca> wrote:

> > A better fix probably involves cleaning up how connection status is
> > tracked and ensuring it's actually used in places where it's
> > relevant... I'd imagine this would be sometime _before_ the disconnect
> > phase.
>
> I've attached a possible patch, please let me know your opinion. I'll
> appreciate every suggestion, the connection/error tracking subsystem
> doesn't always work properly.

I noticed.

Actually, I found the code very clean and the tracing framework is just 
lovely... All the different layers are fairly cleanly separated from each 
other enough that I was able to get into it with no prior knowledge of the 
program and fix the problem in just a few hours. That gets high marks in my 
book.

There is one minor problem with your patch... It was actually in the 
original code, not just your patch. I noticed it while I was debugging and 
forgot to mention it:

@@ -349,7 +364,9 @@
 int
 PConn_isonline(PConnection *p)
 {
-	return p->status & PCONNSTAT_UP;
+	pconn_stat status = PConn_get_status(p);
+
+	return (status & PCONNSTAT_UP) ? 1 : 0;
 }

PCONNSTAT_* is an enumerated type, not a bitmask. 
PCONNSTAT_CLOSED&PCONNSTAT_UP evaluates to true. Try:

+	return (status == PCONNSTAT_UP) ? 1 : 0;

Otherwise, it handles the status changes a whole lot better.

Your patch unfortunately doesn't address the problem that the status is 
essentially ignored by everything until sometime after it's too late. I 
suspect that this bug would never have lived as long as it did if a few 
PConn_inonline() calls happened _during_ the sync process rather than 
during the teardown.

I'm not sure the best place to check. Maybe the dlp_ layer could throw 
errors when called on any connection that isn't in the UP state? I don't 
know enough about the connection initialization process to say if this can 
be done cleanly (I assume that connection setup is done on a connection 
that isn't UP, right? Is dlp used, or just a lower layer like padp?)

c.

-- 
This message was sent through the coldsync-hackers mailing list.  To remove
yourself from this mailing list, send a message to majordomo@thedotin.net
with the words "unsubscribe coldsync-hackers" in the message body.  For more
information on Coldsync, send mail to coldsync-hackers-owner@thedotin.net.