From 03550b580b6963b009582a01dc9cd86f29494e9d Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Fri, 2 Mar 2012 16:16:02 -0500 Subject: [PATCH] Fix: kernel session closes fd 0 after create This fix the make check test for kernel session handling. We ensure that all FDs that are unset are set to -1. Acked-by: David Goulet Signed-off-by: Mathieu Desnoyers --- src/bin/lttng-sessiond/main.c | 27 +++++++++------ src/bin/lttng-sessiond/trace-kernel.c | 48 ++++++++++++++++----------- tests/test_kernel_data_trace.c | 14 ++++---- 3 files changed, 52 insertions(+), 37 deletions(-) diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c index 9e4bab468..ff29eb035 100644 --- a/src/bin/lttng-sessiond/main.c +++ b/src/bin/lttng-sessiond/main.c @@ -100,16 +100,22 @@ static struct consumer_data kconsumer_data = { .type = LTTNG_CONSUMER_KERNEL, .err_unix_sock_path = DEFAULT_KCONSUMERD_ERR_SOCK_PATH, .cmd_unix_sock_path = DEFAULT_KCONSUMERD_CMD_SOCK_PATH, + .err_sock = -1, + .cmd_sock = -1, }; static struct consumer_data ustconsumer64_data = { .type = LTTNG_CONSUMER64_UST, .err_unix_sock_path = DEFAULT_USTCONSUMERD64_ERR_SOCK_PATH, .cmd_unix_sock_path = DEFAULT_USTCONSUMERD64_CMD_SOCK_PATH, + .err_sock = -1, + .cmd_sock = -1, }; static struct consumer_data ustconsumer32_data = { .type = LTTNG_CONSUMER32_UST, .err_unix_sock_path = DEFAULT_USTCONSUMERD32_ERR_SOCK_PATH, .cmd_unix_sock_path = DEFAULT_USTCONSUMERD32_CMD_SOCK_PATH, + .err_sock = -1, + .cmd_sock = -1, }; static int dispatch_thread_exit; @@ -324,7 +330,8 @@ static void teardown_kernel_session(struct ltt_session *session) * If a custom kernel consumer was registered, close the socket before * tearing down the complete kernel session structure */ - if (session->kernel_session->consumer_fd != kconsumer_data.cmd_sock) { + if (kconsumer_data.cmd_sock >= 0 && + session->kernel_session->consumer_fd != kconsumer_data.cmd_sock) { lttcomm_close_unix_sock(session->kernel_session->consumer_fd); } @@ -573,12 +580,12 @@ static int send_kconsumer_session_streams(struct consumer_data *consumer_data, DBG("Sending metadata stream fd"); - /* Extra protection. It's NOT supposed to be set to 0 at this point */ - if (session->consumer_fd == 0) { + /* Extra protection. It's NOT supposed to be set to -1 at this point */ + if (session->consumer_fd < 0) { session->consumer_fd = consumer_data->cmd_sock; } - if (session->metadata_stream_fd != 0) { + if (session->metadata_stream_fd >= 0) { /* Send metadata channel fd */ lkm.cmd_type = LTTNG_CONSUMER_ADD_CHANNEL; lkm.u.channel.channel_key = session->metadata->fd; @@ -752,8 +759,8 @@ static int update_kernel_stream(struct consumer_data *consumer_data, int fd) continue; } - /* This is not suppose to be 0 but this is an extra security check */ - if (session->kernel_session->consumer_fd == 0) { + /* This is not suppose to be -1 but this is an extra security check */ + if (session->kernel_session->consumer_fd < 0) { session->kernel_session->consumer_fd = consumer_data->cmd_sock; } @@ -1851,10 +1858,10 @@ static int init_kernel_tracing(struct ltt_kernel_session *session) if (session->consumer_fds_sent == 0) { /* * Assign default kernel consumer socket if no consumer assigned to the - * kernel session. At this point, it's NOT suppose to be 0 but this is + * kernel session. At this point, it's NOT supposed to be -1 but this is * an extra security check. */ - if (session->consumer_fd == 0) { + if (session->consumer_fd < 0) { session->consumer_fd = kconsumer_data.cmd_sock; } @@ -1942,7 +1949,7 @@ static int create_kernel_session(struct ltt_session *session) } /* Set kernel consumer socket fd */ - if (kconsumer_data.cmd_sock) { + if (kconsumer_data.cmd_sock >= 0) { session->kernel_session->consumer_fd = kconsumer_data.cmd_sock; } @@ -2851,7 +2858,7 @@ static int cmd_start_trace(struct ltt_session *session) } /* Open kernel metadata stream */ - if (ksession->metadata_stream_fd == 0) { + if (ksession->metadata_stream_fd < 0) { ret = kernel_open_metadata_stream(ksession); if (ret < 0) { ERR("Kernel create metadata stream failed"); diff --git a/src/bin/lttng-sessiond/trace-kernel.c b/src/bin/lttng-sessiond/trace-kernel.c index 8fb92f12a..7789d6c20 100644 --- a/src/bin/lttng-sessiond/trace-kernel.c +++ b/src/bin/lttng-sessiond/trace-kernel.c @@ -96,12 +96,12 @@ struct ltt_kernel_session *trace_kernel_create_session(char *path) } /* Init data structure */ - lks->fd = 0; - lks->metadata_stream_fd = 0; + lks->fd = -1; + lks->metadata_stream_fd = -1; lks->channel_count = 0; lks->stream_count_global = 0; lks->metadata = NULL; - lks->consumer_fd = 0; + lks->consumer_fd = -1; CDS_INIT_LIST_HEAD(&lks->channel_list.head); /* Set session path */ @@ -140,7 +140,7 @@ struct ltt_kernel_channel *trace_kernel_create_channel(struct lttng_channel *cha } memcpy(lkc->channel, chan, sizeof(struct lttng_channel)); - lkc->fd = 0; + lkc->fd = -1; lkc->stream_count = 0; lkc->event_count = 0; lkc->enabled = 1; @@ -221,7 +221,7 @@ struct ltt_kernel_event *trace_kernel_create_event(struct lttng_event *ev) attr->name[LTTNG_KERNEL_SYM_NAME_LEN - 1] = '\0'; /* Setting up a kernel event */ - lke->fd = 0; + lke->fd = -1; lke->event = attr; lke->enabled = 1; lke->ctx = NULL; @@ -259,7 +259,7 @@ struct ltt_kernel_metadata *trace_kernel_create_metadata(char *path) chan->attr.output = DEFAULT_KERNEL_CHANNEL_OUTPUT; /* Init metadata */ - lkm->fd = 0; + lkm->fd = -1; lkm->conf = chan; /* Set default metadata path */ ret = asprintf(&lkm->pathname, "%s/metadata", path); @@ -291,7 +291,7 @@ struct ltt_kernel_stream *trace_kernel_create_stream(void) } /* Init stream */ - lks->fd = 0; + lks->fd = -1; lks->pathname = NULL; lks->state = 0; @@ -310,9 +310,11 @@ void trace_kernel_destroy_stream(struct ltt_kernel_stream *stream) DBG("[trace] Closing stream fd %d", stream->fd); /* Close kernel fd */ - ret = close(stream->fd); - if (ret) { - PERROR("close"); + if (stream->fd >= 0) { + ret = close(stream->fd); + if (ret) { + PERROR("close"); + } } /* Remove from stream list */ cds_list_del(&stream->list); @@ -358,9 +360,11 @@ void trace_kernel_destroy_channel(struct ltt_kernel_channel *channel) DBG("[trace] Closing channel fd %d", channel->fd); /* Close kernel fd */ - ret = close(channel->fd); - if (ret) { - PERROR("close"); + if (channel->fd >= 0) { + ret = close(channel->fd); + if (ret) { + PERROR("close"); + } } /* For each stream in the channel list */ @@ -391,9 +395,11 @@ void trace_kernel_destroy_metadata(struct ltt_kernel_metadata *metadata) DBG("[trace] Closing metadata fd %d", metadata->fd); /* Close kernel fd */ - ret = close(metadata->fd); - if (ret) { - PERROR("close"); + if (metadata->fd >= 0) { + ret = close(metadata->fd); + if (ret) { + PERROR("close"); + } } free(metadata->conf); @@ -411,12 +417,14 @@ void trace_kernel_destroy_session(struct ltt_kernel_session *session) DBG("[trace] Closing session fd %d", session->fd); /* Close kernel fds */ - ret = close(session->fd); - if (ret) { - PERROR("close"); + if (session->fd >= 0) { + ret = close(session->fd); + if (ret) { + PERROR("close"); + } } - if (session->metadata_stream_fd != 0) { + if (session->metadata_stream_fd >= 0) { DBG("[trace] Closing metadata stream fd %d", session->metadata_stream_fd); ret = close(session->metadata_stream_fd); if (ret) { diff --git a/tests/test_kernel_data_trace.c b/tests/test_kernel_data_trace.c index 0a534699c..41728544f 100644 --- a/tests/test_kernel_data_trace.c +++ b/tests/test_kernel_data_trace.c @@ -72,13 +72,13 @@ static void create_one_kernel_session(void) PRINT_OK(); printf("Validating kernel session: "); - assert(kern->fd == 0); - assert(kern->metadata_stream_fd == 0); + assert(kern->fd == -1); + assert(kern->metadata_stream_fd == -1); assert(kern->consumer_fds_sent == 0); assert(kern->channel_count == 0); assert(kern->stream_count_global == 0); assert(kern->metadata == NULL); - assert(kern->consumer_fd == 0); + assert(kern->consumer_fd == -1); PRINT_OK(); /* Init list in order to avoid sefaults from cds_list_del */ @@ -95,7 +95,7 @@ static void create_kernel_metadata(void) PRINT_OK(); printf("Validating kernel session metadata: "); - assert(kern->metadata->fd == 0); + assert(kern->metadata->fd == -1); assert(strlen(kern->metadata->pathname)); assert(kern->metadata->conf != NULL); assert(kern->metadata->conf->attr.overwrite @@ -128,7 +128,7 @@ static void create_kernel_channel(void) PRINT_OK(); printf("Validating kernel channel: "); - assert(chan->fd == 0); + assert(chan->fd == -1); assert(chan->enabled == 1); assert(strcmp(PATH1, chan->pathname) == 0); assert(chan->stream_count == 0); @@ -157,7 +157,7 @@ static void create_kernel_event(void) PRINT_OK(); printf("Validating kernel event: "); - assert(event->fd == 0); + assert(event->fd == -1); assert(event->enabled == 1); assert(event->ctx == NULL); assert(event->event->instrumentation == LTTNG_KERNEL_TRACEPOINT); @@ -179,7 +179,7 @@ static void create_kernel_stream(void) PRINT_OK(); printf("Validating kernel stream: "); - assert(stream->fd == 0); + assert(stream->fd == -1); assert(stream->pathname == NULL); assert(stream->state == 0); PRINT_OK(); -- 2.34.1