[GitHub] [httpd] ylavic commented on a change in pull request #185: Rewrite of mod_unique_id:

[GitBox <gi...@apache.org>]

ylavic commented on a change in pull request #185:
URL: https://github.com/apache/httpd/pull/185#discussion_r692955246



##########
File path: modules/metadata/mod_unique_id.c
##########
@@ -19,238 +19,285 @@
  *
  * Original author: Dean Gaudet <dg...@arctic.org>
  * UUencoding modified by: Alvaro Martinez Echevarria <al...@lander.es>
+ * Complete rewrite by: Atle Solbakken <at...@goliathdns.no>, April 2021
  */
 
-#define APR_WANT_BYTEFUNC   /* for htons() et al */
-#include "apr_want.h"
-#include "apr_general.h"    /* for APR_OFFSETOF                */
-#include "apr_network_io.h"
+/* Enable when ready to use new library encoder
+ * #define WITH_APR_ENCODE
+ */
+
+/* Enable debug to error log
+ * #define UNIQUE_ID_DEBUG
+ */
 
+#define THREADED_COUNTER "unique_id_counter"
+
+#include "apr.h"
+
+#ifdef WITH_APR_ENCODE
+#    include "apr_encode.h"
+#endif
+#include "apr_thread_proc.h"
+#include "apr_cstr.h"
+
+#include "ap_mpm.h"
 #include "httpd.h"
 #include "http_config.h"
 #include "http_log.h"
-#include "http_protocol.h"  /* for ap_hook_post_read_request */
+#include "http_protocol.h"
+
+#ifdef HAVE_SYS_GETTID
+#    include <sys/syscall.h>
+#    include <sys/types.h>
+#endif
+
+#if APR_HAVE_UNISTD_H
+#    include <unistd.h>
+#endif
+#if APR_HAVE_PROCESS_H
+#    include <process.h>
+#endif
 
-#define ROOT_SIZE 10
+typedef apr_uint16_t unique_counter;
 
-typedef struct {
-    unsigned int stamp;
-    char root[ROOT_SIZE];
-    unsigned short counter;
-    unsigned int thread_index;
+/* Unique ID structure members must be aligned on 1-byte boundaries */
+
+#pragma pack(push)
+#pragma pack(1)
+
+typedef struct unique_id_rec {
+    apr_uint32_t process_id;
+    apr_uint64_t thread_id;

Review comment:
       Since the struct is packed the field `thread_id` (uint64_t) is now unaligned, can we access it without faulting (SIGBUS) on platforms like sparc?
   Do we really need packing here, or e.g. `process_id` could be moved below `timestamp` to let the struct be naturally aligned?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@httpd.apache.org
For additional commands, e-mail: notifications-help@httpd.apache.org