refactor hal_struct to use a dedicated namespace

grandixximo · grandixximo · commit 8013f3bd53bb · 2026-03-28T07:01:54.000+08:00
Replace the param-backed implementation with a proper struct namespace
inside HAL shmem.  hal_struct_newf now maintains its own linked list
(struct_list_ptr / struct_free_ptr in hal_data_t), sorted by name,
entirely separate from pins, signals, and parameters.

hal_struct_attach increments attach_count; hal_struct_detach decrements
it, making detach a meaningful operation.  No hal_param_s32_new call
anywhere in the new path.

HAL_VER bumped to 0x11 for the hal_data_t layout change.
diff --git a/src/hal/hal.h b/src/hal/hal.h
@@ -985,18 +985,22 @@ extern void hal_stream_wait_writable(hal_stream_t *stream, sig_atomic_t *stop);
 
 
 /** Named struct API
+ *
+ * Structs live in their own namespace inside HAL shmem, separate from
+ * pins, signals, and parameters.  They are not visible in halcmd show pin/
+ * param/sig output.
  *
  * hal_struct_newf() allocates 'size' bytes in HAL shmem, optionally
- * initialises it from 'defval' (or zeroes if NULL), and registers a HAL_RO
- * s32 param named according to the printf-style format so that the data can
- * be found by name from any process that has HAL shmem mapped.
+ * initialises it from 'defval' (or zeroes if NULL), and registers the blob
+ * in the struct namespace under the printf-style name.  Must be called
+ * before hal_ready().
  *
- * hal_struct_attach() resolves a name previously registered by
- * hal_struct_newf() and sets *memptr to point at the data.  May be called
- * from both RT and userspace after hal_init().
+ * hal_struct_attach() finds an entry by name, increments its reference
+ * count, and sets *memptr to point at the data.  May be called from both
+ * RT and userspace after hal_init().
  *
- * hal_struct_detach() is a no-op; HAL shmem lifetime is managed by the
- * creating component.
+ * hal_struct_detach() decrements the reference count.  The data itself
+ * lives for the lifetime of the HAL shmem block.
  */
 extern int hal_struct_newf(int comp_id, long int size, const void *defval,
                             const char *fmt, ...)
diff --git a/src/hal/hal_lib.c b/src/hal/hal_lib.c
@@ -128,6 +128,8 @@ hal_comp_t *halpr_alloc_comp_struct(void);
 static hal_pin_t *alloc_pin_struct(void);
 static hal_sig_t *alloc_sig_struct(void);
 static hal_param_t *alloc_param_struct(void);
+static hal_struct_entry_t *alloc_struct_entry(void);
+static void free_struct_entry(hal_struct_entry_t *p);
 static hal_oldname_t *halpr_alloc_oldname_struct(void);
 #ifdef RTAPI
 static hal_funct_t *alloc_funct_struct(void);
@@ -2688,6 +2690,23 @@ hal_param_t *halpr_find_param_by_name(const char *name)
     return 0;
 }
 
+hal_struct_entry_t *halpr_find_struct_by_name(const char *name)
+{
+    int next;
+    hal_struct_entry_t *entry;
+
+    /* search struct list for 'name' */
+    next = hal_data->struct_list_ptr;
+    while (next != 0) {
+	entry = SHMPTR(next);
+	if (strcmp(entry->name, name) == 0) {
+	    return entry;
+	}
+	next = entry->next_ptr;
+    }
+    return 0;
+}
+
 hal_thread_t *halpr_find_thread_by_name(const char *name)
 {
     int next;
@@ -3084,6 +3103,7 @@ static int init_hal_data(void)
     hal_data->pin_list_ptr = 0;
     hal_data->sig_list_ptr = 0;
     hal_data->param_list_ptr = 0;
+    hal_data->struct_list_ptr = 0;
     hal_data->funct_list_ptr = 0;
     hal_data->thread_list_ptr = 0;
     hal_data->base_period = 0;
@@ -3093,6 +3113,7 @@ static int init_hal_data(void)
     hal_data->pin_free_ptr = 0;
     hal_data->sig_free_ptr = 0;
     hal_data->param_free_ptr = 0;
+    hal_data->struct_free_ptr = 0;
     hal_data->funct_free_ptr = 0;
     hal_data->pending_constructor = 0;
     hal_data->constructor_prefix[0] = 0;
@@ -3285,6 +3306,34 @@ static hal_param_t *alloc_param_struct(void)
     return p;
 }
 
+static hal_struct_entry_t *alloc_struct_entry(void)
+{
+    hal_struct_entry_t *p;
+
+    /* check the free list */
+    if (hal_data->struct_free_ptr != 0) {
+	p = SHMPTR(hal_data->struct_free_ptr);
+	hal_data->struct_free_ptr = p->next_ptr;
+	p->next_ptr = 0;
+    } else {
+	p = shmalloc_dn(sizeof(hal_struct_entry_t));
+    }
+    if (p) {
+	p->next_ptr = 0;
+	p->owner_ptr = 0;
+	p->data_ptr = 0;
+	p->attach_count = 0;
+	p->name[0] = '\0';
+    }
+    return p;
+}
+
+static void free_struct_entry(hal_struct_entry_t *p)
+{
+    p->next_ptr = hal_data->struct_free_ptr;
+    hal_data->struct_free_ptr = SHMOFF(p);
+}
+
 static hal_oldname_t *halpr_alloc_oldname_struct(void)
 {
     hal_oldname_t *p;
@@ -4356,19 +4405,20 @@ int hal_stream_num_underruns(hal_stream_t *stream) {
 }
 
 /* ========================================================================
- * Named struct API — allocates an opaque blob in HAL shmem and makes it
- * discoverable by name.  The blob is registered as a HAL_RO s32 param
- * whose value is the shmem offset of the data; hal_struct_attach() resolves
- * that offset back to a pointer.  No new shmem segment is created — the
- * allocation lives inside the existing HAL shmem block.
+ * Named struct API — allocates an opaque blob in HAL shmem and registers
+ * it in a dedicated struct namespace, separate from pins, signals, and
+ * parameters.  The struct list in hal_data tracks all entries by name and
+ * maintains a reference count so hal_struct_detach() is meaningful.
  * ======================================================================== */
 
 int hal_struct_newf(int comp_id, long int size, const void *defval,
                     const char *fmt, ...)
 {
     char name[HAL_NAME_LEN + 1];
-    int sz;
-    hal_s32_t *offset_holder;
+    int sz, cmp;
+    hal_comp_t *comp;
+    hal_struct_entry_t *new_entry, *ptr;
+    rtapi_intptr_t *prev, next;
     void *data;
     va_list ap;
 
@@ -4380,18 +4430,36 @@ int hal_struct_newf(int comp_id, long int size, const void *defval,
             "HAL: hal_struct_newf: name too long\n");
         return -ENOMEM;
     }
+    if (hal_data == 0) {
+        rtapi_print_msg(RTAPI_MSG_ERR,
+            "HAL: hal_struct_newf called before init\n");
+        return -EINVAL;
+    }
+
+    rtapi_mutex_get(&(hal_data->mutex));
+
+    comp = halpr_find_comp_by_id(comp_id);
+    if (!comp) {
+        rtapi_mutex_give(&(hal_data->mutex));
+        rtapi_print_msg(RTAPI_MSG_ERR,
+            "HAL: hal_struct_newf: component %d not found\n", comp_id);
+        return -EINVAL;
+    }
 
-    /* Allocate s32 to hold the shmem offset of the data */
-    offset_holder = (hal_s32_t *)hal_malloc(sizeof(hal_s32_t));
-    if (!offset_holder) {
+    /* Allocate the entry metadata (from shmalloc_dn, no extra mutex needed) */
+    new_entry = alloc_struct_entry();
+    if (!new_entry) {
+        rtapi_mutex_give(&(hal_data->mutex));
         rtapi_print_msg(RTAPI_MSG_ERR,
-            "HAL: hal_struct_newf: can't allocate offset holder\n");
+            "HAL: hal_struct_newf: insufficient memory for entry '%s'\n", name);
         return -ENOMEM;
     }
 
-    /* Allocate the struct data — hal_malloc aligns by size */
-    data = hal_malloc(size);
+    /* Allocate the data blob (shmalloc_up, mutex already held) */
+    data = shmalloc_up(size);
     if (!data) {
+        free_struct_entry(new_entry);
+        rtapi_mutex_give(&(hal_data->mutex));
         rtapi_print_msg(RTAPI_MSG_ERR,
             "HAL: hal_struct_newf: can't allocate %ld bytes for '%s'\n",
             size, name);
@@ -4403,18 +4471,45 @@ int hal_struct_newf(int comp_id, long int size, const void *defval,
     else
         memset(data, 0, (size_t)size);
 
-    /* Store offset so hal_struct_attach can find the data */
-    *offset_holder = (hal_s32_t)(((char *)data) - hal_shmem_base);
+    /* Fill in the entry */
+    new_entry->owner_ptr = SHMOFF(comp);
+    new_entry->data_ptr = SHMOFF(data);
+    new_entry->attach_count = 0;
+    rtapi_snprintf(new_entry->name, sizeof(new_entry->name), "%s", name);
 
-    /* Register as a HAL param so the name is in the HAL namespace */
-    return hal_param_s32_new(name, HAL_RO, offset_holder, comp_id);
+    /* Insert into struct list, sorted by name */
+    prev = &(hal_data->struct_list_ptr);
+    next = *prev;
+    while (1) {
+        if (next == 0) {
+            new_entry->next_ptr = next;
+            *prev = SHMOFF(new_entry);
+            rtapi_mutex_give(&(hal_data->mutex));
+            return 0;
+        }
+        ptr = SHMPTR(next);
+        cmp = strcmp(ptr->name, new_entry->name);
+        if (cmp > 0) {
+            new_entry->next_ptr = next;
+            *prev = SHMOFF(new_entry);
+            rtapi_mutex_give(&(hal_data->mutex));
+            return 0;
+        }
+        if (cmp == 0) {
+            free_struct_entry(new_entry);
+            rtapi_mutex_give(&(hal_data->mutex));
+            rtapi_print_msg(RTAPI_MSG_ERR,
+                "HAL: hal_struct_newf: duplicate name '%s'\n", name);
+            return -EINVAL;
+        }
+        prev = &(ptr->next_ptr);
+        next = *prev;
+    }
 }
 
 int hal_struct_attach(const char *name, void **memptr)
 {
-    hal_param_t *param;
-    void *dptr;
-    hal_s32_t offset;
+    hal_struct_entry_t *entry;
 
     if (hal_data == 0) {
         rtapi_print_msg(RTAPI_MSG_ERR,
@@ -4425,31 +4520,40 @@ int hal_struct_attach(const char *name, void **memptr)
         return -EINVAL;
 
     rtapi_mutex_get(&(hal_data->mutex));
-    param = halpr_find_param_by_name(name);
-    if (param == 0) {
+    entry = halpr_find_struct_by_name(name);
+    if (entry == 0) {
         rtapi_mutex_give(&(hal_data->mutex));
         rtapi_print_msg(RTAPI_MSG_ERR,
             "HAL: hal_struct_attach: '%s' not found\n", name);
         return -ENOENT;
     }
-    if (param->type != HAL_S32) {
-        rtapi_mutex_give(&(hal_data->mutex));
-        rtapi_print_msg(RTAPI_MSG_ERR,
-            "HAL: hal_struct_attach: '%s' is not a struct param\n", name);
-        return -EINVAL;
-    }
-    dptr = SHMPTR(param->data_ptr);
-    offset = *(hal_s32_t *)dptr;
+    entry->attach_count++;
+    *memptr = SHMPTR(entry->data_ptr);
     rtapi_mutex_give(&(hal_data->mutex));
-
-    *memptr = hal_shmem_base + offset;
     return 0;
 }
 
 int hal_struct_detach(const char *name)
 {
-    (void)name;
-    return 0; /* no-op: HAL shmem lifetime managed by creating component */
+    hal_struct_entry_t *entry;
+
+    if (hal_data == 0)
+        return -EINVAL;
+    if (!name)
+        return -EINVAL;
+
+    rtapi_mutex_get(&(hal_data->mutex));
+    entry = halpr_find_struct_by_name(name);
+    if (entry == 0) {
+        rtapi_mutex_give(&(hal_data->mutex));
+        rtapi_print_msg(RTAPI_MSG_ERR,
+            "HAL: hal_struct_detach: '%s' not found\n", name);
+        return -ENOENT;
+    }
+    if (entry->attach_count > 0)
+        entry->attach_count--;
+    rtapi_mutex_give(&(hal_data->mutex));
+    return 0;
 }
 
 #ifdef RTAPI
diff --git a/src/hal/hal_priv.h b/src/hal/hal_priv.h
@@ -118,7 +118,7 @@
 */
 
 #define HAL_KEY   0x48414C32	/* key used to open HAL shared memory */
-#define HAL_VER   0x00000010	/* version code */
+#define HAL_VER   0x00000011	/* version code — bumped for hal_struct namespace */
 #define HAL_SIZE  (256*4096)
 #define HAL_PSEUDO_COMP_PREFIX "__" /* prefix to identify a pseudo component */
 
@@ -233,6 +233,7 @@ typedef struct hal_comp_t hal_comp_t;
 typedef struct hal_pin_t hal_pin_t;
 typedef struct hal_sig_t hal_sig_t;
 typedef struct hal_param_t hal_param_t;
+typedef struct hal_struct_entry_t hal_struct_entry_t;
 typedef struct hal_funct_t hal_funct_t;
 typedef struct hal_funct_entry_t hal_funct_entry_t;
 typedef struct hal_thread_t hal_thread_t;
@@ -261,6 +262,7 @@ typedef struct hal_data_t {
     SHMFIELD(hal_pin_t) pin_list_ptr;		/* root of linked list of pins */
     SHMFIELD(hal_sig_t) sig_list_ptr;		/* root of linked list of signals */
     SHMFIELD(hal_param_t) param_list_ptr;		/* root of linked list of parameters */
+    SHMFIELD(hal_struct_entry_t) struct_list_ptr;	/* root of linked list of struct entries */
     SHMFIELD(hal_funct_t) funct_list_ptr;		/* root of linked list of functions */
     SHMFIELD(hal_thread_t) thread_list_ptr;	/* root of linked list of threads */
     long base_period;		/* timer period for realtime tasks */
@@ -270,6 +272,7 @@ typedef struct hal_data_t {
     SHMFIELD(hal_pin_t) pin_free_ptr;		/* list of free pin structs */
     SHMFIELD(hal_sig_t) sig_free_ptr;		/* list of free signal structs */
     SHMFIELD(hal_param_t) param_free_ptr;		/* list of free parameter structs */
+    SHMFIELD(hal_struct_entry_t) struct_free_ptr;	/* list of free struct entry structs */
     SHMFIELD(hal_funct_t) funct_free_ptr;		/* list of free function structs */
     hal_list_t funct_entry_free;	/* list of free funct entry structs */
     SHMFIELD(hal_thread_t) thread_free_ptr;	/* list of free thread structs */
@@ -347,6 +350,17 @@ struct hal_param_t {
     char name[HAL_NAME_LEN + 1];	/* parameter name */
 };
 
+/** HAL 'struct entry' — a named, ref-counted opaque blob in HAL shmem.
+    Lives in its own namespace, separate from pins, signals, and parameters.
+*/
+struct hal_struct_entry_t {
+    SHMFIELD(hal_struct_entry_t) next_ptr;  /* next entry in list */
+    SHMFIELD(hal_comp_t) owner_ptr;         /* component that created this entry */
+    SHMFIELD(void*) data_ptr;               /* offset of data in shmem */
+    int attach_count;                       /* number of active attachments */
+    char name[HAL_NAME_LEN + 1];            /* entry name */
+};
+
 /** the HAL uses functions and threads to handle synchronization of
     code.  In general, most control systems need to read inputs,
     perform control calculations, and write outputs, in that order.
@@ -449,6 +463,7 @@ extern hal_comp_t *halpr_find_comp_by_name(const char *name);
 extern hal_pin_t *halpr_find_pin_by_name(const char *name);
 extern hal_sig_t *halpr_find_sig_by_name(const char *name);
 extern hal_param_t *halpr_find_param_by_name(const char *name);
+extern hal_struct_entry_t *halpr_find_struct_by_name(const char *name);
 extern hal_thread_t *halpr_find_thread_by_name(const char *name);
 extern hal_funct_t *halpr_find_funct_by_name(const char *name);
 
