diff --git a/CMakeLists.txt b/CMakeLists.txt index bb2c54f..e60b6cb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -115,7 +115,7 @@ check_c_source_compiles(" #include int main() { unsigned int tmp; - getrandom(tmp, sizeof(tmp), GRND_NONBLOCK); + getrandom(&tmp, sizeof(tmp), GRND_NONBLOCK); return 0; }" CTR_HAVE_GETRANDOM) if(CTR_HAVE_GETRANDOM) diff --git a/src/ctr_attributes.c b/src/ctr_attributes.c index 67caebb..49231d3 100644 --- a/src/ctr_attributes.c +++ b/src/ctr_attributes.c @@ -40,6 +40,10 @@ struct ctrace_attributes *ctr_attributes_create() void ctr_attributes_destroy(struct ctrace_attributes *attr) { + if (attr == NULL) { + return; + } + if (attr->kv) { cfl_kvlist_destroy(attr->kv); } diff --git a/src/ctr_id.c b/src/ctr_id.c index 3be5eb0..5b259a5 100644 --- a/src/ctr_id.c +++ b/src/ctr_id.c @@ -37,7 +37,7 @@ struct ctrace_id *ctr_id_create_random(size_t size) } ret = ctr_random_get(buf, size); - if (ret < 0) { + if (ret != (ssize_t) size) { free(buf); return NULL; } @@ -50,6 +50,10 @@ struct ctrace_id *ctr_id_create_random(size_t size) void ctr_id_destroy(struct ctrace_id *cid) { + if (cid == NULL) { + return; + } + cfl_sds_destroy(cid->buf); free(cid); } @@ -59,7 +63,7 @@ struct ctrace_id *ctr_id_create(void *buf, size_t len) int ret; struct ctrace_id *cid; - if (len <= 0) { + if (buf == NULL || len == 0) { return NULL; } @@ -80,15 +84,22 @@ struct ctrace_id *ctr_id_create(void *buf, size_t len) int ctr_id_set(struct ctrace_id *cid, void *buf, size_t len) { - if (cid->buf) { - cfl_sds_destroy(cid->buf); + cfl_sds_t new_buf; + + if (cid == NULL || buf == NULL || len == 0) { + return -1; } - cid->buf = cfl_sds_create_len(buf, len); - if (!cid->buf) { + new_buf = cfl_sds_create_len(buf, len); + if (new_buf == NULL) { return -1; } + if (cid->buf != NULL) { + cfl_sds_destroy(cid->buf); + } + cid->buf = new_buf; + return 0; } diff --git a/src/ctr_link.c b/src/ctr_link.c index a134df4..fb2549d 100644 --- a/src/ctr_link.c +++ b/src/ctr_link.c @@ -25,6 +25,10 @@ struct ctrace_link *ctr_link_create(struct ctrace_span *span, { struct ctrace_link *link; + if (span == NULL) { + return NULL; + } + link = calloc(1, sizeof(struct ctrace_link)); if (!link) { ctr_errno(); @@ -78,15 +82,22 @@ struct ctrace_link *ctr_link_create_with_cid(struct ctrace_span *span, int ctr_link_set_trace_state(struct ctrace_link *link, char *trace_state) { + cfl_sds_t new_trace_state; + if (!link || !trace_state) { return -1; } - link->trace_state = cfl_sds_create(trace_state); - if (!link->trace_state) { + new_trace_state = cfl_sds_create(trace_state); + if (!new_trace_state) { return -1; } + if (link->trace_state != NULL) { + cfl_sds_destroy(link->trace_state); + } + link->trace_state = new_trace_state; + return 0; } @@ -96,7 +107,12 @@ int ctr_link_set_attributes(struct ctrace_link *link, struct ctrace_attributes * return -1; } - link->attr = attr; + if (link->attr != attr) { + if (link->attr != NULL) { + ctr_attributes_destroy(link->attr); + } + link->attr = attr; + } return 0; } @@ -142,6 +158,3 @@ void ctr_link_destroy(struct ctrace_link *link) - - - diff --git a/src/ctr_random.c b/src/ctr_random.c index 0274c23..05215c4 100644 --- a/src/ctr_random.c +++ b/src/ctr_random.c @@ -29,6 +29,7 @@ #endif #ifdef ITS_A_UNIX_FRIEND +#include #include #include #else @@ -41,22 +42,30 @@ #endif -#include - ssize_t ctr_random_get(void *buf, size_t len) { - int i; - ssize_t ret = 0; - unsigned int s; - char *tmp; + size_t offset; + ssize_t ret; + + if (buf == NULL || len == 0) { + return -1; + } #ifdef CTR_HAVE_GETRANDOM - /* - * On Linux systems getrandom() is preferred, note that our use case it's pretty - * simple (no security stuff). - */ - ret = getrandom(buf, len, GRND_NONBLOCK); - return ret; + offset = 0; + while (offset < len) { + ret = getrandom((char *) buf + offset, len - offset, 0); + if (ret > 0) { + offset += ret; + } + else if (ret < 0 && errno == EINTR) { + continue; + } + else { + return -1; + } + } + return (ssize_t) offset; #endif /* if getrandom() is not available and we are on Linux, macOS or BSD, try out /dev/urandom */ @@ -64,23 +73,29 @@ ssize_t ctr_random_get(void *buf, size_t len) int fd; fd = open("/dev/urandom", O_RDONLY); - if (fd > 0) { - ret = read(fd, buf, len); + if (fd >= 0) { + offset = 0; + while (offset < len) { + ret = read(fd, (char *) buf + offset, len - offset); + if (ret > 0) { + offset += ret; + } + else if (ret < 0 && errno == EINTR) { + continue; + } + else { + close(fd); + return -1; + } + } close(fd); - return ret; - } - - s = time(NULL); - - /* fallback... a very slow way to compose a random buffer */ - tmp = buf; - for (i = 0; i < len; i++) { - /* fixme: we need a good entropy here */ - tmp[i] = rand_r(&s); + return (ssize_t) offset; } + return -1; #else /* Windows ? */ - ret = RtlGenRandom(buf, len); + if (RtlGenRandom(buf, len)) { + return (ssize_t) len; + } + return -1; #endif - - return ret; } diff --git a/src/ctr_resource.c b/src/ctr_resource.c index 2d12161..b2db669 100644 --- a/src/ctr_resource.c +++ b/src/ctr_resource.c @@ -59,15 +59,16 @@ struct ctrace_resource *ctr_resource_create_default() int ctr_resource_set_attributes(struct ctrace_resource *res, struct ctrace_attributes *attr) { - if (!attr) { + if (!res || !attr) { return -1; } - if (res->attr) { - ctr_attributes_destroy(res->attr); + if (res->attr != attr) { + if (res->attr != NULL) { + ctr_attributes_destroy(res->attr); + } + res->attr = attr; } - - res->attr = attr; return 0; } @@ -94,6 +95,10 @@ struct ctrace_resource_span *ctr_resource_span_create(struct ctrace *ctx) { struct ctrace_resource_span *resource_span; + if (ctx == NULL) { + return NULL; + } + resource_span = calloc(1, sizeof(struct ctrace_resource_span)); if (!resource_span) { ctr_errno(); @@ -124,15 +129,22 @@ struct ctrace_resource *ctr_resource_span_get_resource(struct ctrace_resource_sp /* Set the schema_url for a resource_span */ int ctr_resource_span_set_schema_url(struct ctrace_resource_span *resource_span, char *url) { - if (resource_span->schema_url) { - cfl_sds_destroy(resource_span->schema_url); + cfl_sds_t new_url; + + if (resource_span == NULL || url == NULL) { + return -1; } - resource_span->schema_url = cfl_sds_create(url); - if (!resource_span->schema_url) { + new_url = cfl_sds_create(url); + if (new_url == NULL) { return -1; } + if (resource_span->schema_url != NULL) { + cfl_sds_destroy(resource_span->schema_url); + } + resource_span->schema_url = new_url; + return 0; } @@ -142,6 +154,15 @@ void ctr_resource_span_destroy(struct ctrace_resource_span *resource_span) struct cfl_list *head; struct ctrace_scope_span *scope_span; + if (resource_span == NULL) { + return; + } + + /* Fluent Bit historically unlinked resource spans before destroying them. */ + if (resource_span->_head.prev != NULL && resource_span->_head.next != NULL) { + cfl_list_del(&resource_span->_head); + } + /* release resource if set */ if (resource_span->resource) { ctr_resource_destroy(resource_span->resource); diff --git a/src/ctr_scope.c b/src/ctr_scope.c index 2470eae..c41297c 100644 --- a/src/ctr_scope.c +++ b/src/ctr_scope.c @@ -23,6 +23,10 @@ struct ctrace_scope_span *ctr_scope_span_create(struct ctrace_resource_span *res { struct ctrace_scope_span *scope_span; + if (resource_span == NULL) { + return NULL; + } + scope_span = calloc(1, sizeof(struct ctrace_scope_span)); if (!scope_span) { ctr_errno(); @@ -63,15 +67,22 @@ void ctr_scope_span_destroy(struct ctrace_scope_span *scope_span) /* Set the schema_url for a resource_span */ int ctr_scope_span_set_schema_url(struct ctrace_scope_span *scope_span, char *url) { - if (scope_span->schema_url) { - cfl_sds_destroy(scope_span->schema_url); + cfl_sds_t new_url; + + if (scope_span == NULL || url == NULL) { + return -1; } - scope_span->schema_url = cfl_sds_create(url); - if (!scope_span->schema_url) { + new_url = cfl_sds_create(url); + if (new_url == NULL) { return -1; } + if (scope_span->schema_url) { + cfl_sds_destroy(scope_span->schema_url); + } + scope_span->schema_url = new_url; + return 0; } @@ -100,9 +111,18 @@ struct ctrace_instrumentation_scope *ctr_instrumentation_scope_create(char *name if (name) { ins_scope->name = cfl_sds_create(name); + if (ins_scope->name == NULL) { + free(ins_scope); + return NULL; + } } if (version) { ins_scope->version = cfl_sds_create(version); + if (ins_scope->version == NULL) { + cfl_sds_destroy(ins_scope->name); + free(ins_scope); + return NULL; + } } ins_scope->dropped_attr_count = dropped_attr_count; @@ -127,4 +147,3 @@ void ctr_instrumentation_scope_destroy(struct ctrace_instrumentation_scope *ins_ free(ins_scope); } - diff --git a/src/ctr_span.c b/src/ctr_span.c index 0df7fd9..5d309b0 100644 --- a/src/ctr_span.c +++ b/src/ctr_span.c @@ -90,20 +90,21 @@ struct ctrace_span *ctr_span_create(struct ctrace *ctx, struct ctrace_scope_span /* Set the Span ID with a given buffer and length */ int ctr_span_set_trace_id(struct ctrace_span *span, void *buf, size_t len) { + struct ctrace_id *new_id; + if (!buf || len <= 0) { return -1; } - /* If trace_id is already set, free it first */ - if (span->trace_id != NULL) { - ctr_id_destroy(span->trace_id); - span->trace_id = NULL; + new_id = ctr_id_create(buf, len); + if (!new_id) { + return -1; } - span->trace_id = ctr_id_create(buf, len); - if (!span->trace_id) { - return -1; + if (span->trace_id != NULL) { + ctr_id_destroy(span->trace_id); } + span->trace_id = new_id; return 0; } @@ -119,16 +120,20 @@ int ctr_span_set_trace_id_with_cid(struct ctrace_span *span, struct ctrace_id *c /* Set the Span ID with a given buffer and length */ int ctr_span_set_span_id(struct ctrace_span *span, void *buf, size_t len) { + struct ctrace_id *new_id; + if (!buf || len <= 0) { return -1; } + new_id = ctr_id_create(buf, len); + if (!new_id) { + return -1; + } + if (span->span_id != NULL) { ctr_id_destroy(span->span_id); } - span->span_id = ctr_id_create(buf, len); - if (!span->span_id) { - return -1; - } + span->span_id = new_id; return 0; } @@ -144,18 +149,21 @@ int ctr_span_set_span_id_with_cid(struct ctrace_span *span, struct ctrace_id *ci /* Set the Span Parent ID with a given buffer and length */ int ctr_span_set_parent_span_id(struct ctrace_span *span, void *buf, size_t len) { + struct ctrace_id *new_id; + if (!buf || len <= 0) { return -1; } - if (span->parent_span_id) { - ctr_id_destroy(span->parent_span_id); + new_id = ctr_id_create(buf, len); + if (!new_id) { + return -1; } - span->parent_span_id = ctr_id_create(buf, len); - if (!span->parent_span_id) { - return -1; + if (span->parent_span_id) { + ctr_id_destroy(span->parent_span_id); } + span->parent_span_id = new_id; return 0; } @@ -280,35 +288,44 @@ void ctr_span_end_ts(struct ctrace *ctx, struct ctrace_span *span, uint64_t ts) int ctr_span_set_status(struct ctrace_span *span, int code, char *message) { + cfl_sds_t new_message; struct ctrace_span_status *status; - status = &span->status; - if (status->message) { - cfl_sds_destroy(status->message); - } - + new_message = NULL; if (message) { - status->message = cfl_sds_create(message); - if (!status->message) { + new_message = cfl_sds_create(message); + if (!new_message) { return -1; } } + status = &span->status; + if (status->message) { + cfl_sds_destroy(status->message); + } + status->message = new_message; status->code = code; return 0; } int ctr_span_set_trace_state(struct ctrace_span *span, char *state, int len) { - if (span->trace_state) { - cfl_sds_destroy(span->trace_state); + cfl_sds_t new_state; + + if (span == NULL || state == NULL || len < 0) { + return -1; } - span->trace_state = cfl_sds_create_len(state, len); - if (!span->trace_state) { + new_state = cfl_sds_create_len(state, len); + if (!new_state) { return -1; } + if (span->trace_state) { + cfl_sds_destroy(span->trace_state); + } + span->trace_state = new_state; + return 0; } @@ -320,11 +337,21 @@ int ctr_span_set_flags(struct ctrace_span *span, uint32_t flags) void ctr_span_set_schema_url(struct ctrace_span *span, char *url) { + cfl_sds_t new_url; + + if (span == NULL || url == NULL) { + return; + } + + new_url = cfl_sds_create(url); + if (new_url == NULL) { + return; + } + if (span->schema_url) { cfl_sds_destroy(span->schema_url); } - - span->schema_url = cfl_sds_create(url); + span->schema_url = new_url; } void ctr_span_set_dropped_link_count(struct ctrace_span *span, uint32_t count) @@ -355,6 +382,10 @@ void ctr_span_destroy(struct ctrace_span *span) struct ctrace_span_status *status; struct ctrace_link *link; + if (span == NULL) { + return; + } + if (span->name != NULL) { cfl_sds_destroy(span->name); } @@ -429,6 +460,11 @@ struct ctrace_span_event *ctr_span_event_add_ts(struct ctrace_span *span, char * return NULL; } ev->attr = ctr_attributes_create(); + if (ev->attr == NULL) { + cfl_sds_destroy(ev->name); + free(ev); + return NULL; + } ev->dropped_attr_count = 0; /* if no timestamp is given, use the current time */ @@ -513,4 +549,3 @@ void ctr_span_event_delete(struct ctrace_span_event *event) cfl_list_del(&event->_head); free(event); } - diff --git a/tests/span.c b/tests/span.c index 9344d68..42427d2 100644 --- a/tests/span.c +++ b/tests/span.c @@ -86,7 +86,43 @@ void test_span() ctr_destroy(ctx); } +void test_resource_span_direct_destroy() +{ + struct ctrace *ctx; + struct ctrace_resource_span *resource_span; + + ctx = ctr_create(NULL); + TEST_CHECK(ctx != NULL); + + resource_span = ctr_resource_span_create(ctx); + TEST_CHECK(resource_span != NULL); + + ctr_resource_span_destroy(resource_span); + TEST_CHECK(cfl_list_is_empty(&ctx->resource_spans)); + + /* Preserve compatibility with Fluent Bit's historical manual unlink. */ + resource_span = ctr_resource_span_create(ctx); + TEST_CHECK(resource_span != NULL); + cfl_list_del(&resource_span->_head); + ctr_resource_span_destroy(resource_span); + + /* Must not traverse the resource span that was already destroyed. */ + ctr_destroy(ctx); +} + +void test_random_id_length() +{ + struct ctrace_id *id; + + id = ctr_id_create_random(CTR_ID_OTEL_TRACE_SIZE); + TEST_CHECK(id != NULL); + TEST_CHECK(ctr_id_get_len(id) == CTR_ID_OTEL_TRACE_SIZE); + ctr_id_destroy(id); +} + TEST_LIST = { {"span", test_span}, + {"resource_span_direct_destroy", test_resource_span_direct_destroy}, + {"random_id_length", test_random_id_length}, { 0 } };