Skip to content

Commit cf54273

Browse files
grandixximograndixximo
committed
fix hal_struct: indentation, comp->ready check, duplicate/detach errors
Four fixes from review: - Fix indentation: new functions used 4-space instead of tabs - Add comp->ready check in hal_struct_newf, consistent with hal_param_new and hal_pin_new - Check for duplicate name before shmalloc_up: the bump allocator has no free, so allocating then detecting a duplicate would silently leak shmem - hal_struct_detach now returns -EINVAL when attach_count is already 0 instead of silently ignoring the over-detach
1 parent 1af95ba commit cf54273

1 file changed

Lines changed: 128 additions & 121 deletions

File tree

‎src/hal/hal_lib.c‎

Lines changed: 128 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -4412,148 +4412,155 @@ int hal_stream_num_underruns(hal_stream_t *stream) {
44124412
* ======================================================================== */
44134413

44144414
int hal_struct_newf(int comp_id, long int size, const void *defval,
4415-
const char *fmt, ...)
4415+
const char *fmt, ...)
44164416
{
4417-
char name[HAL_NAME_LEN + 1];
4418-
int sz, cmp;
4419-
hal_comp_t *comp;
4420-
hal_struct_entry_t *new_entry, *ptr;
4421-
rtapi_intptr_t *prev, next;
4422-
void *data;
4423-
va_list ap;
4417+
char name[HAL_NAME_LEN + 1];
4418+
int sz, cmp;
4419+
hal_comp_t *comp;
4420+
hal_struct_entry_t *new_entry, *ptr;
4421+
rtapi_intptr_t *prev, next;
4422+
void *data;
4423+
va_list ap;
4424+
4425+
va_start(ap, fmt);
4426+
sz = rtapi_vsnprintf(name, sizeof(name), fmt, ap);
4427+
va_end(ap);
4428+
if (sz < 0 || sz > HAL_NAME_LEN) {
4429+
rtapi_print_msg(RTAPI_MSG_ERR,
4430+
"HAL: hal_struct_newf: name too long\n");
4431+
return -ENOMEM;
4432+
}
4433+
if (hal_data == 0) {
4434+
rtapi_print_msg(RTAPI_MSG_ERR,
4435+
"HAL: hal_struct_newf called before init\n");
4436+
return -EINVAL;
4437+
}
44244438

4425-
va_start(ap, fmt);
4426-
sz = rtapi_vsnprintf(name, sizeof(name), fmt, ap);
4427-
va_end(ap);
4428-
if (sz < 0 || sz > HAL_NAME_LEN) {
4429-
rtapi_print_msg(RTAPI_MSG_ERR,
4430-
"HAL: hal_struct_newf: name too long\n");
4431-
return -ENOMEM;
4432-
}
4433-
if (hal_data == 0) {
4434-
rtapi_print_msg(RTAPI_MSG_ERR,
4435-
"HAL: hal_struct_newf called before init\n");
4436-
return -EINVAL;
4437-
}
4439+
rtapi_mutex_get(&(hal_data->mutex));
44384440

4439-
rtapi_mutex_get(&(hal_data->mutex));
4441+
comp = halpr_find_comp_by_id(comp_id);
4442+
if (!comp) {
4443+
rtapi_mutex_give(&(hal_data->mutex));
4444+
rtapi_print_msg(RTAPI_MSG_ERR,
4445+
"HAL: hal_struct_newf: component %d not found\n", comp_id);
4446+
return -EINVAL;
4447+
}
4448+
if (comp->ready) {
4449+
rtapi_mutex_give(&(hal_data->mutex));
4450+
rtapi_print_msg(RTAPI_MSG_ERR,
4451+
"HAL: hal_struct_newf called after hal_ready\n");
4452+
return -EINVAL;
4453+
}
44404454

4441-
comp = halpr_find_comp_by_id(comp_id);
4442-
if (!comp) {
4443-
rtapi_mutex_give(&(hal_data->mutex));
4444-
rtapi_print_msg(RTAPI_MSG_ERR,
4445-
"HAL: hal_struct_newf: component %d not found\n", comp_id);
4446-
return -EINVAL;
4447-
}
4455+
/* Check for duplicate name before allocating the data blob.
4456+
* shmalloc_up has no corresponding free, so we must not allocate
4457+
* memory we cannot use. */
4458+
prev = &(hal_data->struct_list_ptr);
4459+
next = *prev;
4460+
while (next != 0) {
4461+
ptr = SHMPTR(next);
4462+
cmp = strcmp(ptr->name, name);
4463+
if (cmp == 0) {
4464+
rtapi_mutex_give(&(hal_data->mutex));
4465+
rtapi_print_msg(RTAPI_MSG_ERR,
4466+
"HAL: hal_struct_newf: duplicate name '%s'\n", name);
4467+
return -EINVAL;
4468+
}
4469+
if (cmp > 0)
4470+
break; /* insertion point found */
4471+
prev = &(ptr->next_ptr);
4472+
next = *prev;
4473+
}
44484474

4449-
/* Allocate the entry metadata (from shmalloc_dn, no extra mutex needed) */
4450-
new_entry = alloc_struct_entry();
4451-
if (!new_entry) {
4452-
rtapi_mutex_give(&(hal_data->mutex));
4453-
rtapi_print_msg(RTAPI_MSG_ERR,
4454-
"HAL: hal_struct_newf: insufficient memory for entry '%s'\n", name);
4455-
return -ENOMEM;
4456-
}
4475+
/* Allocate the entry metadata */
4476+
new_entry = alloc_struct_entry();
4477+
if (!new_entry) {
4478+
rtapi_mutex_give(&(hal_data->mutex));
4479+
rtapi_print_msg(RTAPI_MSG_ERR,
4480+
"HAL: hal_struct_newf: insufficient memory for entry '%s'\n", name);
4481+
return -ENOMEM;
4482+
}
44574483

4458-
/* Allocate the data blob (shmalloc_up, mutex already held) */
4459-
data = shmalloc_up(size);
4460-
if (!data) {
4461-
free_struct_entry(new_entry);
4462-
rtapi_mutex_give(&(hal_data->mutex));
4463-
rtapi_print_msg(RTAPI_MSG_ERR,
4464-
"HAL: hal_struct_newf: can't allocate %ld bytes for '%s'\n",
4465-
size, name);
4466-
return -ENOMEM;
4467-
}
4484+
/* Allocate the data blob — shmalloc_up, mutex already held */
4485+
data = shmalloc_up(size);
4486+
if (!data) {
4487+
free_struct_entry(new_entry);
4488+
rtapi_mutex_give(&(hal_data->mutex));
4489+
rtapi_print_msg(RTAPI_MSG_ERR,
4490+
"HAL: hal_struct_newf: can't allocate %ld bytes for '%s'\n",
4491+
size, name);
4492+
return -ENOMEM;
4493+
}
44684494

4469-
if (defval)
4470-
memcpy(data, defval, (size_t)size);
4471-
else
4472-
memset(data, 0, (size_t)size);
4495+
if (defval)
4496+
memcpy(data, defval, (size_t)size);
4497+
else
4498+
memset(data, 0, (size_t)size);
44734499

4474-
/* Fill in the entry */
4475-
new_entry->owner_ptr = SHMOFF(comp);
4476-
new_entry->data_ptr = SHMOFF(data);
4477-
new_entry->attach_count = 0;
4478-
rtapi_snprintf(new_entry->name, sizeof(new_entry->name), "%s", name);
4500+
/* Fill in and insert the entry at the position found above */
4501+
new_entry->owner_ptr = SHMOFF(comp);
4502+
new_entry->data_ptr = SHMOFF(data);
4503+
new_entry->attach_count = 0;
4504+
rtapi_snprintf(new_entry->name, sizeof(new_entry->name), "%s", name);
4505+
new_entry->next_ptr = next;
4506+
*prev = SHMOFF(new_entry);
44794507

4480-
/* Insert into struct list, sorted by name */
4481-
prev = &(hal_data->struct_list_ptr);
4482-
next = *prev;
4483-
while (1) {
4484-
if (next == 0) {
4485-
new_entry->next_ptr = next;
4486-
*prev = SHMOFF(new_entry);
4487-
rtapi_mutex_give(&(hal_data->mutex));
4488-
return 0;
4489-
}
4490-
ptr = SHMPTR(next);
4491-
cmp = strcmp(ptr->name, new_entry->name);
4492-
if (cmp > 0) {
4493-
new_entry->next_ptr = next;
4494-
*prev = SHMOFF(new_entry);
4495-
rtapi_mutex_give(&(hal_data->mutex));
4496-
return 0;
4497-
}
4498-
if (cmp == 0) {
4499-
free_struct_entry(new_entry);
4500-
rtapi_mutex_give(&(hal_data->mutex));
4501-
rtapi_print_msg(RTAPI_MSG_ERR,
4502-
"HAL: hal_struct_newf: duplicate name '%s'\n", name);
4503-
return -EINVAL;
4504-
}
4505-
prev = &(ptr->next_ptr);
4506-
next = *prev;
4507-
}
4508+
rtapi_mutex_give(&(hal_data->mutex));
4509+
return 0;
45084510
}
45094511

45104512
int hal_struct_attach(const char *name, void **memptr)
45114513
{
4512-
hal_struct_entry_t *entry;
4514+
hal_struct_entry_t *entry;
45134515

4514-
if (hal_data == 0) {
4515-
rtapi_print_msg(RTAPI_MSG_ERR,
4516-
"HAL: hal_struct_attach called before init\n");
4517-
return -EINVAL;
4518-
}
4519-
if (!name || !memptr)
4520-
return -EINVAL;
4516+
if (hal_data == 0) {
4517+
rtapi_print_msg(RTAPI_MSG_ERR,
4518+
"HAL: hal_struct_attach called before init\n");
4519+
return -EINVAL;
4520+
}
4521+
if (!name || !memptr)
4522+
return -EINVAL;
45214523

4522-
rtapi_mutex_get(&(hal_data->mutex));
4523-
entry = halpr_find_struct_by_name(name);
4524-
if (entry == 0) {
4525-
rtapi_mutex_give(&(hal_data->mutex));
4526-
rtapi_print_msg(RTAPI_MSG_ERR,
4527-
"HAL: hal_struct_attach: '%s' not found\n", name);
4528-
return -ENOENT;
4529-
}
4530-
entry->attach_count++;
4531-
*memptr = SHMPTR(entry->data_ptr);
4532-
rtapi_mutex_give(&(hal_data->mutex));
4533-
return 0;
4524+
rtapi_mutex_get(&(hal_data->mutex));
4525+
entry = halpr_find_struct_by_name(name);
4526+
if (entry == 0) {
4527+
rtapi_mutex_give(&(hal_data->mutex));
4528+
rtapi_print_msg(RTAPI_MSG_ERR,
4529+
"HAL: hal_struct_attach: '%s' not found\n", name);
4530+
return -ENOENT;
4531+
}
4532+
entry->attach_count++;
4533+
*memptr = SHMPTR(entry->data_ptr);
4534+
rtapi_mutex_give(&(hal_data->mutex));
4535+
return 0;
45344536
}
45354537

45364538
int hal_struct_detach(const char *name)
45374539
{
4538-
hal_struct_entry_t *entry;
4540+
hal_struct_entry_t *entry;
45394541

4540-
if (hal_data == 0)
4541-
return -EINVAL;
4542-
if (!name)
4543-
return -EINVAL;
4542+
if (hal_data == 0)
4543+
return -EINVAL;
4544+
if (!name)
4545+
return -EINVAL;
45444546

4545-
rtapi_mutex_get(&(hal_data->mutex));
4546-
entry = halpr_find_struct_by_name(name);
4547-
if (entry == 0) {
4548-
rtapi_mutex_give(&(hal_data->mutex));
4549-
rtapi_print_msg(RTAPI_MSG_ERR,
4550-
"HAL: hal_struct_detach: '%s' not found\n", name);
4551-
return -ENOENT;
4552-
}
4553-
if (entry->attach_count > 0)
4554-
entry->attach_count--;
4555-
rtapi_mutex_give(&(hal_data->mutex));
4556-
return 0;
4547+
rtapi_mutex_get(&(hal_data->mutex));
4548+
entry = halpr_find_struct_by_name(name);
4549+
if (entry == 0) {
4550+
rtapi_mutex_give(&(hal_data->mutex));
4551+
rtapi_print_msg(RTAPI_MSG_ERR,
4552+
"HAL: hal_struct_detach: '%s' not found\n", name);
4553+
return -ENOENT;
4554+
}
4555+
if (entry->attach_count == 0) {
4556+
rtapi_mutex_give(&(hal_data->mutex));
4557+
rtapi_print_msg(RTAPI_MSG_ERR,
4558+
"HAL: hal_struct_detach: '%s' has no attachments\n", name);
4559+
return -EINVAL;
4560+
}
4561+
entry->attach_count--;
4562+
rtapi_mutex_give(&(hal_data->mutex));
4563+
return 0;
45574564
}
45584565

45594566
#ifdef RTAPI

0 commit comments

Comments
 (0)