some addtl sanity checks detect incorrect API use

some addtl sanity checks detect incorrect API use
unconditionally call mcdb_make_destroy() in mcdb_make_finish()

contrib/MCDB_File/MCDB_File.xs

@@ -294,5 +294,8 @@ void
 mcdbxs_make_finish(mk)
     struct mcdb_make * mk;
   CODE:
-    if (mcdb_make_finish(mk) != 0 || mcdb_makefn_finish(mk, true) != 0)
+    if (mcdb_make_finish(mk) != 0 || mcdb_makefn_finish(mk, true) != 0) {
+        /*mcdb_make_destroy(mk);*//* already called in mcdb_make_finish() */
+        mcdb_makefn_cleanup(mk);
         croak("MCDB_File::Make::finish: %s", Strerror(errno));
+    }

mcdb.c

@@ -290,8 +290,8 @@ mcdb_mmap_unmap(struct mcdb_mmap * const restrict map)
 {
     if (map->ptr)
         munmap(map->ptr, map->size);
-    map->ptr   = NULL;
-    map->size  = 0;    /* map->size initialization required for mcdb_read() */
+    map->ptr  = NULL;
+    map->size = 0;    /* map->size initialization required for mcdb_read() */
 }
 
 bool  __attribute_noinline__
@@ -335,6 +335,7 @@ mcdb_mmap_prefault(const struct mcdb_mmap * const restrict map)
 void  __attribute_noinline__
 mcdb_mmap_free(struct mcdb_mmap * const restrict map)
 {
+    if (map == NULL) return;
     mcdb_mmap_unmap(map);
     if (map->fn_free) {
         if (map->fname != NULL && map->fname != map->fnamebuf) {
@@ -348,8 +349,11 @@ mcdb_mmap_free(struct mcdb_mmap * const restrict map)
 void  __attribute_noinline__
 mcdb_mmap_destroy(struct mcdb_mmap * const restrict map)
 {
-    if (map->dfd != -1)
+    if (map == NULL) return;
+    if (map->dfd != -1) {
         (void) nointr_close(map->dfd);
+        map->dfd = -1;
+    }
     mcdb_mmap_free(map);
 }
 

mcdb.h

@@ -126,7 +126,7 @@ mcdb_mmap_create(struct mcdb_mmap * restrict,
   __attribute_nonnull_x__((3,4,5))  __attribute_warn_unused_result__;
 extern void
 mcdb_mmap_destroy(struct mcdb_mmap * restrict)
-  __attribute_nonnull__;
+  ;
 /* check if constant db has been updated and refresh mmap
  * (for use with mcdb mmaps held open for any period of time)
  * (i.e. for any use other than mcdb_mmap_create(), query, mcdb_mmap_destroy())
@@ -149,7 +149,7 @@ mcdb_mmap_prefault(const struct mcdb_mmap * restrict)
   __attribute_nonnull__;
 extern void
 mcdb_mmap_free(struct mcdb_mmap * restrict)
-  __attribute_nonnull__;
+  ;
 extern bool
 mcdb_mmap_reopen(struct mcdb_mmap * restrict)
   __attribute_nonnull__  __attribute_warn_unused_result__;

mcdb_make.c

@@ -82,8 +82,9 @@ struct mcdb_hplist {
 /* routine marked to indicate unlikely branch;
  * __attribute_cold__ can be used instead of __builtin_expect() */
 static int  __attribute_noinline__  __attribute_cold__
-mcdb_make_err(int errnum)
+mcdb_make_err(struct mcdb_make * const restrict m, int errnum)
 {
+    if (m != NULL) mcdb_make_destroy(m);
     errno = errnum;
     return -1;
 }
@@ -211,17 +212,17 @@ mcdb_make_addbegin(struct mcdb_make * const restrict m,
     char *p;
     const size_t pos = m->pos;
     const size_t len = 8 + keylen + datalen;/* arbitrary ~2 GB limit for lens */
-    if (m->map == MAP_FAILED)                      return mcdb_make_err(EPERM);
-    if (m->hp.l == ~0 && !mcdb_hplist_alloc(m))    return mcdb_make_err(errno);
+    if (m->map == MAP_FAILED)                 return mcdb_make_err(NULL,EPERM);
+    if (m->hp.l== ~0 && !mcdb_hplist_alloc(m))return mcdb_make_err(NULL,errno);
     m->hp.p = pos;
     m->hp.h = UINT32_HASH_DJB_INIT;
-    if (keylen > INT_MAX-8 || datalen > INT_MAX-8) return mcdb_make_err(EINVAL);
+    if (keylen>INT_MAX-8 || datalen>INT_MAX-8)return mcdb_make_err(NULL,EINVAL);
     m->hp.l = (uint32_t)keylen;
   #if !defined(_LP64) && !defined(__LP64__)  /* (no 4 GB limit in 64-bit) */
-    if (pos > UINT_MAX-len)                        return mcdb_make_err(ENOMEM);
+    if (pos > UINT_MAX-len)                   return mcdb_make_err(NULL,ENOMEM);
   #endif
-    if (m->fsz < pos+len && !mcdb_mmap_upsize(m,pos+len))
-                                                   return mcdb_make_err(errno);
+    if (m->fsz < pos+len && !mcdb_mmap_upsize(m, pos+len))
+                                              return mcdb_make_err(NULL,errno);
     p = m->map + pos - m->offset;
     uint32_strpack_bigendian_macro(p,keylen);
     uint32_strpack_bigendian_macro(p+4,datalen);
@@ -335,27 +336,27 @@ mcdb_make_finish(struct mcdb_make * const restrict m)
     char *p;
     const uint32_t * const restrict count = m->count;
     char header[MCDB_HEADER_SZ];
-    if (m->map == MAP_FAILED)                  return mcdb_make_err(EPERM);
+    if (m->map == MAP_FAILED)                  return mcdb_make_err(m,EPERM);
 
     for (u = 0, i = 0; i < MCDB_SLOTS; ++i)
         u += count[i];  /* no overflow; limited in mcdb_hplist_alloc */
 
     /* check for integer overflow and that sufficient space allocated in file */
-    if (u > INT_MAX)                           return mcdb_make_err(ENOMEM);
+    if (u > INT_MAX)                           return mcdb_make_err(m,ENOMEM);
   #if !defined(_LP64) && !defined(__LP64__)
-    if (u > (UINT_MAX>>4))                     return mcdb_make_err(ENOMEM);
+    if (u > (UINT_MAX>>4))                     return mcdb_make_err(m,ENOMEM);
     u <<= 4;  /* 8 byte hash entries in 32-bit; x 2 for space in table */
-    if (m->pos > (UINT_MAX-u))                 return mcdb_make_err(ENOMEM);
+    if (m->pos > (UINT_MAX-u))                 return mcdb_make_err(m,ENOMEM);
   #endif
 
     /* add "hole" for alignment; incompatible with djb cdbdump */
     /* padding to align hash tables to MCDB_PAD_ALIGN bytes (16) */
     d = (MCDB_PAD_ALIGN - (m->pos & MCDB_PAD_MASK)) & MCDB_PAD_MASK;
   #if !defined(_LP64) && !defined(__LP64__)
-    if (d > (UINT_MAX-(m->pos+u)))             return mcdb_make_err(ENOMEM);
+    if (d > (UINT_MAX-(m->pos+u)))             return mcdb_make_err(m,ENOMEM);
   #endif
     if (m->fsz < m->pos+d && !mcdb_mmap_upsize(m,m->pos+d))
-                                               return mcdb_make_err(errno);
+                                               return mcdb_make_err(m,errno);
     if (d) memset(m->map + m->pos - m->offset, ~0, d);
     m->pos += d; /*set all bits in hole so code can detect end of data padding*/
 
@@ -426,7 +427,7 @@ mcdb_make_finish(struct mcdb_make * const restrict m)
 }
 
 /* caller should call mcdb_make_destroy() upon errors from mcdb_make_*() calls
- * (already called when mcdb_make_finish() is successful)
+ * (already called unconditionally in mcdb_make_finish() (successful or not))
  * m->fd is not closed here since mcdb_make_start() takes open file descriptor
  * (caller should cleanup m->fd)
  */

mcdb_makefn.c

@@ -127,9 +127,12 @@ mcdb_makefn_cleanup (struct mcdb_make * const restrict m)
         unlink(m->fntmp);
         if (m->fd >= 0)
             (void) nointr_close(m->fd);
+        m->fd = -1;
     }
-    if (m->fntmp != NULL)
+    if (m->fntmp != NULL) {
         m->fn_free(m->fntmp);
+        m->fntmp = NULL;
+    }
     if (errsave != 0)
         errno = errsave;
     return -1;

nss_mcdb.c

@@ -103,12 +103,8 @@ static void _nss_mcdb_atexit(void)
     struct mcdb_mmap * restrict map;
     for (uintptr_t i = 0; i < _nss_num_dbs; ++i) {
         map = &_nss_mcdb_mmap_st[i];
-        if (   _nss_mcdb_mmap_st[i].ptr != NULL
-            || _nss_mcdb_mmap_st[i].fname != NULL  ) {
-            mcdb_mmap_destroy(&_nss_mcdb_mmap_st[i]);
-            _nss_mcdb_mmap_st[i].ptr = NULL;
-            _nss_mcdb_mmap_st[i].fname = NULL;
-        }
+        if (map->ptr || map->fname)
+            mcdb_mmap_destroy(map);
     }
 }
 #endif