Hi!
Once again, a set of updated patches.
> well, you cut away the part where i kind of implied that it really
> should be IncludeCmd. (it kind of is awkward, but i think consistency
> trumps that.)
That might've been a bit too implicit for me. ;) Changed it to IncludeCmd.
>
> >> on that matter, have you tested all error scenarios?
> >
> >The ones I added? Or all that were modified?
> >
> modified only to the degree that the change cannot be confidently
> claimed to be irrelevant. so if you changed the output format or the
> control flow, that would need testing.
>
> >On the ones that I added,
> >yes, except for failure of popen and the `ret < 0` case in `eval_pclose`.
> >I have to admit I do not really know how to trigger those.
> >
> yeah, that's kind of impossible without some error injection via ptrace
> or some such. but you can fake the failure by just hacking the code. i
> wouldn't bother with the 3rd pclose branch, but popen should be tested.
Spent some more time testing. Everything looks good to me.
> >I don't suppose you have some formatter config file?
> >
> bah! Real Hackers (TM) write their own formatters as shell-perl hybrids
> with recursive regular expressions. ON PUNCH CARDS.
> (i can post it if you're really curious, but you need to sign a
> liability waiver. ;-)
I'll trade you my magnetic needles so you can do it directly on disk
next time. ;)
Maybe just add it to the repository?
> >(Hopefully) All your other proposed changes were incorporated in these
> >revised patches.
> >
> mostly. but as it goes, the longer you look, the more you find. :-D
Yeah, on that note, thanks again for all the patience!
> >+ fprintf( stderr, "%s:%d: ", cfile->file, cfile->line );
> >+ va_start( va, fmt );
> >+ vfprintf( stderr, fmt, va );
> >
> totally on a tangent ...
> i have a bit of an aversion towards tearing apart format strings. it
> isn't a big deal here, because we're just printing to a stream with no
> concurrency, but if the target was syslog or a dynamically allocated
> string, things would look differently. this could be avoided by printing
> the "payload" to a temporary, but this would obviously have a runtime
> cost, esp. when using nfasprintf() to be super-safe. but as we already
> have xvprintf_core(), this overhead could be avoided by supporting
> recursive format strings: xprintf("%s:%d: %@\n", file, line, fmt, va).
> of course this would be total overkill here ... though once you have it,
> all kinds of use cases pop up. just thinking aloud ...
Not fixed yet due to 1: the next point, and 2: do you want this? It would
introduce some duplication because conf_error and conf_sys_error print
to stderr and a buffer respectively. But if you see a way to avoid this,
I'd like to hear it.
>
> >+void
> >+conf_sys_error( conffile_t *cfile, const char *fmt, ... )
> >+{
> >+ va_list va;
> >+
> >+ fprintf( stderr, "%s:%d: ", cfile->file, cfile->line );
> >+ va_start( va, fmt );
> >+ vsys_error( fmt, va );
> >
> that call is theoretically unsafe; see the implementation.
Sorry, my lack of experience with C is showing here. I looked at
vsys_error, but are not sure what you mean. Is it the `abort`? Do I fix
it with inlining `vsys_error` and a dynamically allocated buffer?
> signal %d",... WTERMSIG().
> pedantically, that quoting is insufficient because it doesn't do
> escaping, but there's no need to overdo it here.
That's exactly the reason my inner pedant thought it was best to use
the previous version. But, changed it.
> this isn't quite right; it would miss excessive arguments.
> you can probably use `return getcline()` (and rely on the compiler
> eliding the tail recursion).
Yes. Because the error message needs to show the correct line
(`include_fp` must still be null when printing the error message)
I reorganized the code a bit.
> come to think of it, you were probably right to make a separate section
> for it. but it may be better to put it at the end, after the global one
> - it's kind of more like the global options. otoh, it's kind of a
> syntactic/structural part of the config, so having it at the top also
> makes sense. hmm.
Well, I now put your suggestion verbatim at the bottom. Choices...
Do you think that the `pclose` branch `ret < 0` error shouldn't be a
`conf_sys_error` but the normal one (`sys_error`)?
Cheers!>From 5c47b627095a5b69c7096f667dfd441f86e163d6 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 | 116 +++++++++++++++++++++-------------------------
src/config.h | 3 ++
src/driver.c | 6 +--
src/drv_imap.c | 78 ++++++++++++-------------------
src/drv_maildir.c | 18 +++----
5 files changed, 93 insertions(+), 128 deletions(-)
diff --git a/src/config.c b/src/config.c
index 456bd47..2995458 100644
--- a/src/config.c
+++ b/src/config.c
@@ -61,6 +61,31 @@ expand_strdup( const char *s, const conffile_t *cfile )
}
}
+void
+conf_error( conffile_t *cfile, const char *fmt, ... )
+{
+ va_list va;
+
+ flushn();
+ 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;
+
+ fprintf( stderr, "%s:%d: ", cfile->file, cfile->line );
+ 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 +100,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 +121,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 +145,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 +158,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 +178,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 +264,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 +276,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 +305,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 +323,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 +552,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 +563,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 +615,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 +625,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.41.0
>From fdc1dc5e2e908918fcd486674091b64b77faa2de 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 | 72 +++++++++++++++++++++++++++++++++++++++++++++++-----
src/config.h | 3 +++
src/mbsync.1 | 8 ++++++
3 files changed, 77 insertions(+), 6 deletions(-)
diff --git a/src/config.c b/src/config.c
index 2995458..0cbf607 100644
--- a/src/config.c
+++ b/src/config.c
@@ -61,13 +61,21 @@ expand_strdup( const char *s, const conffile_t *cfile )
}
}
+static void
+conf_print_loc( const conffile_t *cfile )
+{
+ if (cfile->include_fp)
+ fprintf( stderr, "%s:%d:included:%d: ", cfile->file, cfile->line, cfile->include_line );
+ else
+ fprintf( stderr, "%s:%d: ", cfile->file, cfile->line );
+}
+
void
conf_error( conffile_t *cfile, const char *fmt, ... )
{
va_list va;
- flushn();
- 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 +87,7 @@ conf_sys_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 );
vsys_error( fmt, va );
va_end( va );
@@ -317,6 +325,50 @@ getopt_helper( conffile_t *cfile, int *cops, channel_conf_t *conf )
return 1;
}
+static void
+include_cmd_popen( conffile_t *cfile, const char *cmd )
+{
+ if (!(cfile->include_fp = popen( cmd, "r" ))) {
+ sys_error( "popen" );
+ cfile->err = 1;
+ return;
+ }
+ cfile->include_line = 0;
+ cfile->include_command = nfstrdup( cmd );
+}
+
+static void
+include_cmd_pclose( conffile_t *cfile )
+{
+ int ret;
+
+ if ((ret = pclose( cfile->include_fp ))) {
+ if (ret < 0)
+ conf_sys_error( cfile, "pclose" );
+ else if (WIFSIGNALED( ret ))
+ conf_error( cfile, "command \"%s\" crashed with signal %d\n",
+ cfile->include_command, WTERMSIG( ret ) );
+ else
+ conf_error( cfile, "command \"%s\" exited with status %d\n",
+ cfile->include_command, WEXITSTATUS( ret ) );
+ }
+ cfile->include_fp = NULL;
+ free( cfile->include_command );
+}
+
+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 )))
+ return 1;
+ include_cmd_pclose( cfile );
+ }
+ cfile->line++;
+ return (cfile->rest = fgets( cfile->buf, cfile->bufl, cfile->fp )) != NULL;
+}
+
int
getcline( conffile_t *cfile )
{
@@ -325,9 +377,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->include_fp)
+ conf_error( cfile, "nested IncludeCmd\n" );
+ else
+ include_cmd_popen( cfile, cfile->val );
+ }
+ while (read_cline( cfile )) {
if (!(cfile->cmd = get_arg( cfile, ARG_OPTIONAL, &comment ))) {
if (comment)
continue;
@@ -335,6 +391,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;
@@ -487,6 +545,7 @@ load_config( const char *where )
return 1;
}
buf[sizeof(buf) - 1] = 0;
+ cfile.include_fp = NULL;
cfile.buf = buf;
cfile.bufl = sizeof(buf) - 1;
cfile.line = 0;
@@ -494,6 +553,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..694178b 100644
--- a/src/config.h
+++ b/src/config.h
@@ -12,10 +12,13 @@
typedef struct {
const char *file;
+ char *include_command;
FILE *fp;
+ FILE *include_fp;
char *buf;
int bufl;
int line;
+ int include_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.41.0
_______________________________________________
isync-devel mailing list
isync-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/isync-devel
