Code review for simplehash.h.

Fix reference to non-existent file in comment.

Add SH_ prefix to the EMPTY and IN_USE tokens, to reduce likelihood of
collisions with unrelated macros.

Add include guards around the function definitions that are not
"parameterized", so the header can be used again in the same translation
unit.

Undefine SH_EQUAL macro where other "parameter" macros are undefined, for
the same reason.

Author: Thomas Munro
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/CAEepm%3D1LdXZ3mMTM8tHt_b%3DK1kREit%3Dp8sikesak%3DkzHHM07Nw%40mail.gmail.com

diff --git a/src/include/lib/simplehash.h b/src/include/lib/simplehash.h
index 5273d494600ca2ffa3baae5987649995e0f4f604..3b30c185cdf0570075305201ac46ed7dfecda3cb 100644 (file)
--- a/src/include/lib/simplehash.h
+++ b/src/include/lib/simplehash.h
@@ -32,7 +32,7 @@
  *   - SH_STORE_HASH - if defined the hash is stored in the elements
  *   - SH_GET_HASH(tb, a) - return the field to store the hash in
  *
- *   For examples of usage look at simplehash.c (file local definition) and
+ *   For examples of usage look at tidbitmap.c (file local definition) and
  *   execnodes.h/execGrouping.c (exposed declaration, file local
  *   implementation).
  *
@@ -65,8 +65,8 @@
 /* type declarations */
 #define SH_TYPE SH_MAKE_NAME(hash)
 #define SH_STATUS SH_MAKE_NAME(status)
-#define SH_STATUS_EMPTY SH_MAKE_NAME(EMPTY)
-#define SH_STATUS_IN_USE SH_MAKE_NAME(IN_USE)
+#define SH_STATUS_EMPTY SH_MAKE_NAME(SH_EMPTY)
+#define SH_STATUS_IN_USE SH_MAKE_NAME(SH_IN_USE)
 #define SH_ITERATOR SH_MAKE_NAME(iterator)
 
 /* function declarations */
@@ -185,6 +185,16 @@ SH_SCOPE void SH_STAT(SH_TYPE * tb);
 #define SH_COMPARE_KEYS(tb, ahash, akey, b) (SH_EQUAL(tb, b->SH_KEY, akey))
 #endif
 
+/*
+ * Wrap the following definitions in include guards, to avoid multiple
+ * definition errors if this header is included more than once.  The rest of
+ * the file deliberately has no include guards, because it can be included
+ * with different parameters to define functions and types with non-colliding
+ * names.
+ */
+#ifndef SIMPLEHASH_H
+#define SIMPLEHASH_H
+
 /* FIXME: can we move these to a central location? */
 
 /* calculate ceil(log base 2) of num */
@@ -206,6 +216,8 @@ sh_pow2(uint64 num)
    return ((uint64) 1) << sh_log2(num);
 }
 
+#endif
+
 /*
  * Compute sizing parameters for hashtable. Called when creating and growing
  * the hashtable.
@@ -924,6 +936,7 @@ SH_STAT(SH_TYPE * tb)
 #undef SH_GET_HASH
 #undef SH_STORE_HASH
 #undef SH_USE_NONDEFAULT_ALLOCATOR
+#undef SH_EQUAL
 
 /* undefine locally declared macros */
 #undef SH_MAKE_PREFIX