GCC reports the following warning:
libtool: compile: g++ -std=gnu++11 -DHAVE_CONFIG_H -I../../../include -I../../../include -I../../../src -I../../../src -include config.h -I/home/mjeanson/opt/include -I/home/mjeanson/opt/include -DINSTALL_BIN_PATH=\"/home/mjeanson/opt/lib/lttng/libexec\" -DINSTALL_LIB_PATH=\"/home/mjeanson/opt/lib\" -fvisibility=hidden -fvisibility-inlines-hidden -fno-strict-aliasing -Wall -Wextra -Wmissing-declarations -Wnull-dereference -Wundef -Wredundant-decls -Wshadow -Wsuggest-attribute=format -Wwrite-strings -Wformat=2 -Wstrict-aliasing -Wmissing-noreturn -Wduplicated-cond -Wduplicated-branches -Wlogical-op -Winit-self -Wno-incomplete-setjmp-declaration -Wno-gnu-folding-constant -Wno-sign-compare -Werror -pthread -g -O2 -MT notification-thread-events.lo -MD -MP -MF .deps/notification-thread-events.Tpo -c notification-thread-events.cpp -fPIC -DPIC -o .libs/notification-thread-events.o
In file included from field.hpp:14,
from ust-field-convert.hpp:11,
from ust-app.hpp:14,
from event-notifier-error-accounting.hpp:11,
from notification-thread-events.cpp:12:
../../../src/vendor/optional.hpp: In function ‘int handle_notification_thread_command(notification_thread_handle*, notification_thread_state*)’:
../../../src/vendor/optional.hpp:877:17: error: ‘cmd’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
877 | return &data;
| ^~~~
notification-thread-events.cpp:3181:45: note: ‘cmd’ was declared here
3181 | struct notification_thread_command *cmd;
|
When failing to pop a command, cmd can indeed be used uninitialized when
jumping to the error label.
Reported-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I5e63451ed4936638e9fbe05146f16c86ea2e42e2
#include <common/futex.hpp>
#include <common/hashtable/utils.hpp>
#include <common/macros.hpp>
#include <common/futex.hpp>
#include <common/hashtable/utils.hpp>
#include <common/macros.hpp>
+#include <common/pthread-lock.hpp>
#include <common/sessiond-comm/sessiond-comm.hpp>
#include <common/unix.hpp>
#include <common/urcu.hpp>
#include <common/sessiond-comm/sessiond-comm.hpp>
#include <common/unix.hpp>
#include <common/urcu.hpp>
-static int pop_cmd_queue(struct notification_thread_handle *handle,
- struct notification_thread_command **cmd)
+static notification_thread_command *pop_cmd_queue(notification_thread_handle *handle)
- int ret;
- uint64_t counter;
+ lttng::pthread::lock_guard queue_lock(handle->cmd_queue.lock);
- pthread_mutex_lock(&handle->cmd_queue.lock);
- ret = lttng_read(handle->cmd_queue.event_fd, &counter, sizeof(counter));
- if (ret != sizeof(counter)) {
- ret = -1;
- goto error_unlock;
+ uint64_t counter;
+ const auto read_ret = lttng_read(handle->cmd_queue.event_fd, &counter, sizeof(counter));
+ if (read_ret != sizeof(counter)) {
+ if (read_ret < 0) {
+ LTTNG_THROW_POSIX("Failed to read counter value from event_fd", errno);
+ } else {
+ LTTNG_THROW_ERROR(lttng::format(
+ "Failed to read counter value from event_fd because of a truncated read: ret={}, expected read size={}",
+ read_ret,
+ sizeof(counter)));
+ }
- *cmd = cds_list_first_entry(
+ auto command = cds_list_first_entry(
&handle->cmd_queue.list, struct notification_thread_command, cmd_list_node);
&handle->cmd_queue.list, struct notification_thread_command, cmd_list_node);
- cds_list_del(&((*cmd)->cmd_list_node));
- ret = 0;
-
-error_unlock:
- pthread_mutex_unlock(&handle->cmd_queue.lock);
- return ret;
+ cds_list_del(&((command)->cmd_list_node));
+ return command;
}
/* Returns 0 on success, 1 on exit requested, negative value on error. */
}
/* Returns 0 on success, 1 on exit requested, negative value on error. */
struct notification_thread_state *state)
{
int ret;
struct notification_thread_state *state)
{
int ret;
- struct notification_thread_command *cmd;
+ struct notification_thread_command *cmd = nullptr;
- ret = pop_cmd_queue(handle, &cmd);
- if (ret) {
+ try {
+ cmd = pop_cmd_queue(handle);
+ } catch (const std::exception& ex) {
+ ERR("Failed to get next notification thread command: %s", ex.what());
- if (cmd->is_async) {
- delete cmd;
- cmd = nullptr;
- } else {
- cmd->command_completed_waker->wake();
+ if (cmd) {
+ if (cmd->is_async) {
+ delete cmd;
+ cmd = nullptr;
+ } else {
+ cmd->command_completed_waker->wake();
+ }