[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [coldsync-hackers] Reporting conduit errors



On Sunday 21 September 2003 11:53, Andrew Arensburger wrote:

> 	Currently, the log on the Palm is just a series of lines of
> the form
> 	<database-name>: <status>
> Having the main ColdSync binary collect all of the messages from all
> of the conduits and log them in a consistent format would please my
> sense of esthetics.

Fair enough. Would the attached patch be a step in the right direction? It 
does the right thing for fetch, install, and sync conduits... Mostly. The 
success/debug message filtering might be iffy.

There's a placeholder for dump behaviour, but I'm too scared to just pipe 
the status line to mail without handling badly behaved/chatty conduit 
situations.

> 	I wouldn't call it hack. Rather, it's an optimization for user
> time: a Dump conduit can potentially run for a long time, but the user
> can just grab ver Palm and walk away, without having to wait for the
> conduits to finish running.

"Hack" might be too strong a word, I guess. It's a neat thing, although I'm 
not entirely convinced that dump conduits are all that slow in practice. 
Fetch conduits that hit the network are pretty nasty, that's for sure.

> 	It's clear that a Sync conduit can do anything that any other
> conduit can do. The reason there are Fetch and Dump conduits in the
> first place is that they're significantly easier to write than Sync
> conduits, in the general case.

Sorry, maybe I wasn't clear.

I'm not talking about writing a sync conduit in the "talk to the PDA using 
DLP sense".

I'm talking about a perl script that's instantiated using the "sync" flavor. 
Unless I'm missing something, the only functional difference between a 
fetch and sync flavor conduit is the availability of the SPC pipe. They're 
both called with $PDB open to the InputDB. Changing the flavor in 
coldsync.conf from fetch to sync and changing the StartConduit call to use 
sync in the fetch script is functionally identical _except_ for the 
ordering.

Going from a dump conduit to sync should be about as easy.

> 	It should be possible to do the same thing here: write a Perl
> module that implements the generic sync, with all of its complications
> and flourishes. It should use methods for most things, e.g., reading
> the on-disk PDB, parsing a raw record, comparing two records to see if
> they're identical, and so forth. Throw in some Emacs-like hooks for
> doing things before and after the sync.

conduit.c:run_conduits() seems to be a mostly generic sync.

All the conduit execution code calls run_conduits. Near as I can see, 
there's only three variables in the process. Calling order, flavor_mask, 
and the SPC pipe. flavor_mask is essentially ignored (in fact, the attached 
patch is probably the only thing that uses it). SPC is only set for sync 
conduits, but there's no practical reason it can't be set for fetch and 
install conduits.

I guess the current level of abstraction could be condensed better into a 
simple perl class, but it's not too bad as it is.

Okay, my wife is bugging me for the computer. Must go.

c.
Index: conduit.c
===================================================================
RCS file: /var/lib/cvs/coldsync/src/conduit.c,v
retrieving revision 2.66
diff -u -r2.66 conduit.c
--- conduit.c	14 Jun 2003 17:44:41 -0000	2.66
+++ conduit.c	21 Sep 2003 18:08:20 -0000
@@ -87,7 +87,10 @@
 static int cond_readline(char *buf,
 			 int len,
 			 FILE *fromchild);
-static int cond_readstatus(FILE *fromchild);
+static int cond_readstatus(struct Palm *palm,
+                           const struct dlp_dbinfo *dbinfo,
+                           unsigned short flavor_mask,
+                           FILE *fromchild);
 static RETSIGTYPE sigchld_handler(int sig);
 static INLINE Bool crea_type_matches(
 	const conduit_block *cond,
@@ -686,7 +689,7 @@
 			/* The child has printed something. Read it, then
 			 * check again.
 			 */
-			err = cond_readstatus(fromchild);
+			err = cond_readstatus(palm,dbinfo,flavor_mask,fromchild);
 			if (err > 0)
 				laststatus = err;
 			goto check_status;
@@ -834,7 +837,7 @@
 					"Child has printed to stdout.\n");
 
 			block_sigchld(&sigmask);
-			err = cond_readstatus(fromchild);
+			err = cond_readstatus(palm,dbinfo,flavor_mask,fromchild);
 			unblock_sigchld(&sigmask);
 
 			CONDUIT_TRACE(2)
@@ -1129,7 +1132,7 @@
 			break;
 
 		/* If we get here, then something was printed to 'fromchild' */
-		err = cond_readstatus(fromchild);
+		err = cond_readstatus(palm,dbinfo,flavor_mask,fromchild);
 		if (err > 0)
 			laststatus = err;
 		else
@@ -2132,7 +2135,10 @@
  * given above), assume an error code of 501.
  */
 static int
-cond_readstatus(FILE *fromchild)
+cond_readstatus(struct Palm *palm,
+                const struct dlp_dbinfo *dbinfo,
+                unsigned short flavor_mask,
+					 FILE *fromchild)
 {
 	int err;			/* Internal error status */
 	int errcode;			/* Error code */
@@ -2188,7 +2194,22 @@
 		errmsg = buf;
 	}
 
-	/* XXX - Do something intelligent */
+	if (flavor_mask & FLAVORFL_DUMP)
+	{
+		/* XXX dump conduits aren't connected... we should try to e-mail the
+		palm owner with any serious error messages, but we probably don't
+		want to generate a separate mail for each and every error line generated.
+		That implies buffering of some kind. So we'll just do nothing for now. */
+
+	} else if( (errcode / 100) > 0 && ((errcode / 100)) != 2 )
+	{
+		/* Don't send debug messages. Also don't send success messages...
+		The last status messages will be repeated, but the filtering of
+		the success messages means that we should only repeat error messages.
+		That doesn't seem like such a bad thing. */
+		va_add_to_log(palm_pconn(palm), "%s - %s\n", dbinfo->name, errmsg);
+	}
+
 	fprintf(stderr, "%s\n", errmsg);
 				/* XXX add conduit name here.  */