Hintsdb: tidy coding for DB create

diff --git a/src/src/dbfn.c b/src/src/dbfn.c
index 5f30ccf58760c1ad7652e0c34a251f48163137f7..f1989d4136e74ed134dce041da90dda65378c708 100644 (file)
--- a/src/src/dbfn.c
+++ b/src/src/dbfn.c
@@ -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"
   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).
   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.
 
            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 *
 */
 
 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;
 {
 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];
 
 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. */
 
 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);
 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"
   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
     : "??");
 
 /* Pass back the block containing the opened database handle and the open fd

diff --git a/src/src/dbfunctions.h b/src/src/dbfunctions.h
index 1f0dec1f7f0c6703ba9d68bf9f848c3b8fb149c5..0b0bcab22839c2906f02c17f9a53ad2700ab1e1e 100644 (file)
--- a/src/src/dbfunctions.h
+++ b/src/src/dbfunctions.h
@@ -14,7 +14,7 @@
 
 void     dbfn_close(open_db *);
 int      dbfn_delete(open_db *, const uschar *);
 
 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 **);
 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 **);

diff --git a/src/src/directory.c b/src/src/directory.c
index 94303db0bdc244c205f7a22bcb5c835c197ba579..876d097b51551ea8b3018b783dbea3a293fa0a1b 100644 (file)
--- a/src/src/directory.c
+++ b/src/src/directory.c
@@ -36,7 +36,7 @@ Returns:    panic on failure if panic is set; otherwise return FALSE;
 */
 
 BOOL
 */
 
 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;
                int mode, BOOL panic)
 {
 BOOL use_chown = parent == spool_directory && geteuid() == root_uid;

diff --git a/src/src/exim_dbutil.c b/src/src/exim_dbutil.c
index 4e8b291c6a0875c5895ba3042ee6f72129f6d551..b152df14587086147c9c46b687f078b6cf2aabaa 100644 (file)
--- a/src/src/exim_dbutil.c
+++ b/src/src/exim_dbutil.c
@@ -287,28 +287,23 @@ Returns:   NULL if the open failed, or the locking failed.
 */
 
 open_db *
 */
 
 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;
 {
 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. */
 
 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;
 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));
   {
   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",
 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);
     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. */
 
 /* 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;
 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),
   {
   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 */
 #endif  /* EXIM_TIDYDB */
 
 /* End of exim_dbutil.c */

+/* vi: aw ai sw=2
+*/

diff --git a/src/src/hintsdb.h b/src/src/hintsdb.h
index 17b5c243c07d764d355909e84249bfccb02ca2ba..3898b0221aa2175c44a4cc403f0b67cadc1ee327 100644 (file)
--- a/src/src/hintsdb.h
+++ b/src/src/hintsdb.h
@@ -23,7 +23,7 @@ binary blobs.
 
 The API is:
   Functions:
 
 The API is:
   Functions:

-    exim_dbopen
+    exim_dbopen                O_RDONLY/O_RDWR, optionally OR'd with O_CREAT

     exim_dbclose
     exim_dbget
     exim_dbput
     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,
   {
   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;
              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,
 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;
          mode)
      ) == 0
   ? dbp : NULL;

diff --git a/src/src/verify.c b/src/src/verify.c
index c5b99a97c1a1cf64ccaf93dc8c6c60eab1098e05..b1e5d68021f5e5ffec9f88c856e61f0a719df852 100644 (file)
--- a/src/src/verify.c
+++ b/src/src/verify.c
@@ -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)
 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");
     }
     {
     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)
 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");
   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 (!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;
   {
   HDEBUG(D_verify) debug_printf_indent("quota cache: not available\n");
   return;