Hintsdb: tidy coding for DB create
authorJeremy Harris <jgh146exb@wizmail.org>
Fri, 7 Jun 2024 20:45:57 +0000 (21:45 +0100)
committerJeremy Harris <jgh146exb@wizmail.org>
Fri, 7 Jun 2024 20:45:57 +0000 (21:45 +0100)
src/src/dbfn.c
src/src/dbfunctions.h
src/src/directory.c
src/src/exim_dbutil.c
src/src/hintsdb.h
src/src/verify.c

index 5f30ccf58760c1ad7652e0c34a251f48163137f7..f1989d4136e74ed134dce041da90dda65378c708 100644 (file)
@@ -67,8 +67,6 @@ Arguments:
   name     The single-component name of one of Exim's database files.
   flags    Either O_RDONLY or O_RDWR, indicating the type of open required;
              O_RDWR implies "create if necessary"
-XXX this is a mess.  hintsdb.h has grown lots of code expecting O_CREAT
-XXX with the obvious semantics, and not that described above.
   dbblock  Points to an open_db block to be filled in.
   lof      If TRUE, write to the log for actual open failures (locking failures
            are always logged).
@@ -79,17 +77,14 @@ Returns:   NULL if the open failed, or the locking failed. After locking
 
            On success, dbblock is returned. This contains the dbm pointer and
            the fd of the locked lock file.
-
-There are some calls that use O_RDWR|O_CREAT for the flags. Having discovered
-this in December 2005, I'm not sure if this is correct or not, but for the
-moment I haven't changed them.
 */
 
 open_db *
-dbfn_open(uschar *name, int flags, open_db *dbblock, BOOL lof, BOOL panic)
+dbfn_open(const uschar * name, int flags, open_db * dbblock,
+  BOOL lof, BOOL panic)
 {
 int rc, save_errno;
-BOOL read_only = flags == O_RDONLY;
+BOOL read_only = flags & O_RDONLY;
 flock_t lock_data;
 uschar dirname[PATHLEN], filename[PATHLEN];
 
@@ -165,6 +160,7 @@ databases - often this is caused by non-matching db.h and the library. To make
 it easy to pin this down, there are now debug statements on either side of the
 open call. */
 
+flags &= O_RDONLY | O_RDWR;
 snprintf(CS filename, sizeof(filename), "%s/%s", dirname, name);
 
 priv_drop_temp(exim_uid, exim_gid);
@@ -202,7 +198,6 @@ DEBUG(D_hints_lookup)
   debug_printf_indent("opened hints database %s: flags=%s\n", filename,
     flags == O_RDONLY ? "O_RDONLY"
     : flags == O_RDWR ? "O_RDWR"
-    : flags == (O_RDWR|O_CREAT) ? "O_RDWR|O_CREAT"
     : "??");
 
 /* Pass back the block containing the opened database handle and the open fd
index 1f0dec1f7f0c6703ba9d68bf9f848c3b8fb149c5..0b0bcab22839c2906f02c17f9a53ad2700ab1e1e 100644 (file)
@@ -14,7 +14,7 @@
 
 void     dbfn_close(open_db *);
 int      dbfn_delete(open_db *, const uschar *);
-open_db *dbfn_open(uschar *, int, open_db *, BOOL, BOOL);
+open_db *dbfn_open(const uschar *, int, open_db *, BOOL, BOOL);
 void    *dbfn_read_with_length(open_db *, const uschar *, int *);
 void    *dbfn_read_enforce_length(open_db *, const uschar *, size_t);
 uschar  *dbfn_scan(open_db *, BOOL, EXIM_CURSOR **);
index 94303db0bdc244c205f7a22bcb5c835c197ba579..876d097b51551ea8b3018b783dbea3a293fa0a1b 100644 (file)
@@ -36,7 +36,7 @@ Returns:    panic on failure if panic is set; otherwise return FALSE;
 */
 
 BOOL
-directory_make(const uschar *parent, const uschar *name,
+directory_make(const uschar * parent, const uschar * name,
                int mode, BOOL panic)
 {
 BOOL use_chown = parent == spool_directory && geteuid() == root_uid;
index 4e8b291c6a0875c5895ba3042ee6f72129f6d551..b152df14587086147c9c46b687f078b6cf2aabaa 100644 (file)
@@ -287,28 +287,23 @@ Returns:   NULL if the open failed, or the locking failed.
 */
 
 open_db *
-dbfn_open(uschar *name, int flags, open_db *dbblock, BOOL lof, BOOL panic)
+dbfn_open(const uschar * name, int flags, open_db * dbblock,
+  BOOL lof, BOOL panic)
 {
 int rc;
 struct flock lock_data;
-BOOL read_only = flags == O_RDONLY;
+BOOL read_only = flags & O_RDONLY;
 uschar * dirname, * filename;
 
 /* The first thing to do is to open a separate file on which to lock. This
 ensures that Exim has exclusive use of the database before it even tries to
 open it. If there is a database, there should be a lock file in existence. */
 
-#ifdef COMPILE_UTILITY
 if (  asprintf(CSS &dirname, "%s/db", spool_directory) < 0
    || asprintf(CSS &filename, "%s/%s.lockfile", dirname, name) < 0)
   return NULL;
-#else
-dirname = string_sprintf("%s/db", spool_directory);
-filename = string_sprintf("%s/%s.lockfile", dirname, name);
-#endif
 
-dbblock->lockfd = Uopen(filename, flags, 0);
-if (dbblock->lockfd < 0)
+if ((dbblock->lockfd = Uopen(filename, O_RDWR|O_CREAT, 0)) < 0)
   {
   printf("** Failed to open database lock file %s: %s\n", filename,
     strerror(errno));
@@ -331,7 +326,7 @@ if (sigalrm_seen) errno = ETIMEDOUT;
 if (rc < 0)
   {
   printf("** Failed to get %s lock for %s: %s",
-    flags & O_WRONLY ? "write" : "read",
+    read_only ? "read" : "write",
     filename,
     errno == ETIMEDOUT ? "timed out" : strerror(errno));
   (void)close(dbblock->lockfd);
@@ -341,14 +336,11 @@ if (rc < 0)
 /* At this point we have an opened and locked separate lock file, that is,
 exclusive access to the database, so we can go ahead and open it. */
 
-#ifdef COMPILE_UTILITY
 if (asprintf(CSS &filename, "%s/%s", dirname, name) < 0) return NULL;
-#else
-filename = string_sprintf("%s/%s", dirname, name);
-#endif
-dbblock->dbptr = exim_dbopen(filename, dirname, flags, 0);
 
-if (!dbblock->dbptr)
+if (flags & O_RDWR) flags |= O_CREAT;
+
+if (!(dbblock->dbptr = exim_dbopen(filename, dirname, flags, 0)))
   {
   printf("** Failed to open DBM file %s for %s:\n   %s%s\n", filename,
     read_only? "reading" : "writing", strerror(errno),
@@ -1427,3 +1419,5 @@ return 0;
 #endif  /* EXIM_TIDYDB */
 
 /* End of exim_dbutil.c */
+/* vi: aw ai sw=2
+*/
index 17b5c243c07d764d355909e84249bfccb02ca2ba..3898b0221aa2175c44a4cc403f0b67cadc1ee327 100644 (file)
@@ -23,7 +23,7 @@ binary blobs.
 
 The API is:
   Functions:
-    exim_dbopen
+    exim_dbopen                O_RDONLY/O_RDWR, optionally OR'd with O_CREAT
     exim_dbclose
     exim_dbget
     exim_dbput
@@ -542,8 +542,10 @@ if (db_create(&b, dbp, 0) == 0)
   {
   dbp->app_private = b;
   if (b->open(b, NULL, CS name, NULL,
-             flags == O_RDONLY ? DB_UNKNOWN : DB_HASH,
-             flags == O_RDONLY ? DB_RDONLY : DB_CREATE,
+             flags & O_CREAT ? DB_HASH : DB_UNKNOWN,
+             flags & O_CREAT ? DB_CREATE
+             : flags & O_RDONLY ? DB_RDONLY
+             : 0,              /*XXX is there a writeable if exists option? */
              mode) == 0
          )
     return dbp;
@@ -676,8 +678,10 @@ EXIM_DB * dbp;
 return db_create(&dbp, NULL, 0) == 0
   && (  dbp->set_errcall(dbp, dbfn_bdb_error_callback),
        dbp->open(dbp, CS name, NULL,
-         flags == O_RDONLY ? DB_UNKNOWN : DB_HASH,
-         flags == O_RDONLY ? DB_RDONLY : DB_CREATE,
+         flags & O_CREAT ? DB_HASH : DB_UNKNOWN,
+         flags & O_CREAT ? DB_CREATE
+         : flags & O_RDONLY ? DB_RDONLY
+         : 0,          /*XXX*/
          mode)
      ) == 0
   ? dbp : NULL;
index c5b99a97c1a1cf64ccaf93dc8c6c60eab1098e05..b1e5d68021f5e5ffec9f88c856e61f0a719df852 100644 (file)
@@ -294,7 +294,7 @@ implying some kind of I/O error. We don't want to write the cache in that case.
 Otherwise the value is ccache_accept, ccache_reject, or ccache_reject_mfnull. */
 
 if (dom_rec->result != ccache_unknown)
-  if (!(dbm_file = dbfn_open(US"callout", O_RDWR|O_CREAT, &dbblock, FALSE, TRUE)))
+  if (!(dbm_file = dbfn_open(US"callout", O_RDWR, &dbblock, FALSE, TRUE)))
     {
     HDEBUG(D_verify) debug_printf_indent("callout cache: not available\n");
     }
@@ -316,7 +316,7 @@ is disabled. */
 if (done  &&  addr_rec->result != ccache_unknown)
   {
   if (!dbm_file)
-    dbm_file = dbfn_open(US"callout", O_RDWR|O_CREAT, &dbblock, FALSE, TRUE);
+    dbm_file = dbfn_open(US"callout", O_RDWR, &dbblock, FALSE, TRUE);
   if (!dbm_file)
     {
     HDEBUG(D_verify) debug_printf_indent("no callout cache available\n");
@@ -3523,7 +3523,7 @@ dbdata_callout_cache_address cache_address_record;
 
 if (!pos_cache && !neg_cache)
   return;
-if (!(dbm_file = dbfn_open(US"callout", O_RDWR|O_CREAT, &dbblock, FALSE, TRUE)))
+if (!(dbm_file = dbfn_open(US"callout", O_RDWR, &dbblock, FALSE, TRUE)))
   {
   HDEBUG(D_verify) debug_printf_indent("quota cache: not available\n");
   return;