Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Misc cleanups #147

Merged
merged 9 commits into from
Sep 27, 2023
Merged

Misc cleanups #147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a486a0c
resource: ubridge: initialize lists early in sid_resource_create
bmarzins Aug 17, 2023
4a23c36
iface: service-link: remove unnecessary return codes
bmarzins Aug 17, 2023
6439caa
resource: ubridge: close fd_pass on _init_connection failure
bmarzins Aug 21, 2023
15b6c96
resource: ubridge: verify that message size before checking header
bmarzins Aug 21, 2023
bc5ea5c
resource: ubridge: Always allocate enough space for the vvalue header.
bmarzins Aug 29, 2023
bc6275d
resource: ubridge: free dirents on failure when refreshing disks
bmarzins Aug 29, 2023
6b166ff
resource: ubridge: free unused key when refreshing disks
bmarzins Aug 29, 2023
25d03de
resource: ubridge: free key on partition refresh failure
bmarzins Aug 29, 2023
f1cdc3a
ucmd: module: dm: don't write garbage keys
bmarzins Sep 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions src/iface/service-link.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,16 +70,14 @@ void service_link_destroy(struct service_link *sl)
free(sl);
}

int service_link_add_notification(struct service_link *sl, service_link_notification_t notification)
void service_link_add_notification(struct service_link *sl, service_link_notification_t notification)
{
sl->notification |= notification;
return 0;
}

int service_link_remove_notification(struct service_link *sl, service_link_notification_t notification)
void service_link_remove_notification(struct service_link *sl, service_link_notification_t notification)
{
sl->notification &= ~notification;
return 0;
}

struct service_link_group *service_link_group_create(const char *name)
Expand Down Expand Up @@ -115,12 +113,10 @@ void service_link_group_destroy_with_members(struct service_link_group *slg)
free(slg);
}

int service_link_group_add_member(struct service_link_group *slg, struct service_link *sl)
void service_link_group_add_member(struct service_link_group *slg, struct service_link *sl)
{
list_add(&slg->members, &sl->list);
sl->group = slg;

return 0;
}

int service_link_group_remove_member(struct service_link_group *slg, struct service_link *sl)
Expand Down Expand Up @@ -150,7 +146,7 @@ static const char *_get_arg_value(const char *str, const char *key_eq, size_t *s
if (!strncmp(key_eq, str, strlen(key_eq))) {
/* get the value and its size */
str += strlen(key_eq);
*size = line_end - str;
*size = line_end - str;
return str;
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/include/iface/service-link.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,15 @@ struct service_link_group;
struct service_link *service_link_create(service_link_type_t type, const char *name);
void service_link_destroy(struct service_link *sl);

int service_link_add_notification(struct service_link *sl, service_link_notification_t notification);
int service_link_remove_notification(struct service_link *sl, service_link_notification_t notification);
void service_link_add_notification(struct service_link *sl, service_link_notification_t notification);
void service_link_remove_notification(struct service_link *sl, service_link_notification_t notification);

struct service_link_group *service_link_group_create(const char *name);
void service_link_group_destroy(struct service_link_group *slg);
void service_link_group_destroy_with_members(struct service_link_group *slg);

int service_link_group_add_member(struct service_link_group *slg, struct service_link *sl);
int service_link_group_remove_member(struct service_link_group *slg, struct service_link *sl);
void service_link_group_add_member(struct service_link_group *slg, struct service_link *sl);
int service_link_group_remove_member(struct service_link_group *slg, struct service_link *sl);

/*
* Send service notification.
Expand Down
10 changes: 8 additions & 2 deletions src/modules/ucmd/type/dm/dm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,18 @@ static int _dm_ident(struct module *module, struct sid_ucmd_ctx *ucmd_ctx)
log_debug(DM_ID, "ident");

snprintf(path, sizeof(path), "%s%s/dm/uuid", SYSTEM_SYSFS_PATH, sid_ucmd_event_get_dev_path(ucmd_ctx));
sid_util_sysfs_get_value(path, uuid, sizeof(uuid));
if (sid_util_sysfs_get_value(path, uuid, sizeof(uuid)) < 0) {
log_error(DM_ID, "Failed to get DM uuid.");
return -1;
}
sid_ucmd_dev_add_alias(module, ucmd_ctx, "uuid", uuid);
sid_ucmd_set_kv(module, ucmd_ctx, KV_NS_DEVMOD, "uuid", uuid, strlen(uuid) + 1, KV_SYNC | KV_SUB_RD);

snprintf(path, sizeof(path), "%s%s/dm/name", SYSTEM_SYSFS_PATH, sid_ucmd_event_get_dev_path(ucmd_ctx));
sid_util_sysfs_get_value(path, name, sizeof(name));
if (sid_util_sysfs_get_value(path, name, sizeof(name)) < 0) {
log_error(DM_ID, "Failed to get DM name.");
return -1;
}
sid_ucmd_dev_add_alias(module, ucmd_ctx, "name", name);
sid_ucmd_set_kv(module, ucmd_ctx, KV_NS_DEVMOD, "name", name, strlen(name) + 1, KV_SYNC | KV_SUB_RD);

Expand Down
14 changes: 5 additions & 9 deletions src/resource/resource.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,8 @@ static int _create_service_link_group(sid_resource_t *res, sid_resource_service_
goto out;
}

if ((r = service_link_add_notification(sl, def->notification)) < 0)
goto out;

if ((r = service_link_group_add_member(slg, sl)) < 0)
goto out;
service_link_add_notification(sl, def->notification);
service_link_group_add_member(slg, sl);
}

res->slg = slg;
Expand Down Expand Up @@ -264,6 +261,8 @@ sid_resource_t *sid_resource_create(sid_resource_t *parent_res,
goto fail;

res->id = id;
list_init(&res->children);
list_init(&res->event_sources);

/*
* Take temporary reference!
Expand All @@ -280,8 +279,7 @@ sid_resource_t *sid_resource_create(sid_resource_t *parent_res,
if (_create_service_link_group(res, service_link_defs) < 0)
goto fail;

res->flags = flags;
list_init(&res->children);
res->flags = flags;
res->type = type;
res->prio = prio;
res->event_loop.sd_event_loop = NULL;
Expand All @@ -291,8 +289,6 @@ sid_resource_t *sid_resource_create(sid_resource_t *parent_res,
if (type->with_event_loop && sd_event_new(&res->event_loop.sd_event_loop) < 0)
goto fail;

list_init(&res->event_sources);

_add_res_to_parent_res(res, parent_res);

if (type->with_event_loop && type->with_watchdog && sd_event_set_watchdog(res->event_loop.sd_event_loop, 1) < 0)
Expand Down
54 changes: 31 additions & 23 deletions src/resource/ubridge.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3667,12 +3667,13 @@ static int _refresh_device_disk_hierarchy_from_sysfs(sid_resource_t *cmd_res)
* +VVALUE_HEADER_CNT to include record header
* -2 to subtract "." and ".." directory which we're not interested in
*/
if (!(vec_buf = sid_buffer_create(
&((struct sid_buffer_spec) {.backend = SID_BUFFER_BACKEND_MALLOC,
.type = SID_BUFFER_TYPE_VECTOR,
.mode = SID_BUFFER_MODE_PLAIN}),
&((struct sid_buffer_init) {.size = count + VVALUE_HEADER_CNT - 2, .alloc_step = 1, .limit = 0}),
&r))) {
if (!(vec_buf = sid_buffer_create(&((struct sid_buffer_spec) {.backend = SID_BUFFER_BACKEND_MALLOC,
.type = SID_BUFFER_TYPE_VECTOR,
.mode = SID_BUFFER_MODE_PLAIN}),
&((struct sid_buffer_init) {.size = VVALUE_HEADER_CNT + (count >= 2 ? count - 2 : 0),
.alloc_step = 1,
.limit = 0}),
&r))) {
log_error_errno(ID(cmd_res),
r,
"Failed to create buffer to record hierarchy for device " CMD_DEV_NAME_NUM_FMT,
Expand All @@ -3691,10 +3692,8 @@ static int _refresh_device_disk_hierarchy_from_sysfs(sid_resource_t *cmd_res)
/* Read relatives from sysfs into vec_buf. */
if (ucmd_ctx->req_env.dev.udev.action != UDEV_ACTION_REMOVE) {
for (i = 0; i < count; i++) {
if (dirent[i]->d_name[0] == '.') {
free(dirent[i]);
continue;
}
if (dirent[i]->d_name[0] == '.')
goto next;

if (sid_buffer_fmt_add(ucmd_ctx->common->gen_buf,
(const void **) &s,
Expand All @@ -3717,7 +3716,7 @@ static int _refresh_device_disk_hierarchy_from_sysfs(sid_resource_t *cmd_res)
"Failed to read related disk device number from sysfs file %s.",
s);
sid_buffer_rewind_mem(ucmd_ctx->common->gen_buf, s);
continue;
goto next;
}
sid_buffer_rewind_mem(ucmd_ctx->common->gen_buf, s);

Expand All @@ -3729,7 +3728,7 @@ static int _refresh_device_disk_hierarchy_from_sysfs(sid_resource_t *cmd_res)
log_error(ID(cmd_res),
"Failed to generate UUID for device " CMD_DEV_NAME_NUM_FMT ".",
CMD_DEV_NAME_NUM(ucmd_ctx));
continue;
goto next;
}
rel_spec.rel_key_spec->ns_part = mem.base;

Expand All @@ -3744,18 +3743,21 @@ static int _refresh_device_disk_hierarchy_from_sysfs(sid_resource_t *cmd_res)
log_error(ID(cmd_res),
"Failed to add devno alias for device " CMD_DEV_NAME_NUM_FMT,
CMD_DEV_NAME_NUM(ucmd_ctx));
continue;
goto next;
}
}

s = _compose_key_prefix(NULL, rel_spec.rel_key_spec);
if (!s || ((r = sid_buffer_add(vec_buf, (void *) s, strlen(s) + 1, NULL, NULL)) < 0))
if (!s || ((r = sid_buffer_add(vec_buf, (void *) s, strlen(s) + 1, NULL, NULL)) < 0)) {
_destroy_key(NULL, s);
goto out;
}
}

next:
free(dirent[i]);
}
free(dirent);
dirent = NULL;
rel_spec.rel_key_spec->ns_part = ID_NULL;
}

Expand All @@ -3777,6 +3779,11 @@ static int _refresh_device_disk_hierarchy_from_sysfs(sid_resource_t *cmd_res)
_destroy_key(NULL, s);
r = 0;
out:
if (dirent) {
for (; i < count; i++)
free(dirent[i]);
free(dirent);
}
if (vec_buf) {
if (!vsize)
sid_buffer_get_data(vec_buf, (const void **) (&vvalue), &vsize);
Expand All @@ -3795,8 +3802,8 @@ static int _refresh_device_partition_hierarchy_from_sysfs(sid_resource_t *cmd_re
kv_vector_t vvalue[VVALUE_SINGLE_CNT];
char devno_buf[16];
char devid_buf[UTIL_UUID_STR_SIZE];
const char *s;
char *key;
const char *s = NULL;
char *key = NULL;
util_mem_t mem;
int r = -1;

Expand Down Expand Up @@ -3869,10 +3876,10 @@ static int _refresh_device_partition_hierarchy_from_sysfs(sid_resource_t *cmd_re
*/
_kv_delta_set(key, vvalue, VVALUE_SINGLE_CNT, &update_arg, true);

_destroy_key(NULL, key);
_destroy_key(NULL, s);
r = 0;
out:
_destroy_key(NULL, key);
_destroy_key(NULL, s);
return r;
}

Expand Down Expand Up @@ -4594,13 +4601,13 @@ static int _check_msg(sid_resource_t *res, struct sid_msg *msg)
{
struct sid_msg_header header;

memcpy(&header, msg->header, sizeof(header));

if (msg->size < sizeof(struct sid_msg_header)) {
log_error(ID(res), "Incorrect message header size.");
return -1;
}

memcpy(&header, msg->header, sizeof(header));

/* Sanitize command number - map all out of range command numbers to CMD_UNKNOWN. */
switch (msg->cat) {
case MSG_CATEGORY_SYSTEM:
Expand Down Expand Up @@ -4638,11 +4645,11 @@ static int _create_command_resource(sid_resource_t *parent_res, struct sid_msg *
{
struct sid_msg_header header;

memcpy(&header, msg->header, sizeof(header));

if (_check_msg(parent_res, msg) < 0)
return -1;

memcpy(&header, msg->header, sizeof(header));

if (!sid_resource_create(parent_res,
&sid_resource_type_ubridge_command,
SID_RESOURCE_NO_FLAGS,
Expand Down Expand Up @@ -4737,6 +4744,7 @@ static int _init_connection(sid_resource_t *res, const void *kickstart_data, voi
sid_buffer_destroy(conn->buf);
free(conn);
}
close(data_spec->ext.socket.fd_pass);
return -1;
}

Expand Down
14 changes: 7 additions & 7 deletions tests/test_notify.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ static void test_notify_ready(void **state)
struct service_link *sl = service_link_create(SERVICE_TYPE_SYSTEMD, "systemd");

assert_non_null(sl);
assert_int_equal(service_link_add_notification(sl, SERVICE_NOTIFICATION_READY), 0);
service_link_add_notification(sl, SERVICE_NOTIFICATION_READY);
will_return(__wrap_sd_notify, "READY=1\n");
assert_int_equal(service_link_notify(sl, SERVICE_NOTIFICATION_READY, NULL), 0);
service_link_destroy(sl);
Expand All @@ -39,8 +39,8 @@ static void test_notify_ready_reloading(void **state)
struct service_link *sl = service_link_create(SERVICE_TYPE_SYSTEMD, "systemd");

assert_non_null(sl);
assert_int_equal(service_link_add_notification(sl, SERVICE_NOTIFICATION_READY), 0);
assert_int_equal(service_link_add_notification(sl, SERVICE_NOTIFICATION_RELOADING), 0);
service_link_add_notification(sl, SERVICE_NOTIFICATION_READY);
service_link_add_notification(sl, SERVICE_NOTIFICATION_RELOADING);
will_return(__wrap_sd_notify, "READY=1\nRELOADING=1\n");
assert_int_equal(service_link_notify(sl, SERVICE_NOTIFICATION_READY | SERVICE_NOTIFICATION_RELOADING, NULL), 0);
service_link_destroy(sl);
Expand All @@ -51,7 +51,7 @@ static void test_notify_blank(void **state)
struct service_link *sl = service_link_create(SERVICE_TYPE_SYSTEMD, "systemd");

assert_non_null(sl);
assert_int_equal(service_link_add_notification(sl, SERVICE_NOTIFICATION_STATUS), 0);
service_link_add_notification(sl, SERVICE_NOTIFICATION_STATUS);
will_return(__wrap_sd_notify, "");
assert_int_equal(service_link_notify(sl, SERVICE_NOTIFICATION_STATUS, NULL), 0);
service_link_destroy(sl);
Expand All @@ -62,7 +62,7 @@ static void test_notify_errno(void **state)
struct service_link *sl = service_link_create(SERVICE_TYPE_SYSTEMD, "systemd");

assert_non_null(sl);
assert_int_equal(service_link_add_notification(sl, SERVICE_NOTIFICATION_ERRNO), 0);
service_link_add_notification(sl, SERVICE_NOTIFICATION_ERRNO);
will_return(__wrap_sd_notify, "ERRNO=2\n");
assert_int_equal(service_link_notify(sl, SERVICE_NOTIFICATION_ERRNO, "ERRNO=%d\n", 2), 0);
service_link_destroy(sl);
Expand All @@ -73,8 +73,8 @@ static void test_notify_errno_status(void **state)
struct service_link *sl = service_link_create(SERVICE_TYPE_SYSTEMD, "systemd");

assert_non_null(sl);
assert_int_equal(service_link_add_notification(sl, SERVICE_NOTIFICATION_ERRNO), 0);
assert_int_equal(service_link_add_notification(sl, SERVICE_NOTIFICATION_STATUS), 0);
service_link_add_notification(sl, SERVICE_NOTIFICATION_ERRNO);
service_link_add_notification(sl, SERVICE_NOTIFICATION_STATUS);
will_return(__wrap_sd_notify, "STATUS=testing\nERRNO=2\n");
assert_int_equal(
service_link_notify(sl, SERVICE_NOTIFICATION_ERRNO | SERVICE_NOTIFICATION_STATUS, "ERRNO=%d\nSTATUS=testing", 2),
Expand Down
Loading