Fix: reduce scope of kconsumer consumed_pos and produced_pos

The consumed_pos and produced_pos accesses are protected by the
stream mutex, which is fine as-is. However, consumed_pos is
passed to consumer_get_consume_start_pos() and is flagged by
Coverity as a possible use of a "stale" consumed_pos.

From an analyzer's standpoint, this makes sense since
both lttng_kconsumer_get_produced_snapshot() and
lttng_kconsumer_get_consumed_snapshot() could leave their output
parameter uninitialized and return 0 since they both  assume that
ioctl() will set errno if ret != 0.

IOCTL(3P) specifies that errno is only set if ret < 0.

A bug in lttng-modules could cause ioctl() to return a positive
value, leaving the errno variable unset. In such a case,
both functions would return 0, leaving the positions uninitialized.

A follow-up fix enforces this assumption (ret never > 0) as part
of the kernctl API.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/src/common/kernel-consumer/kernel-consumer.c b/src/common/kernel-consumer/kernel-consumer.c
index b1cc03e860a0e9b10d34a5e8f528f3fcb4d90ae5..68dcc02309a452c756838c6947ecdf0e6a76957b 100644 (file)
--- a/src/common/kernel-consumer/kernel-consumer.c
+++ b/src/common/kernel-consumer/kernel-consumer.c
@@ -118,7 +118,6 @@ int lttng_kconsumer_snapshot_channel(uint64_t key, char *path,
                struct lttng_consumer_local_data *ctx)
 {
        int ret;
-       unsigned long consumed_pos, produced_pos;
        struct lttng_consumer_channel *channel;
        struct lttng_consumer_stream *stream;
 
@@ -143,6 +142,7 @@ int lttng_kconsumer_snapshot_channel(uint64_t key, char *path,
        cds_list_for_each_entry(stream, &channel->streams.head, send_node) {
                /* Are we at a position _before_ the first available packet ? */
                bool before_first_packet = true;
+               unsigned long consumed_pos, produced_pos;
 
                health_code_update();