From a03d417592d1fa02cb427d1ce239bad1e4f42c90 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 16 Feb 2022 12:28:53 -0500 Subject: [PATCH] lib: add enum bt_resolve_field_xref_status Add the bt_resolve_field_xref_status enumeration, and use it throughout the resolve-field-path.c file to make error handling more explicit, and clearer, in my opinion. Change-Id: I51ddb22ac41dde0f58ca2e5d7fb90267984412e9 Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/7304 Tested-by: jenkins Reviewed-by: Philippe Proulx Reviewed-on: https://review.lttng.org/c/babeltrace/+/7328 --- src/lib/trace-ir/event-class.c | 36 ++++---- src/lib/trace-ir/resolve-field-path.c | 120 +++++++++++++++----------- src/lib/trace-ir/resolve-field-path.h | 2 +- src/lib/trace-ir/resolve-field-xref.h | 5 ++ src/lib/trace-ir/stream-class.c | 36 ++++---- 5 files changed, 110 insertions(+), 89 deletions(-) diff --git a/src/lib/trace-ir/event-class.c b/src/lib/trace-ir/event-class.c index 63ab7133..12bb1120 100644 --- a/src/lib/trace-ir/event-class.c +++ b/src/lib/trace-ir/event-class.c @@ -279,7 +279,8 @@ bt_event_class_set_specific_context_field_class( struct bt_event_class *event_class, struct bt_field_class *field_class) { - int ret; + enum bt_event_class_set_field_class_status status; + enum bt_resolve_field_xref_status resolve_status; struct bt_stream_class *stream_class; struct bt_resolve_field_xref_context resolve_ctx = { .packet_context = NULL, @@ -300,14 +301,9 @@ bt_event_class_set_specific_context_field_class( resolve_ctx.event_common_context = stream_class->event_common_context_fc; - ret = bt_resolve_field_paths(field_class, &resolve_ctx, __func__); - if (ret) { - /* - * This is the only reason for which - * bt_resolve_field_paths() can fail: anything else - * would be because a precondition is not satisfied. - */ - ret = BT_FUNC_STATUS_MEMORY_ERROR; + resolve_status = bt_resolve_field_paths(field_class, &resolve_ctx, __func__); + if (resolve_status != BT_RESOLVE_FIELD_XREF_STATUS_OK) { + status = (int) resolve_status; goto end; } @@ -319,8 +315,10 @@ bt_event_class_set_specific_context_field_class( BT_LIB_LOGD("Set event class's specific context field class: %!+E", event_class); + status = BT_EVENT_CLASS_SET_FIELD_CLASS_STATUS_OK; + end: - return ret; + return status; } BT_EXPORT @@ -345,7 +343,8 @@ bt_event_class_set_payload_field_class( struct bt_event_class *event_class, struct bt_field_class *field_class) { - int ret; + enum bt_event_class_set_field_class_status status; + enum bt_resolve_field_xref_status resolve_status; struct bt_stream_class *stream_class; struct bt_resolve_field_xref_context resolve_ctx = { .packet_context = NULL, @@ -366,14 +365,9 @@ bt_event_class_set_payload_field_class( stream_class->event_common_context_fc; resolve_ctx.event_specific_context = event_class->specific_context_fc; - ret = bt_resolve_field_paths(field_class, &resolve_ctx, __func__); - if (ret) { - /* - * This is the only reason for which - * bt_resolve_field_paths() can fail: anything else - * would be because a precondition is not satisfied. - */ - ret = BT_FUNC_STATUS_MEMORY_ERROR; + resolve_status = bt_resolve_field_paths(field_class, &resolve_ctx, __func__); + if (resolve_status != BT_RESOLVE_FIELD_XREF_STATUS_OK) { + status = (int) resolve_status; goto end; } @@ -384,8 +378,10 @@ bt_event_class_set_payload_field_class( bt_field_class_freeze(field_class); BT_LIB_LOGD("Set event class's payload field class: %!+E", event_class); + status = BT_EVENT_CLASS_SET_FIELD_CLASS_STATUS_OK; + end: - return ret; + return status; } void _bt_event_class_freeze(const struct bt_event_class *event_class) diff --git a/src/lib/trace-ir/resolve-field-path.c b/src/lib/trace-ir/resolve-field-path.c index 0cbbaf93..3d4814bc 100644 --- a/src/lib/trace-ir/resolve-field-path.c +++ b/src/lib/trace-ir/resolve-field-path.c @@ -91,20 +91,23 @@ end: } static -int find_field_class(struct bt_field_class *root_fc, - enum bt_field_path_scope root_scope, struct bt_field_class *tgt_fc, +enum bt_resolve_field_xref_status find_field_class( + struct bt_field_class *root_fc, + enum bt_field_path_scope root_scope, + struct bt_field_class *tgt_fc, struct bt_field_path **ret_field_path) { - int ret = 0; + enum bt_resolve_field_xref_status ret; struct bt_field_path *field_path = NULL; if (!root_fc) { + ret = BT_RESOLVE_FIELD_XREF_STATUS_OK; goto end; } field_path = bt_field_path_create(); if (!field_path) { - ret = -1; + ret = BT_RESOLVE_FIELD_XREF_STATUS_MEMORY_ERROR; goto end; } @@ -114,44 +117,49 @@ int find_field_class(struct bt_field_class *root_fc, BT_OBJECT_PUT_REF_AND_RESET(field_path); } + ret = BT_RESOLVE_FIELD_XREF_STATUS_OK; + end: *ret_field_path = field_path; return ret; } static -struct bt_field_path *find_field_class_in_ctx(struct bt_field_class *fc, - struct bt_resolve_field_xref_context *ctx) +enum bt_resolve_field_xref_status find_field_class_in_ctx( + struct bt_field_class *fc, + struct bt_resolve_field_xref_context *ctx, + struct bt_field_path **ret_field_path) { - struct bt_field_path *field_path = NULL; - int ret; + enum bt_resolve_field_xref_status ret; + + *ret_field_path = NULL; - ret = find_field_class(ctx->packet_context, BT_FIELD_PATH_SCOPE_PACKET_CONTEXT, - fc, &field_path); - if (ret || field_path) { + ret = find_field_class(ctx->packet_context, + BT_FIELD_PATH_SCOPE_PACKET_CONTEXT, fc, ret_field_path); + if (ret != BT_RESOLVE_FIELD_XREF_STATUS_OK || *ret_field_path) { goto end; } ret = find_field_class(ctx->event_common_context, - BT_FIELD_PATH_SCOPE_EVENT_COMMON_CONTEXT, fc, &field_path); - if (ret || field_path) { + BT_FIELD_PATH_SCOPE_EVENT_COMMON_CONTEXT, fc, ret_field_path); + if (ret != BT_RESOLVE_FIELD_XREF_STATUS_OK || *ret_field_path) { goto end; } ret = find_field_class(ctx->event_specific_context, - BT_FIELD_PATH_SCOPE_EVENT_SPECIFIC_CONTEXT, fc, &field_path); - if (ret || field_path) { + BT_FIELD_PATH_SCOPE_EVENT_SPECIFIC_CONTEXT, fc, ret_field_path); + if (ret != BT_RESOLVE_FIELD_XREF_STATUS_OK || *ret_field_path) { goto end; } - ret = find_field_class(ctx->event_payload, BT_FIELD_PATH_SCOPE_EVENT_PAYLOAD, - fc, &field_path); - if (ret || field_path) { + ret = find_field_class(ctx->event_payload, + BT_FIELD_PATH_SCOPE_EVENT_PAYLOAD, fc, ret_field_path); + if (ret != BT_RESOLVE_FIELD_XREF_STATUS_OK || *ret_field_path) { goto end; } end: - return field_path; + return ret; } BT_ASSERT_COND_DEV_FUNC @@ -422,11 +430,13 @@ bool field_path_is_valid(struct bt_field_class *src_fc, struct bt_field_class *tgt_fc, struct bt_resolve_field_xref_context *ctx) { - bool is_valid = true; - struct bt_field_path *src_field_path = find_field_class_in_ctx( - src_fc, ctx); - struct bt_field_path *tgt_field_path = find_field_class_in_ctx( - tgt_fc, ctx); + struct bt_field_path *src_field_path; + struct bt_field_path *tgt_field_path = NULL; + enum bt_resolve_field_xref_status status; + bool is_valid; + + status = find_field_class_in_ctx(src_fc, ctx, &src_field_path); + BT_ASSERT(status == BT_RESOLVE_FIELD_XREF_STATUS_OK); if (!src_field_path) { BT_ASSERT_COND_DEV_MSG("Cannot find requesting field class in " @@ -435,6 +445,9 @@ bool field_path_is_valid(struct bt_field_class *src_fc, goto end; } + status = find_field_class_in_ctx(tgt_fc, ctx, &tgt_field_path); + BT_ASSERT(status == BT_RESOLVE_FIELD_XREF_STATUS_OK); + if (!tgt_field_path) { BT_ASSERT_COND_DEV_MSG("Cannot find target field class in " "resolving context: %!+F", tgt_fc); @@ -487,6 +500,8 @@ bool field_path_is_valid(struct bt_field_class *src_fc, goto end; } + is_valid = true; + end: bt_object_put_ref(src_field_path); bt_object_put_ref(tgt_field_path); @@ -494,23 +509,26 @@ end: } static -struct bt_field_path *resolve_field_path(struct bt_field_class *src_fc, +enum bt_resolve_field_xref_status resolve_field_path( + struct bt_field_class *src_fc, struct bt_field_class *tgt_fc, struct bt_resolve_field_xref_context *ctx, - const char *api_func) + const char *api_func, + struct bt_field_path **ret_field_path) { BT_ASSERT_PRE_DEV_FROM_FUNC(api_func, "valid-field-class", field_path_is_valid(src_fc, tgt_fc, ctx), "Invalid target field class: %![req-fc-]+F, %![tgt-fc-]+F", src_fc, tgt_fc); - return find_field_class_in_ctx(tgt_fc, ctx); + return find_field_class_in_ctx(tgt_fc, ctx, ret_field_path); } -int bt_resolve_field_paths(struct bt_field_class *fc, +enum bt_resolve_field_xref_status bt_resolve_field_paths( + struct bt_field_class *fc, struct bt_resolve_field_xref_context *ctx, const char *api_func) { - int ret = 0; + enum bt_resolve_field_xref_status status; BT_ASSERT(fc); @@ -522,10 +540,10 @@ int bt_resolve_field_paths(struct bt_field_class *fc, if (opt_fc->selector_field_xref_kind == FIELD_XREF_KIND_PATH) { BT_ASSERT(opt_fc->selector_field.path.class); BT_ASSERT(!opt_fc->selector_field.path.path); - opt_fc->selector_field.path.path = resolve_field_path( - fc, opt_fc->selector_field.path.class, ctx, __func__); - if (!opt_fc->selector_field.path.path) { - ret = -1; + status = resolve_field_path( + fc, opt_fc->selector_field.path.class, ctx, __func__, + &opt_fc->selector_field.path.path); + if (status != BT_RESOLVE_FIELD_XREF_STATUS_OK) { goto end; } } @@ -535,10 +553,10 @@ int bt_resolve_field_paths(struct bt_field_class *fc, if (dyn_array_fc->length_field.xref_kind == FIELD_XREF_KIND_PATH) { BT_ASSERT(dyn_array_fc->length_field.path.class); BT_ASSERT(!dyn_array_fc->length_field.path.path); - dyn_array_fc->length_field.path.path = resolve_field_path( - fc, dyn_array_fc->length_field.path.class, ctx, __func__); - if (!dyn_array_fc->length_field.path.path) { - ret = -1; + status = resolve_field_path( + fc, dyn_array_fc->length_field.path.class, ctx, __func__, + &dyn_array_fc->length_field.path.path); + if (status != BT_RESOLVE_FIELD_XREF_STATUS_OK) { goto end; } } @@ -550,12 +568,10 @@ int bt_resolve_field_paths(struct bt_field_class *fc, if (var_fc->selector_field_xref_kind == FIELD_XREF_KIND_PATH) { BT_ASSERT(var_fc->selector_field.path.class); BT_ASSERT(!var_fc->selector_field.path.path); - var_fc->selector_field.path.path = - resolve_field_path(fc, - (void *) var_fc->selector_field.path.class, ctx, - __func__); - if (!var_fc->selector_field.path.path) { - ret = -1; + status = resolve_field_path(fc, + (void *) var_fc->selector_field.path.class, ctx, + __func__, &var_fc->selector_field.path.path); + if (status != BT_RESOLVE_FIELD_XREF_STATUS_OK) { goto end; } } @@ -565,7 +581,10 @@ int bt_resolve_field_paths(struct bt_field_class *fc, if (bt_field_class_type_is(fc->type, BT_FIELD_CLASS_TYPE_OPTION)) { struct bt_field_class_option *opt_fc = (void *) fc; - ret = bt_resolve_field_paths(opt_fc->content_fc, ctx, api_func); + status = bt_resolve_field_paths(opt_fc->content_fc, ctx, api_func); + if (status != BT_RESOLVE_FIELD_XREF_STATUS_OK) { + goto end; + } } else if (fc->type == BT_FIELD_CLASS_TYPE_STRUCTURE || bt_field_class_type_is(fc->type, BT_FIELD_CLASS_TYPE_VARIANT)) { @@ -577,9 +596,9 @@ int bt_resolve_field_paths(struct bt_field_class *fc, struct bt_named_field_class *named_fc = container_fc->named_fcs->pdata[i]; - ret = bt_resolve_field_paths(named_fc->fc, ctx, + status = bt_resolve_field_paths(named_fc->fc, ctx, api_func); - if (ret) { + if (status != BT_RESOLVE_FIELD_XREF_STATUS_OK) { goto end; } } @@ -587,10 +606,15 @@ int bt_resolve_field_paths(struct bt_field_class *fc, BT_FIELD_CLASS_TYPE_ARRAY)) { struct bt_field_class_array *array_fc = (void *) fc; - ret = bt_resolve_field_paths(array_fc->element_fc, ctx, + status = bt_resolve_field_paths(array_fc->element_fc, ctx, api_func); + if (status != BT_RESOLVE_FIELD_XREF_STATUS_OK) { + goto end; + } } + status = BT_RESOLVE_FIELD_XREF_STATUS_OK; + end: - return ret; + return status; } diff --git a/src/lib/trace-ir/resolve-field-path.h b/src/lib/trace-ir/resolve-field-path.h index f5867a70..e0ec37dc 100644 --- a/src/lib/trace-ir/resolve-field-path.h +++ b/src/lib/trace-ir/resolve-field-path.h @@ -15,7 +15,7 @@ #include #include "resolve-field-xref.h" -int bt_resolve_field_paths( +enum bt_resolve_field_xref_status bt_resolve_field_paths( struct bt_field_class *field_class, struct bt_resolve_field_xref_context *ctx, const char *api_func); diff --git a/src/lib/trace-ir/resolve-field-xref.h b/src/lib/trace-ir/resolve-field-xref.h index 98f50e15..2a0fd3bf 100644 --- a/src/lib/trace-ir/resolve-field-xref.h +++ b/src/lib/trace-ir/resolve-field-xref.h @@ -9,6 +9,11 @@ #include "lib/func-status.h" +enum bt_resolve_field_xref_status { + BT_RESOLVE_FIELD_XREF_STATUS_OK = BT_FUNC_STATUS_OK, + BT_RESOLVE_FIELD_XREF_STATUS_MEMORY_ERROR = BT_FUNC_STATUS_MEMORY_ERROR +}; + struct bt_resolve_field_xref_context { struct bt_field_class *packet_context; struct bt_field_class *event_common_context; diff --git a/src/lib/trace-ir/stream-class.c b/src/lib/trace-ir/stream-class.c index 7ad12108..47e5e155 100644 --- a/src/lib/trace-ir/stream-class.c +++ b/src/lib/trace-ir/stream-class.c @@ -299,7 +299,8 @@ bt_stream_class_set_packet_context_field_class( struct bt_stream_class *stream_class, struct bt_field_class *field_class) { - int ret; + enum bt_stream_class_set_field_class_status status; + enum bt_resolve_field_xref_status resolve_status; struct bt_resolve_field_xref_context resolve_ctx = { .packet_context = field_class, .event_common_context = NULL, @@ -317,14 +318,9 @@ bt_stream_class_set_packet_context_field_class( BT_ASSERT_PRE_DEV_STREAM_CLASS_HOT(stream_class); BT_ASSERT_PRE_FC_IS_STRUCT("field-class", field_class, "Packet context field class"); - ret = bt_resolve_field_paths(field_class, &resolve_ctx, __func__); - if (ret) { - /* - * This is the only reason for which - * bt_resolve_field_paths() can fail: anything else - * would be because a precondition is not satisfied. - */ - ret = BT_FUNC_STATUS_MEMORY_ERROR; + resolve_status = bt_resolve_field_paths(field_class, &resolve_ctx, __func__); + if (resolve_status != BT_RESOLVE_FIELD_XREF_STATUS_OK) { + status = (int) resolve_status; goto end; } @@ -336,8 +332,10 @@ bt_stream_class_set_packet_context_field_class( BT_LIB_LOGD("Set stream class's packet context field class: %!+S", stream_class); + status = BT_STREAM_CLASS_SET_FIELD_CLASS_STATUS_OK; + end: - return ret; + return status; } BT_EXPORT @@ -364,7 +362,8 @@ bt_stream_class_set_event_common_context_field_class( struct bt_stream_class *stream_class, struct bt_field_class *field_class) { - int ret; + enum bt_stream_class_set_field_class_status status; + enum bt_resolve_field_xref_status resolve_status; struct bt_resolve_field_xref_context resolve_ctx = { .packet_context = NULL, .event_common_context = field_class, @@ -379,14 +378,9 @@ bt_stream_class_set_event_common_context_field_class( BT_ASSERT_PRE_FC_IS_STRUCT("field-class", field_class, "Event common context field class"); resolve_ctx.packet_context = stream_class->packet_context_fc; - ret = bt_resolve_field_paths(field_class, &resolve_ctx, __func__); - if (ret) { - /* - * This is the only reason for which - * bt_resolve_field_paths() can fail: anything else - * would be because a precondition is not satisfied. - */ - ret = BT_FUNC_STATUS_MEMORY_ERROR; + resolve_status = bt_resolve_field_paths(field_class, &resolve_ctx, __func__); + if (resolve_status != BT_RESOLVE_FIELD_XREF_STATUS_OK) { + status = (int) resolve_status; goto end; } @@ -398,8 +392,10 @@ bt_stream_class_set_event_common_context_field_class( BT_LIB_LOGD("Set stream class's event common context field class: %!+S", stream_class); + status = BT_STREAM_CLASS_SET_FIELD_CLASS_STATUS_OK; + end: - return ret; + return status; } void _bt_stream_class_freeze(const struct bt_stream_class *stream_class) -- 2.34.1