From df0139b8dec30ef11734f92977a5e02f7caeb5bb Mon Sep 17 00:00:00 2001 From: Philippe Proulx Date: Tue, 3 Oct 2017 20:10:17 -0400 Subject: [PATCH] Fix: ctf: notif-iter: do not call request_medium_bytes() when not needed MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Do not systematically call request_medium_bytes(), that is, at the beginning of read_dscope_begin_state(), because the scope field which is about to be decoded might be empty, an already aligned structure field or the equivalent (through a variant field for example, of which the tag is outside this scope). To do this we also need to make bt_btr_start() accept a size of 0. In this case, if the field to decode is an empty structure which is already aligned, then bt_btr_start() does not need to consume any bit and returns BT_BTR_STATUS_OK with 0 consumed bits. In this case, the next notif-iter state is not a continuing state, thus request_medium_bytes() is not called yet. Checking for a bad state when request_medium_bytes() receives BT_NOTIF_ITER_MEDIUM_STATUS_EOF from the medium is easier. Good states are: * If the current packet size is set: ** Current packet offset is current packet size. * Otherwise: ** Current packet offset is 0. ** Last event header offset is set and current offset is equal to it. All other states are invalid. Signed-off-by: Philippe Proulx Signed-off-by: Jérémie Galarneau --- plugins/ctf/common/btr/btr.c | 7 +- plugins/ctf/common/notif-iter/notif-iter.c | 129 ++++----------------- 2 files changed, 24 insertions(+), 112 deletions(-) diff --git a/plugins/ctf/common/btr/btr.c b/plugins/ctf/common/btr/btr.c index e393f301..b6ccc5d4 100644 --- a/plugins/ctf/common/btr/btr.c +++ b/plugins/ctf/common/btr/btr.c @@ -482,6 +482,7 @@ void stitch_append_from_buf(struct bt_btr *btr, size_t sz) buf_byte_at = BITS_TO_BYTES_FLOOR(buf_at_from_addr(btr)); nb_bytes = BITS_TO_BYTES_CEIL(sz); assert(nb_bytes > 0); + assert(btr->buf.addr); memcpy(&btr->stitch.buf[stitch_byte_at], &btr->buf.addr[buf_byte_at], nb_bytes); btr->stitch.at += sz; @@ -903,6 +904,7 @@ enum bt_btr_status read_basic_type_and_call_begin(struct bt_btr *btr, if (field_size <= available) { /* We have all the bits; decode and set now */ + assert(btr->buf.addr); status = read_basic_and_call_cb(btr, btr->buf.addr, buf_at_from_addr(btr)); if (status != BT_BTR_STATUS_OK) { @@ -1004,6 +1006,7 @@ enum bt_btr_status read_basic_string_type_and_call( assert(buf_at_from_addr(btr) % 8 == 0); available_bytes = BITS_TO_BYTES_FLOOR(available_bits(btr)); buf_at_bytes = BITS_TO_BYTES_FLOOR(buf_at_from_addr(btr)); + assert(btr->buf.addr); first_chr = &btr->buf.addr[buf_at_bytes]; result = memchr(first_chr, '\0', available_bytes); @@ -1463,9 +1466,7 @@ size_t bt_btr_start(struct bt_btr *btr, enum bt_btr_status *status) { assert(btr); - assert(buf); - assert(sz > 0); - assert(BYTES_TO_BITS(sz) > offset); + assert(BYTES_TO_BITS(sz) >= offset); reset(btr); btr->buf.addr = buf; btr->buf.offset = offset; diff --git a/plugins/ctf/common/notif-iter/notif-iter.c b/plugins/ctf/common/notif-iter/notif-iter.c index d7164952..527147e5 100644 --- a/plugins/ctf/common/notif-iter/notif-iter.c +++ b/plugins/ctf/common/notif-iter/notif-iter.c @@ -489,118 +489,42 @@ enum bt_notif_iter_status request_medium_bytes( BT_LOGV_MEM(buffer_addr, buffer_sz, "Returned bytes at %p:", buffer_addr); } else if (m_status == BT_NOTIF_ITER_MEDIUM_STATUS_EOF) { - struct bt_field_type *ph_ft = - bt_trace_get_packet_header_type(notit->meta.trace); - struct bt_field_type *eh_ft = NULL; - struct bt_field_type *sec_ft = NULL; - struct bt_field_type *ec_ft = NULL; - /* * User returned end of stream: validate that we're not * in the middle of a packet header, packet context, or * event. */ - if (notit->state == STATE_DSCOPE_TRACE_PACKET_HEADER_BEGIN) { - /* Beginning of packet: always valid */ - goto good_state; - } - - if (!notit->meta.stream_class) { - goto bad_state; - } - - if (notit->state == STATE_DSCOPE_STREAM_PACKET_CONTEXT_BEGIN) { - /* - * Beginning of packet context context is only - * valid if there's no packet header. - */ - if (!ph_ft) { - goto good_state; - } - } - - eh_ft = bt_stream_class_get_event_header_type( - notit->meta.stream_class); - sec_ft = bt_stream_class_get_event_context_type( - notit->meta.stream_class); - - if (notit->state == STATE_DSCOPE_STREAM_EVENT_HEADER_BEGIN) { - /* - * Beginning of event's header is only valid if - * the packet is not supposed to have a specific - * size (whole packet sequence has in fact only - * one packet). - */ - if (notit->cur_packet_size == -1) { - goto good_state; + if (notit->cur_packet_size >= 0) { + if (packet_at(notit) == notit->cur_packet_size) { + goto end; } - } - - if (notit->state == STATE_DSCOPE_STREAM_EVENT_CONTEXT_BEGIN) { - /* - * Beginning of event's stream event context is - * only valid if the packet is not supposed to - * have a specific size (whole packet sequence - * has in fact only one packet), and there's no - * event header. - */ - if (notit->cur_packet_size == -1 && !eh_ft) { - goto good_state; - } - } - - if (!notit->meta.event_class) { - goto bad_state; - } - - ec_ft = bt_event_class_get_context_type( - notit->meta.event_class); - - if (notit->state == STATE_DSCOPE_EVENT_CONTEXT_BEGIN) { - /* - * Beginning of event's context is only valid if - * the packet is not supposed to have a specific - * size (whole packet sequence has in fact only - * one packet), and there's no event header and - * no stream event context. - */ - if (notit->cur_packet_size == -1 && !eh_ft && !sec_ft) { - goto good_state; + } else { + if (packet_at(notit) == 0) { + goto end; } - } - if (notit->state == STATE_DSCOPE_EVENT_PAYLOAD_BEGIN) { - /* - * Beginning of event's context is only valid if - * the packet is not supposed to have a specific - * size (whole packet sequence has in fact only - * one packet), and there's no event header, no - * stream event context, and no event context. - */ - if (notit->cur_packet_size == -1 && !eh_ft && !sec_ft && - !ec_ft) { - goto good_state; + if (notit->buf.last_eh_at != SIZE_MAX && + notit->buf.at == notit->buf.last_eh_at) { + goto end; } } -bad_state: /* All other states are invalid */ BT_LOGW("User function returned %s, but notification iterator is in an unexpected state: " - "state=%s", + "state=%s, cur-packet-size=%" PRId64 ", cur=%zu, " + "packet-cur=%zu, last-eh-at=%zu", bt_notif_iter_medium_status_string(m_status), - state_string(notit->state)); + state_string(notit->state), + notit->cur_packet_size, + notit->buf.at, packet_at(notit), + notit->buf.last_eh_at); m_status = BT_NOTIF_ITER_MEDIUM_STATUS_ERROR; - -good_state: - bt_put(ph_ft); - bt_put(eh_ft); - bt_put(sec_ft); - bt_put(ec_ft); } else if (m_status < 0) { BT_LOGW("User function failed: status=%s", bt_notif_iter_medium_status_string(m_status)); } +end: return notif_iter_status_from_m_status(m_status); } @@ -632,21 +556,6 @@ enum bt_notif_iter_status read_dscope_begin_state( enum bt_btr_status btr_status; size_t consumed_bits; - status = buf_ensure_available_bits(notit); - if (status != BT_NOTIF_ITER_STATUS_OK) { - if (status < 0) { - BT_LOGW("Cannot ensure that buffer has at least one byte: " - "notif-addr=%p, status=%s", - notit, bt_notif_iter_status_string(status)); - } else { - BT_LOGV("Cannot ensure that buffer has at least one byte: " - "notif-addr=%p, status=%s", - notit, bt_notif_iter_status_string(status)); - } - - goto end; - } - bt_put(*dscope_field); notit->cur_dscope_field = dscope_field; BT_LOGV("Starting BTR: notit-addr=%p, btr-addr=%p, ft-addr=%p", @@ -689,6 +598,9 @@ enum bt_notif_iter_status read_dscope_continue_state( enum bt_btr_status btr_status; size_t consumed_bits; + BT_LOGV("Continuing BTR: notit-addr=%p, btr-addr=%p", + notit, notit->btr); + status = buf_ensure_available_bits(notit); if (status != BT_NOTIF_ITER_STATUS_OK) { if (status < 0) { @@ -704,8 +616,7 @@ enum bt_notif_iter_status read_dscope_continue_state( goto end; } - BT_LOGV("Continuing BTR: notit-addr=%p, btr-addr=%p", - notit, notit->btr); + consumed_bits = bt_btr_continue(notit->btr, notit->buf.addr, notit->buf.sz, &btr_status); BT_LOGV("BTR consumed bits: size=%zu", consumed_bits); -- 2.34.1