address review

Author: picnixz

Modules/_hashopenssl.c

@@ -278,14 +278,14 @@ get_hashlib_state(PyObject *module)
 }
 
 typedef struct {
-    PyObject_HASHLIB_HEAD
+    HASHLIB_OBJECT_HEAD
     EVP_MD_CTX *ctx;    /* OpenSSL message digest context */
 } HASHobject;
 
 #define HASHobject_CAST(op) ((HASHobject *)(op))
 
 typedef struct {
-    PyObject_HASHLIB_HEAD
+    HASHLIB_OBJECT_HEAD
     HMAC_CTX *ctx;            /* OpenSSL hmac context */
 } HMACobject;
 

Modules/blake2module.c

@@ -352,7 +352,7 @@ type_to_impl(PyTypeObject *type)
 }
 
 typedef struct {
-    PyObject_HASHLIB_HEAD
+    HASHLIB_OBJECT_HEAD
     union {
         Hacl_Hash_Blake2s_state_t *blake2s_state;
         Hacl_Hash_Blake2b_state_t *blake2b_state;

Modules/hashlib.h

@@ -3,9 +3,9 @@
 #include "pycore_lock.h"        // PyMutex
 
 /*
- * Given a PyObject* obj, fill in the Py_buffer* viewp with the result
- * of PyObject_GetBuffer.  Sets an exception and issues the erraction
- * on any errors, e.g. 'return NULL' or 'goto error'.
+ * Given a PyObject* 'obj', fill in the Py_buffer* 'viewp' with the result
+ * of PyObject_GetBuffer.  Sets an exception and issues the 'erraction'
+ * on any errors, e.g., 'return NULL' or 'goto error'.
  */
 #define GET_BUFFER_VIEW_OR_ERROR(obj, viewp, erraction) do { \
         if (PyUnicode_Check((obj))) { \
@@ -34,36 +34,46 @@
 
 /*
  * Helper code to synchronize access to the hash object when the GIL is
- * released around a CPU consuming hashlib operation. All code paths that
- * access a mutable part of obj must be enclosed in an ENTER_HASHLIB /
- * LEAVE_HASHLIB block or explicitly acquire and release the lock inside
- * a PY_BEGIN / END_ALLOW_THREADS block if they wish to release the GIL for
- * an operation.
+ * released around a CPU consuming hashlib operation.
  *
- * These only drop the GIL if the lock acquisition itself is likely to
- * block. Thus the non-blocking acquire gating the GIL release for a
- * blocking lock acquisition. The intent of these macros is to surround
- * the assumed always "fast" operations that you aren't releasing the
- * GIL around.  Otherwise use code similar to what you see in hash
- * function update() methods.
+ * Code accessing a mutable part of the hash object must be enclosed in
+ * an HASHLIB_{ACQUIRE,RELEASE}_LOCK block or explicitly acquire and release
+ * the mutex inside a Py_BEGIN_ALLOW_THREADS -- Py_END_ALLOW_THREADS block if
+ * they wish to release the GIL for an operation.
  */
 
-#include "pythread.h"
-
-#define PyObject_HASHLIB_HEAD                                           \
+#define HASHLIB_OBJECT_HEAD                                             \
     PyObject_HEAD                                                       \
     /* Guard against race conditions during incremental update(). */    \
     PyMutex mutex;
 
-#define HASHLIB_ACQUIRE_LOCK(OBJ)   PyMutex_Lock(&(OBJ)->mutex)
-#define HASHLIB_RELEASE_LOCK(OBJ)   PyMutex_Unlock(&(OBJ)->mutex)
-
 #define HASHLIB_INIT_MUTEX(OBJ)         \
     do {                                \
         (OBJ)->mutex = (PyMutex){0};    \
     } while (0)
 
+#define HASHLIB_ACQUIRE_LOCK(OBJ)   PyMutex_Lock(&(OBJ)->mutex)
+#define HASHLIB_RELEASE_LOCK(OBJ)   PyMutex_Unlock(&(OBJ)->mutex)
+
+/*
+ * Message length above which the GIL is to be released
+ * when performing hashing operations.
+ */
 #define HASHLIB_GIL_MINSIZE         2048
+
+// Macros for executing code while conditionally holding the GIL.
+//
+// These only drop the GIL if the lock acquisition itself is likely to
+// block. Thus the non-blocking acquire gating the GIL release for a
+// blocking lock acquisition. The intent of these macros is to surround
+// the assumed always "fast" operations that you aren't releasing the
+// GIL around.
+
+/*
+ * Execute a suite of C statements 'STATEMENTS'.
+ *
+ * The GIL is held if 'SIZE' is below the HASHLIB_GIL_MINSIZE threshold.
+ */
 #define HASHLIB_EXTERNAL_INSTRUCTIONS_UNLOCKED(SIZE, STATEMENTS)    \
     do {                                                            \
         if ((SIZE) > HASHLIB_GIL_MINSIZE) {                         \
@@ -76,6 +86,11 @@
         }                                                           \
     } while (0)
 
+/*
+ * Lock 'OBJ' and execute a suite of C statements 'STATEMENTS'.
+ *
+ * The GIL is held if 'SIZE' is below the HASHLIB_GIL_MINSIZE threshold.
+ */
 #define HASHLIB_EXTERNAL_INSTRUCTIONS_LOCKED(OBJ, SIZE, STATEMENTS) \
     do {                                                            \
         if ((SIZE) > HASHLIB_GIL_MINSIZE) {                         \

Modules/hmacmodule.c

@@ -283,7 +283,7 @@ get_hmacmodule_state_by_cls(PyTypeObject *cls)
 typedef Hacl_Streaming_HMAC_agile_state HACL_HMAC_state;
 
 typedef struct HMACObject {
-    PyObject_HASHLIB_HEAD
+    HASHLIB_OBJECT_HEAD
     // Hash function information
     PyObject *name;         // rendered name (exact unicode object)
     HMAC_Hash_Kind kind;    // can be used for runtime dispatch (must be known)

Modules/md5module.c

@@ -38,7 +38,7 @@ class MD5Type "MD5object *" "&PyType_Type"
 #include "_hacl/Hacl_Hash_MD5.h"
 
 typedef struct {
-    PyObject_HASHLIB_HEAD
+    HASHLIB_OBJECT_HEAD
     Hacl_Hash_MD5_state_t *hash_state;
 } MD5object;
 

Modules/sha1module.c

@@ -38,7 +38,7 @@ class SHA1Type "SHA1object *" "&PyType_Type"
 #include "_hacl/Hacl_Hash_SHA1.h"
 
 typedef struct {
-    PyObject_HASHLIB_HEAD
+    HASHLIB_OBJECT_HEAD
     Hacl_Hash_SHA1_state_t *hash_state;
 } SHA1object;
 

Modules/sha2module.c

@@ -50,13 +50,13 @@ class SHA512Type "SHA512object *" "&PyType_Type"
 // TODO: Get rid of int digestsize in favor of Hacl state info?
 
 typedef struct {
-    PyObject_HASHLIB_HEAD
+    HASHLIB_OBJECT_HEAD
     int digestsize;
     Hacl_Hash_SHA2_state_t_256 *state;
 } SHA256object;
 
 typedef struct {
-    PyObject_HASHLIB_HEAD
+    HASHLIB_OBJECT_HEAD
     int digestsize;
     Hacl_Hash_SHA2_state_t_512 *state;
 } SHA512object;

Modules/sha3module.c

@@ -59,7 +59,7 @@ class _sha3.shake_256 "SHA3object *" "&SHAKE256type"
 #include "_hacl/Hacl_Hash_SHA3.h"
 
 typedef struct {
-    PyObject_HASHLIB_HEAD
+    HASHLIB_OBJECT_HEAD
     Hacl_Hash_SHA3_state_t *hash_state;
 } SHA3object;