On Sun, Nov 29, 2020 at 07:30:44PM +0100, Oswald Buddenhagen wrote:
> On Sun, Nov 29, 2020 at 05:37:43PM +0100, Andreas Grapentin wrote:
> > maybe the version XDG_CONFIG_HOME/isync/mbsyncrc makes sense, in case
> > any other utilities that are part of isync will provide config files in
> > the future?
> > 
> i grok the argument, but i just don't think that this is going to happen
> given the track record of the last 25 years. in the worst case, we'd just
> support another fallback location. so XDG_CONFIG_HOME/mbsyncrc it is.

Ok, XDG_CONFIG_HOME/mbsyncrc it is.

> speaking of possible overlap, you need to add a NEWS entry.

Added.

> 
> > +           cfile.fp = fopen( cfile.file, "r" );
> > +
> > +           if (!cfile.fp && errno == ENOENT) {
> 
> > +                   errno = 0;
> > 
> that's unnecessary - the next error would overwrite it anyway, and no sane
> code would rely on this being zero without resetting it immediately before.

I don't like letting the state of errno leak to later code, if the
reported error has been handled. this can lead to unexpected output in
error reporting code in unrelated issues that don't overwrite errno,
such as a later call to sys_error.

> 
> > +                   nfsnprintf( path, sizeof(path), "%s/." EXE "rc", Home );
> 
> > +                   cfile.file = path;
> > +
> that's also unnecessary, as the address didn't change.

Could you please clarify -- do you mean the assignment of path to
cfile.file is unnecssary in general, because I could just use path as
parameter to fopen? or do you believe that this particular assignment is
unnecessary, because path did not change since the last assignment of
cfile.file?

Updated patch is attached again.

Thanks!
-A


-- 

------------------------------------------------------------------------------
my GPG Public Key:                 https://files.grapentin.org/.gpg/public.key
------------------------------------------------------------------------------
From ca9dd3f6afffa5910fee5611074359953f2120b1 Mon Sep 17 00:00:00 2001
From: Andreas Grapentin <andr...@grapentin.org>
Date: Sun, 29 Nov 2020 07:51:20 +0100
Subject: [PATCH] src/config.c: added XDG_CONFIG_HOME support

---
 NEWS         |  3 +++
 src/config.c | 28 ++++++++++++++++++++++------
 src/mbsync.1 |  6 ++++--
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/NEWS b/NEWS
index 6e094e6..37c94a2 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,8 @@
 [1.4.0]
 
+Added support for a second user config file location in
+XDG_CONFIG_HOME/mbsyncrc.
+
 The 'isync' compatibility wrapper was removed.
 
 Added support for disabling TLS v1.3 - adjust SSLVersions if you set it.
diff --git a/src/config.c b/src/config.c
index dec47b2..4be13ab 100644
--- a/src/config.c
+++ b/src/config.c
@@ -33,6 +33,7 @@
 #include <string.h>
 #include <stdlib.h>
 #include <stdio.h>
+#include <errno.h>
 
 static store_conf_t *stores;
 
@@ -331,17 +332,32 @@ load_config( const char *where )
        char buf[1024];
 
        if (!where) {
-               nfsnprintf( path, sizeof(path), "%s/." EXE "rc", Home );
+               const char *config_home = getenv("XDG_CONFIG_HOME");
+               if (config_home)
+                       nfsnprintf( path, sizeof(path), "%s/" EXE "rc", 
config_home );
+               else
+                       nfsnprintf( path, sizeof(path), "%s/.config/" EXE "rc", 
Home );
                cfile.file = path;
-       } else
-               cfile.file = where;
+               cfile.fp = fopen( cfile.file, "r" );
 
-       info( "Reading configuration file %s\n", cfile.file );
+               if (!cfile.fp && errno == ENOENT) {
+                       errno = 0;
+                       nfsnprintf( path, sizeof(path), "%s/." EXE "rc", Home );
+                       cfile.file = path;
+                       cfile.fp = fopen( cfile.file, "r" );
+               }
+       } else {
+               cfile.file = where;
+               cfile.fp = fopen( cfile.file, "r" );
+       }
 
-       if (!(cfile.fp = fopen( cfile.file, "r" ))) {
-               sys_error( "Cannot open config file '%s'", cfile.file );
+       if (!cfile.fp) {
+               sys_error( "Cannot open user config file '%s'", cfile.file );
                return 1;
        }
+
+       info( "Reading configuration file %s\n", cfile.file );
+
        buf[sizeof(buf) - 1] = 0;
        cfile.buf = buf;
        cfile.bufl = sizeof(buf) - 1;
diff --git a/src/mbsync.1 b/src/mbsync.1
index 65a213e..1ae3c89 100644
--- a/src/mbsync.1
+++ b/src/mbsync.1
@@ -47,7 +47,8 @@ Multiple replicas of each mailbox can be maintained.
 .TP
 \fB-c\fR, \fB--config\fR \fIfile\fR
 Read configuration from \fIfile\fR.
-By default, the configuration is read from ~/.mbsyncrc.
+By default, the configuration is read from the first file found in the list
+of default locations. See \fBFILES\fR below for a complete list.
 .TP
 \fB-a\fR, \fB--all\fR
 Select all configured channels. Any channel/group specifications on the command
@@ -803,8 +804,9 @@ There is no risk as long as the IMAP mailbox is accessed by 
only one client
 .
 .SH FILES
 .TP
+.B $XDG_CONFIG_HOME/mbsyncrc
 .B ~/.mbsyncrc
-Default configuration file
+Default configuration files
 .TP
 .B ~/.mbsync/
 Directory containing synchronization state files
-- 
2.29.2

Attachment: signature.asc
Description: PGP signature

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

Reply via email to