Hi Oswald,

The previous mail was me sending -- instead of saving -- a draft. My bad.

Anyway, here are the updated patches.

> i'm attaching it for your masochistic pleasure. there you can see that
> it's actually not even aimed at isync specifically, because c++/qt?! i
> originally wrote this some 22 years ago, when astyle was Teh Shit.
> i wouldn't expect it to actually produce invariably correct results, and
> haven't used it on isync code for ... a very long time.

Ah ok, I'll just keep formatting by hand then. The perl scripts are a
work of art and make me want to punch something. Probably not cards ;)

> depending on what you mean by "this".
>
> if you mean the "short" tangent of using a temporary: nope, not worth
> it.
>
> if you mean the "long" tangent of using xprintf: yes, but not in this
> patch series.
> for background, see the wip/better-stderr branch, which turns the
> personal aversion into an actual requirement. but notably, the config
> parser doesn't strictly _need_ that, because it runs before concurrency
> starts.

> once the infra is there, strerror(errno) (and thus perror()) can be
> handled via the %m format (stolen from syslog), so things can be unified
> pretty much. the branch shows the direction.

A nice future refactoring project. Looks good, but given the amount of
feedback my contributions require, not efficient if I take it up ;)

Thanks for the patience and guidance!

> no, i mean the saving of errno - pedantically, you need to wrap the
> first fprintf().
> otoh, if the first print fails, the second one will most likely fail as
> well, and therefore it doesn't really matter if errno gets corrupted or
> not. but as i pushed e295f483 after much deliberation, i guess we should
> keep up the good practice.

Done!

> >     +static int
> >     +read_cline( conffile_t *cfile )
> >     +{
> >     +      if (cfile->include_fp) {
> >     +              cfile->include_line++;
> >     +              if ((cfile->rest = fgets( cfile->buf, cfile->bufl, 
> > cfile->include_fp )))
>
> assigning cfile->rest right at the start would be less obscure. the
> pointer value is already known anyway.

That gets you an excess token error on (/after) the last line. It's not
about setting it to `buf`, but only doing it if there is actually a line
left to read.

> >     +      if (cfile->cmd && !strcasecmp( cfile->cmd, "IncludeCmd" )) {
> >     +              if (cfile->include_fp)
> >     +                      conf_error( cfile, "nested IncludeCmd\n" );
> >     +              else
> >     +                      include_cmd_popen( cfile, cfile->val );
> >     +      }
>
> wouldn't just moving the body into the conditional below work?
> of course that would do the excess-argument check only after doing the
> popen, but that's consistent with how it works for other options.
> otoh, the side effects of instantly executing a potentially malformed
> command may be somewhat more grave than when processing other options.
> but handlign this properly would actually require a separate check
> anyway, as then we need to actually skip the command.

The problem with doing it after the popen is that `conf_print_loc`
will then show an error as if it is in the included output while it
is in the file itself. Moving it would mean copying the excess token
detection code, which is not worth it IMHO.

Cheers!
>From c8d835e2e3120b7ab562836ed0d21079a86e2809 Mon Sep 17 00:00:00 2001
From: Michiel van den Heuvel <michielvdnheu...@gmail.com>
Date: Thu, 10 Aug 2023 05:20:55 +0200
Subject: [PATCH 1/2] Refactor printing configuration errors

In preparation for adding the output of commands as configuration lines,
which will complicate printing.
---
 src/config.c      | 117 +++++++++++++++++++++-------------------------
 src/config.h      |   3 ++
 src/driver.c      |   6 +--
 src/drv_imap.c    |  78 ++++++++++++-------------------
 src/drv_maildir.c |  18 +++----
 5 files changed, 94 insertions(+), 128 deletions(-)

diff --git a/src/config.c b/src/config.c
index 456bd47..ac90c25 100644
--- a/src/config.c
+++ b/src/config.c
@@ -61,6 +61,32 @@ expand_strdup( const char *s, const conffile_t *cfile )
 	}
 }
 
+void
+conf_error( conffile_t *cfile, const char *fmt, ... )
+{
+	va_list va;
+
+	fprintf( stderr, "%s:%d: ", cfile->file, cfile->line );
+	va_start( va, fmt );
+	vfprintf( stderr, fmt, va );
+	va_end( va );
+	cfile->err = 1;
+}
+
+void
+conf_sys_error( conffile_t *cfile, const char *fmt, ... )
+{
+	va_list va;
+
+	int errno_bak = errno;
+	fprintf( stderr, "%s:%d: ", cfile->file, cfile->line );
+	errno = errno_bak;
+	va_start( va, fmt );
+	vsys_error( fmt, va );
+	va_end( va );
+	cfile->err = 1;
+}
+
 char *
 get_arg( conffile_t *cfile, int required, int *comment )
 {
@@ -75,10 +101,8 @@ get_arg( conffile_t *cfile, int required, int *comment )
 	if (!c || c == '#') {
 		if (comment)
 			*comment = (c == '#');
-		if (required) {
-			error( "%s:%d: parameter missing\n", cfile->file, cfile->line );
-			cfile->err = 1;
-		}
+		if (required)
+			conf_error( cfile, "parameter missing\n" );
 		ret = NULL;
 	} else {
 		for (escaped = 0, quoted = 0, ret = t = p; c; c = *p) {
@@ -98,13 +122,11 @@ get_arg( conffile_t *cfile, int required, int *comment )
 		}
 		*t = 0;
 		if (escaped) {
-			error( "%s:%d: unterminated escape sequence\n", cfile->file, cfile->line );
-			cfile->err = 1;
+			conf_error( cfile, "unterminated escape sequence\n" );
 			ret = NULL;
 		}
 		if (quoted) {
-			error( "%s:%d: missing closing quote\n", cfile->file, cfile->line );
-			cfile->err = 1;
+			conf_error( cfile, "missing closing quote\n" );
 			ret = NULL;
 		}
 	}
@@ -124,9 +146,7 @@ parse_bool( conffile_t *cfile )
 	    strcasecmp( cfile->val, "false" ) &&
 	    strcasecmp( cfile->val, "off" ) &&
 	    strcmp( cfile->val, "0" )) {
-		error( "%s:%d: invalid boolean value '%s'\n",
-		       cfile->file, cfile->line, cfile->val );
-		cfile->err = 1;
+		conf_error( cfile, "invalid boolean value '%s'\n", cfile->val );
 	}
 	return 0;
 }
@@ -139,9 +159,7 @@ parse_int( conffile_t *cfile )
 
 	ret = strtol( cfile->val, &p, 10 );
 	if (*p) {
-		error( "%s:%d: invalid integer value '%s'\n",
-		       cfile->file, cfile->line, cfile->val );
-		cfile->err = 1;
+		conf_error( cfile, "invalid integer value '%s'\n", cfile->val );
 		return 0;
 	}
 	return ret;
@@ -161,9 +179,7 @@ parse_size( conffile_t *cfile )
 	if (*p == 'b' || *p == 'B')
 		p++;
 	if (*p) {
-		fprintf (stderr, "%s:%d: invalid size '%s'\n",
-		         cfile->file, cfile->line, cfile->val);
-		cfile->err = 1;
+		conf_error( cfile, "invalid size '%s'\n", cfile->val);
 		return 0;
 	}
 	return ret;
@@ -249,9 +265,7 @@ getopt_helper( conffile_t *cfile, int *cops, channel_conf_t *conf )
 			} else if (!strcasecmp( "None", arg ) || !strcasecmp( "Noop", arg )) {
 				conf->ops[F] |= XOP_TYPE_NOOP;
 			} else {
-				error( "%s:%d: invalid Sync arg '%s'\n",
-				       cfile->file, cfile->line, arg );
-				cfile->err = 1;
+				conf_error( cfile, "invalid Sync arg '%s'\n", arg );
 			}
 		} while ((arg = get_arg( cfile, ARG_OPTIONAL, NULL )));
 		conf->ops[F] |= XOP_HAVE_TYPE;
@@ -263,15 +277,12 @@ getopt_helper( conffile_t *cfile, int *cops, channel_conf_t *conf )
 		conf->max_messages = parse_int( cfile );
 	} else if (!strcasecmp( "ExpireSide", cfile->cmd )) {
 		arg = cfile->val;
-		if (!strcasecmp( "Far", arg )) {
+		if (!strcasecmp( "Far", arg ))
 			conf->expire_side = F;
-		} else if (!strcasecmp( "Near", arg )) {
+		else if (!strcasecmp( "Near", arg ))
 			conf->expire_side = N;
-		} else {
-			error( "%s:%d: invalid ExpireSide argument '%s'\n",
-			       cfile->file, cfile->line, arg );
-			cfile->err = 1;
-		}
+		else
+			conf_error( cfile, "invalid ExpireSide argument '%s'\n", arg );
 	} else if (!strcasecmp( "ExpireUnread", cfile->cmd )) {
 		conf->expire_unread = parse_bool( cfile );
 	} else {
@@ -295,9 +306,7 @@ getopt_helper( conffile_t *cfile, int *cops, channel_conf_t *conf )
 					} else if (!strcasecmp( "None", arg )) {
 						conf->ops[F] |= op * (XOP_EXPUNGE_NOOP / OP_EXPUNGE);
 					} else {
-						error( "%s:%d: invalid %s arg '%s'\n",
-						       cfile->file, cfile->line, boxOps[i].name, arg );
-						cfile->err = 1;
+						conf_error( cfile, "invalid %s arg '%s'\n", boxOps[i].name, arg );
 					}
 				} while ((arg = get_arg( cfile, ARG_OPTIONAL, NULL )));
 				conf->ops[F] |= op * (XOP_HAVE_EXPUNGE / OP_EXPUNGE);
@@ -315,10 +324,8 @@ getcline( conffile_t *cfile )
 	char *arg;
 	int comment;
 
-	if (cfile->rest && (arg = get_arg( cfile, ARG_OPTIONAL, NULL ))) {
-		error( "%s:%d: excess token '%s'\n", cfile->file, cfile->line, arg );
-		cfile->err = 1;
-	}
+	if (cfile->rest && (arg = get_arg( cfile, ARG_OPTIONAL, NULL )))
+		conf_error( cfile, "excess token '%s'\n", arg );
 	while (fgets( cfile->buf, cfile->bufl, cfile->fp )) {
 		cfile->line++;
 		cfile->rest = cfile->buf;
@@ -546,9 +553,7 @@ load_config( const char *where )
 					cfile.ms_warn = 1;
 				  linkst:
 					if (*cfile.val != ':' || !(p = strchr( cfile.val + 1, ':' ))) {
-						error( "%s:%d: malformed mailbox spec\n",
-						       cfile.file, cfile.line );
-						cfile.err = 1;
+						conf_error( &cfile, "malformed mailbox spec\n" );
 						continue;
 					}
 					*p = 0;
@@ -559,18 +564,14 @@ load_config( const char *where )
 						}
 					}
 					channel->stores[fn] = (void *)~0;
-					error( "%s:%d: unknown store '%s'\n",
-					       cfile.file, cfile.line, cfile.val + 1 );
-					cfile.err = 1;
+					conf_error( &cfile, "unknown store '%s'\n", cfile.val + 1 );
 					continue;
 				  stpcom:
 					if (*++p)
 						channel->boxes[fn] = nfstrdup( p );
 				} else if (!getopt_helper( &cfile, &cops, channel )) {
-					error( "%s:%d: keyword '%s' is not recognized in Channel sections\n",
-					       cfile.file, cfile.line, cfile.cmd );
+					conf_error( &cfile, "keyword '%s' is not recognized in Channel sections\n", cfile.cmd );
 					cfile.rest = NULL;
-					cfile.err = 1;
 				}
 			}
 			if (!channel->stores[F]) {
@@ -615,10 +616,8 @@ load_config( const char *where )
 					arg = cfile.val;
 					goto addone;
 				} else {
-					error( "%s:%d: keyword '%s' is not recognized in Group sections\n",
-					       cfile.file, cfile.line, cfile.cmd );
+					conf_error( &cfile, "keyword '%s' is not recognized in Group sections\n", cfile.cmd );
 					cfile.rest = NULL;
-					cfile.err = 1;
 				}
 			}
 			glob_ok = 0;
@@ -627,36 +626,26 @@ load_config( const char *where )
 			UseFSync = parse_bool( &cfile );
 		} else if (!strcasecmp( "FieldDelimiter", cfile.cmd )) {
 			if (strlen( cfile.val ) != 1) {
-				error( "%s:%d: Field delimiter must be exactly one character long\n", cfile.file, cfile.line );
-				cfile.err = 1;
+				conf_error( &cfile, "Field delimiter must be exactly one character long\n" );
 			} else {
 				FieldDelimiter = cfile.val[0];
-				if (!ispunct( FieldDelimiter )) {
-					error( "%s:%d: Field delimiter must be a punctuation character\n", cfile.file, cfile.line );
-					cfile.err = 1;
-				}
+				if (!ispunct( FieldDelimiter ))
+					conf_error( &cfile, "Field delimiter must be a punctuation character\n" );
 			}
 		} else if (!strcasecmp( "BufferLimit", cfile.cmd )) {
 			BufferLimit = parse_size( &cfile );
-			if (!BufferLimit) {
-				error( "%s:%d: BufferLimit cannot be zero\n", cfile.file, cfile.line );
-				cfile.err = 1;
-			}
+			if (!BufferLimit)
+				conf_error( &cfile, "BufferLimit cannot be zero\n" );
 		} else if (!getopt_helper( &cfile, &gcops, &global_conf )) {
-			error( "%s:%d: '%s' is not a recognized section-starting or global keyword\n",
-			       cfile.file, cfile.line, cfile.cmd );
-			cfile.err = 1;
+			conf_error( &cfile, "'%s' is not a recognized section-starting or global keyword\n", cfile.cmd );
 			cfile.rest = NULL;
 			while (getcline( &cfile ))
 				if (!cfile.cmd)
 					goto reloop;
 			break;
 		}
-		if (!glob_ok) {
-			error( "%s:%d: global options may not follow sections\n",
-			       cfile.file, cfile.line );
-			cfile.err = 1;
-		}
+		if (!glob_ok)
+			conf_error( &cfile, "global options may not follow sections\n" );
 	}
 	fclose (cfile.fp);
 	if (cfile.ms_warn)
diff --git a/src/config.h b/src/config.h
index 0762b58..2177fb4 100644
--- a/src/config.h
+++ b/src/config.h
@@ -27,6 +27,9 @@ extern char FieldDelimiter;
 #define ARG_OPTIONAL 0
 #define ARG_REQUIRED 1
 
+void ATTR_PRINTFLIKE(2, 3) conf_error( conffile_t *cfile, const char *fmt, ... );
+void ATTR_PRINTFLIKE(2, 3) conf_sys_error( conffile_t *cfile, const char *fmt, ... );
+
 char *expand_strdup( const char *s, const conffile_t *cfile );
 
 char *get_arg( conffile_t *cfile, int required, int *comment );
diff --git a/src/driver.c b/src/driver.c
index 6b88dbb..885d7a3 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -83,15 +83,13 @@ parse_generic_store( store_conf_t *store, conffile_t *cfg, const char *type )
 		const char *p;
 		for (p = cfg->val; *p; p++) {
 			if (*p == '/') {
-				error( "%s:%d: flattened hierarchy delimiter cannot contain the canonical delimiter '/'\n", cfg->file, cfg->line );
-				cfg->err = 1;
+				conf_error( cfg, "flattened hierarchy delimiter cannot contain the canonical delimiter '/'\n" );
 				return;
 			}
 		}
 		store->flat_delim = nfstrdup( cfg->val );
 	} else {
-		error( "%s:%d: keyword '%s' is not recognized in %s sections\n", cfg->file, cfg->line, cfg->cmd, type );
+		conf_error( cfg, "keyword '%s' is not recognized in %s sections\n", cfg->cmd, type );
 		cfg->rest = NULL;
-		cfg->err = 1;
 	}
 }
diff --git a/src/drv_imap.c b/src/drv_imap.c
index ad95e3d..7f37fe5 100644
--- a/src/drv_imap.c
+++ b/src/drv_imap.c
@@ -3781,19 +3781,15 @@ imap_parse_store( conffile_t *cfg, store_conf_t **storep )
 #endif
 		} else if (!strcasecmp( "Port", cfg->cmd )) {
 			int port = parse_int( cfg );
-			if ((unsigned)port > 0xffff) {
-				error( "%s:%d: Invalid port number\n", cfg->file, cfg->line );
-				cfg->err = 1;
-			} else {
+			if ((unsigned)port > 0xffff)
+				conf_error( cfg, "Invalid port number\n" );
+			else
 				server->sconf.port = (ushort)port;
-			}
 		} else if (!strcasecmp( "Timeout", cfg->cmd )) {
 			server->sconf.timeout = parse_int( cfg ) * 1000;
 		} else if (!strcasecmp( "PipelineDepth", cfg->cmd )) {
-			if ((server->max_in_progress = parse_int( cfg )) < 1) {
-				error( "%s:%d: PipelineDepth must be at least 1\n", cfg->file, cfg->line );
-				cfg->err = 1;
-			}
+			if ((server->max_in_progress = parse_int( cfg )) < 1)
+				conf_error( cfg, "PipelineDepth must be at least 1\n" );
 		} else if (!strcasecmp( "DisableExtension", cfg->cmd ) ||
 		           !strcasecmp( "DisableExtensions", cfg->cmd )) {
 			arg = cfg->val;
@@ -3804,34 +3800,27 @@ imap_parse_store( conffile_t *cfg, store_conf_t **storep )
 						goto gotcap;
 					}
 				}
-				error( "%s:%d: Unrecognized IMAP extension '%s'\n", cfg->file, cfg->line, arg );
-				cfg->err = 1;
+				conf_error( cfg, "Unrecognized IMAP extension '%s'\n", arg );
 			  gotcap: ;
 			} while ((arg = get_arg( cfg, ARG_OPTIONAL, NULL )));
 #ifdef HAVE_LIBSSL
 		} else if (!strcasecmp( "CertificateFile", cfg->cmd )) {
 			server->sconf.cert_file = expand_strdup( cfg->val, cfg );
-			if (access( server->sconf.cert_file, R_OK )) {
-				sys_error( "%s:%d: CertificateFile '%s'",
-				           cfg->file, cfg->line, server->sconf.cert_file );
-				cfg->err = 1;
-			}
+			if (access( server->sconf.cert_file, R_OK ))
+				conf_sys_error( cfg, "CertificateFile '%s'",
+				                server->sconf.cert_file );
 		} else if (!strcasecmp( "SystemCertificates", cfg->cmd )) {
 			server->sconf.system_certs = parse_bool( cfg );
 		} else if (!strcasecmp( "ClientCertificate", cfg->cmd )) {
 			server->sconf.client_certfile = expand_strdup( cfg->val, cfg );
-			if (access( server->sconf.client_certfile, R_OK )) {
-				sys_error( "%s:%d: ClientCertificate '%s'",
-				           cfg->file, cfg->line, server->sconf.client_certfile );
-				cfg->err = 1;
-			}
+			if (access( server->sconf.client_certfile, R_OK ))
+				conf_sys_error( cfg, "ClientCertificate '%s'",
+				                server->sconf.client_certfile );
 		} else if (!strcasecmp( "ClientKey", cfg->cmd )) {
 			server->sconf.client_keyfile = expand_strdup( cfg->val, cfg );
-			if (access( server->sconf.client_keyfile, R_OK )) {
-				sys_error( "%s:%d: ClientKey '%s'",
-				           cfg->file, cfg->line, server->sconf.client_keyfile );
-				cfg->err = 1;
-			}
+			if (access( server->sconf.client_keyfile, R_OK ))
+				conf_sys_error( cfg, "ClientKey '%s'",
+				                server->sconf.client_keyfile );
 		} else if (!strcasecmp( "CipherString", cfg->cmd )) {
 			server->sconf.cipher_string = nfstrdup( cfg->val );
 		} else if (!strcasecmp( "TLSVersions", cfg->cmd )) {
@@ -3843,8 +3832,7 @@ imap_parse_store( conffile_t *cfg, store_conf_t **storep )
 				} else if (*arg == '-') {
 					and_mask = ~0;
 				} else {
-					error( "%s:%d: TLSVersions arguments must start with +/-\n", cfg->file, cfg->line );
-					cfg->err = 1;
+					conf_error( cfg, "TLSVersions arguments must start with +/-\n" );
 					continue;
 				}
 				arg++;
@@ -3857,8 +3845,7 @@ imap_parse_store( conffile_t *cfg, store_conf_t **storep )
 				} else if (!strcmp( "1.3", arg )) {
 					val = TLSv1_3;
 				} else {
-					error( "%s:%d: Unrecognized TLS version '%s'\n", cfg->file, cfg->line, arg );
-					cfg->err = 1;
+					conf_error( cfg, "Unrecognized TLS version '%s'\n", arg );
 					continue;
 				}
 				or_mask &= val;
@@ -3875,22 +3862,20 @@ imap_parse_store( conffile_t *cfg, store_conf_t **storep )
 			server->sconf.ssl_versions = 0;
 			arg = cfg->val;
 			do {
-				if (!strcasecmp( "SSLv2", arg )) {
+				if (!strcasecmp( "SSLv2", arg ))
 					warn( "Warning: SSLVersion SSLv2 is no longer supported\n" );
-				} else if (!strcasecmp( "SSLv3", arg )) {
+				else if (!strcasecmp( "SSLv3", arg ))
 					warn( "Warning: SSLVersion SSLv3 is no longer supported\n" );
-				} else if (!strcasecmp( "TLSv1", arg )) {
+				else if (!strcasecmp( "TLSv1", arg ))
 					server->sconf.ssl_versions |= TLSv1;
-				} else if (!strcasecmp( "TLSv1.1", arg )) {
+				else if (!strcasecmp( "TLSv1.1", arg ))
 					server->sconf.ssl_versions |= TLSv1_1;
-				} else if (!strcasecmp( "TLSv1.2", arg )) {
+				else if (!strcasecmp( "TLSv1.2", arg ))
 					server->sconf.ssl_versions |= TLSv1_2;
-				} else if (!strcasecmp( "TLSv1.3", arg )) {
+				else if (!strcasecmp( "TLSv1.3", arg ))
 					server->sconf.ssl_versions |= TLSv1_3;
-				} else {
-					error( "%s:%d: Unrecognized SSL version\n", cfg->file, cfg->line );
-					cfg->err = 1;
-				}
+				else
+					conf_error( cfg, "Unrecognized SSL version\n" );
 			} while ((arg = get_arg( cfg, ARG_OPTIONAL, NULL )));
 #else
 		} else if (!strcasecmp( "CertificateFile", cfg->cmd ) ||
@@ -3927,8 +3912,7 @@ imap_parse_store( conffile_t *cfg, store_conf_t **storep )
 				cfg->err = 1;
 #endif
 			} else {
-				error( "%s:%d: Invalid TLS type\n", cfg->file, cfg->line );
-				cfg->err = 1;
+				conf_error( cfg, "Invalid TLS type\n" );
 			}
 		} else if (!strcasecmp( "AuthMech", cfg->cmd ) ||
 		         !strcasecmp( "AuthMechs", cfg->cmd )) {
@@ -3946,8 +3930,7 @@ imap_parse_store( conffile_t *cfg, store_conf_t **storep )
 					if (srv->name && !strcmp( srv->name, cfg->val ))
 						goto gotsrv;
 				store->server = (void *)~0;
-				error( "%s:%d: unknown IMAP account '%s'\n", cfg->file, cfg->line, cfg->val );
-				cfg->err = 1;
+				conf_error( cfg, "unknown IMAP account '%s'\n", cfg->val );
 				continue;
 			  gotsrv:
 				store->server = srv;
@@ -3959,8 +3942,7 @@ imap_parse_store( conffile_t *cfg, store_conf_t **storep )
 				store->path = nfstrdup( cfg->val );
 			} else if (!strcasecmp( "PathDelimiter", cfg->cmd )) {
 				if (strlen( cfg->val ) != 1) {
-					error( "%s:%d: Path delimiter must be exactly one character long\n", cfg->file, cfg->line );
-					cfg->err = 1;
+					conf_error( cfg, "Path delimiter must be exactly one character long\n" );
 					continue;
 				}
 				store->delimiter = cfg->val[0];
@@ -3969,10 +3951,8 @@ imap_parse_store( conffile_t *cfg, store_conf_t **storep )
 			}
 			continue;
 		} else {
-			error( "%s:%d: keyword '%s' is not recognized in IMAPAccount sections\n",
-			       cfg->file, cfg->line, cfg->cmd );
+			conf_error( cfg, "keyword '%s' is not recognized in IMAPAccount sections\n", cfg->cmd );
 			cfg->rest = NULL;
-			cfg->err = 1;
 			continue;
 		}
 		acc_opt = 1;
diff --git a/src/drv_maildir.c b/src/drv_maildir.c
index cdffc32..05b3db4 100644
--- a/src/drv_maildir.c
+++ b/src/drv_maildir.c
@@ -1899,27 +1899,23 @@ maildir_parse_store( conffile_t *cfg, store_conf_t **storep )
 #endif /* USE_DB */
 		} else if (!strcasecmp( "InfoDelimiter", cfg->cmd )) {
 			if (strlen( cfg->val ) != 1) {
-				error( "%s:%d: Info delimiter must be exactly one character long\n", cfg->file, cfg->line );
-				cfg->err = 1;
+				conf_error( cfg, "Info delimiter must be exactly one character long\n" );
 				continue;
 			}
 			store->info_delimiter = cfg->val[0];
 			if (!ispunct( store->info_delimiter )) {
-				error( "%s:%d: Info delimiter must be a punctuation character\n", cfg->file, cfg->line );
-				cfg->err = 1;
+				conf_error( cfg, "Info delimiter must be a punctuation character\n" );
 				continue;
 			}
 		} else if (!strcasecmp( "SubFolders", cfg->cmd )) {
-			if (!strcasecmp( "Verbatim", cfg->val )) {
+			if (!strcasecmp( "Verbatim", cfg->val ))
 				store->sub_style = SUB_VERBATIM;
-			} else if (!strcasecmp( "Maildir++", cfg->val )) {
+			else if (!strcasecmp( "Maildir++", cfg->val ))
 				store->sub_style = SUB_MAILDIRPP;
-			} else if (!strcasecmp( "Legacy", cfg->val )) {
+			else if (!strcasecmp( "Legacy", cfg->val ))
 				store->sub_style = SUB_LEGACY;
-			} else {
-				error( "%s:%d: Unrecognized SubFolders style\n", cfg->file, cfg->line );
-				cfg->err = 1;
-			}
+			else
+				conf_error( cfg, "Unrecognized SubFolders style\n" );
 		} else {
 			parse_generic_store( &store->gen, cfg, "MaildirStore" );
 		}
-- 
2.42.0

>From bffde7e10d521f5dbd02f0938fb5f14bb45a8739 Mon Sep 17 00:00:00 2001
From: Michiel van den Heuvel <michielvdnheu...@gmail.com>
Date: Thu, 17 Aug 2023 20:25:51 +0200
Subject: [PATCH 2/2] Add IncludeCmd directive to config parser

---
 src/config.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++----
 src/config.h |  3 +++
 src/mbsync.1 |  8 ++++++
 3 files changed, 80 insertions(+), 5 deletions(-)

diff --git a/src/config.c b/src/config.c
index ac90c25..084a54e 100644
--- a/src/config.c
+++ b/src/config.c
@@ -61,12 +61,21 @@ expand_strdup( const char *s, const conffile_t *cfile )
 	}
 }
 
+static void
+conf_print_loc( const conffile_t *cfile )
+{
+	if (cfile->eval_fp)
+		fprintf( stderr, "%s:%d:included:%d: ", cfile->file, cfile->line, cfile->eval_line );
+	else
+		fprintf( stderr, "%s:%d: ", cfile->file, cfile->line );
+}
+
 void
 conf_error( conffile_t *cfile, const char *fmt, ... )
 {
 	va_list va;
 
-	fprintf( stderr, "%s:%d: ", cfile->file, cfile->line );
+	conf_print_loc( cfile );
 	va_start( va, fmt );
 	vfprintf( stderr, fmt, va );
 	va_end( va );
@@ -79,7 +88,7 @@ conf_sys_error( conffile_t *cfile, const char *fmt, ... )
 	va_list va;
 
 	int errno_bak = errno;
-	fprintf( stderr, "%s:%d: ", cfile->file, cfile->line );
+	conf_print_loc( cfile );
 	errno = errno_bak;
 	va_start( va, fmt );
 	vsys_error( fmt, va );
@@ -318,6 +327,53 @@ getopt_helper( conffile_t *cfile, int *cops, channel_conf_t *conf )
 	return 1;
 }
 
+static void
+eval_cmd_popen( conffile_t *cfile, const char *cmd )
+{
+	if (!(cfile->eval_fp = popen( cmd, "r" ))) {
+		sys_error( "popen" );
+		cfile->err = 1;
+		return;
+	}
+	cfile->eval_line = 0;
+	cfile->eval_command = nfstrdup( cmd );
+}
+
+static void
+eval_cmd_pclose( conffile_t *cfile )
+{
+	int ret;
+
+	if ((ret = pclose( cfile->eval_fp ))) {
+		if (ret < 0) {
+			sys_error( "pclose" );
+			cfile->err = 1;
+		} else if (WIFSIGNALED( ret )) {
+			conf_error( cfile, "command \"%s\" crashed with signal %d\n",
+			            cfile->eval_command, WTERMSIG( ret ) );
+		} else {
+			conf_error( cfile, "command \"%s\" exited with status %d\n",
+			            cfile->eval_command, WEXITSTATUS( ret ) );
+		}
+	}
+	free( cfile->eval_command );
+	cfile->eval_fp = NULL;
+	cfile->eval_command = NULL;
+}
+
+static int
+read_cline( conffile_t *cfile )
+{
+	if (cfile->eval_fp) {
+		cfile->eval_line++;
+		if ((cfile->rest = fgets( cfile->buf, cfile->bufl, cfile->eval_fp )))
+			return 1;
+		eval_cmd_pclose( cfile );
+	}
+	cfile->line++;
+	return (cfile->rest = fgets( cfile->buf, cfile->bufl, cfile->fp )) != NULL;
+}
+
 int
 getcline( conffile_t *cfile )
 {
@@ -326,9 +382,13 @@ getcline( conffile_t *cfile )
 
 	if (cfile->rest && (arg = get_arg( cfile, ARG_OPTIONAL, NULL )))
 		conf_error( cfile, "excess token '%s'\n", arg );
-	while (fgets( cfile->buf, cfile->bufl, cfile->fp )) {
-		cfile->line++;
-		cfile->rest = cfile->buf;
+	if (cfile->cmd && !strcasecmp( cfile->cmd, "IncludeCmd" )) {
+		if (cfile->eval_fp)
+			conf_error( cfile, "nested IncludeCmd\n" );
+		else
+			eval_cmd_popen( cfile, cfile->val );
+	}
+	while (read_cline( cfile )) {
 		if (!(cfile->cmd = get_arg( cfile, ARG_OPTIONAL, &comment ))) {
 			if (comment)
 				continue;
@@ -336,6 +396,8 @@ getcline( conffile_t *cfile )
 		}
 		if (!(cfile->val = get_arg( cfile, ARG_REQUIRED, NULL )))
 			continue;
+		if (!strcasecmp( cfile->cmd, "IncludeCmd" ))
+			return getcline( cfile );
 		return 1;
 	}
 	return 0;
@@ -488,6 +550,7 @@ load_config( const char *where )
 		return 1;
 	}
 	buf[sizeof(buf) - 1] = 0;
+	cfile.eval_fp = NULL;
 	cfile.buf = buf;
 	cfile.bufl = sizeof(buf) - 1;
 	cfile.line = 0;
@@ -495,6 +558,7 @@ load_config( const char *where )
 	cfile.ms_warn = 0;
 	cfile.renew_warn = 0;
 	cfile.delete_warn = 0;
+	cfile.cmd = NULL;
 	cfile.rest = NULL;
 
 	gcops = 0;
diff --git a/src/config.h b/src/config.h
index 2177fb4..c1642ef 100644
--- a/src/config.h
+++ b/src/config.h
@@ -12,10 +12,13 @@
 
 typedef struct {
 	const char *file;
+	char *eval_command;
 	FILE *fp;
+	FILE *eval_fp;
 	char *buf;
 	int bufl;
 	int line;
+	int eval_line;
 	int err;
 	int ms_warn, renew_warn, delete_warn;
 	int path_len;
diff --git a/src/mbsync.1 b/src/mbsync.1
index 939c8c5..1a9e70e 100644
--- a/src/mbsync.1
+++ b/src/mbsync.1
@@ -786,6 +786,14 @@ absolute limit, as even a single message can consume more memory than
 this.
 (Default: \fI10M\fR)
 .
+.SS Dynamic Configuration
+.TP
+\fBIncludeCmd\fR \fIcommand\fR
+Specify a shell command to obtain configuration file content from.
+This keyword can appear anywhere. The command's output will be
+interpreted as if it appeared inline; it can range from nothing at all
+to multiple sections.
+.
 .SH CONSOLE OUTPUT
 If \fBmbsync\fR's output is connected to a console, it will print progress
 counters by default. The output will look like this:
-- 
2.42.0

_______________________________________________
isync-devel mailing list
isync-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/isync-devel

Reply via email to