[lttng-dev] [RFC PATCH babeltrace-1.5] Propagate error from packet_seek in case of truncated packet

Jérémie Galarneau jeremie.galarneau at efficios.com
Tue Feb 20 15:23:51 EST 2018


Merged with a couple of minor style fixes in master, stable-1.5, and stable-1.4.

Thanks!
Jérémie

On Wed, Feb 07, 2018 at 05:52:05PM -0500, Jonathan Rajotte wrote:
> Report the error all the way up allowing users/scripts to perform error
> detection and act on it.
> 
> Print to stderr the truncated packet information for easier
> identification.
> 
> Introduce bt_packet_seek_error enum for specific error handling.
> 
> Use the ERANGE errno for error propagation inside bt_iter_next and
> ctf_read_event.
> 
> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
> ---
>  converter/babeltrace.c               | 12 ++++++++--
>  formats/ctf/ctf.c                    | 44 ++++++++++++++++++++++++++++--------
>  formats/lttng-live/lttng-live-comm.c |  6 ++---
>  include/babeltrace/error.h           |  5 ++++
>  lib/iterator.c                       |  4 ++++
>  5 files changed, 57 insertions(+), 14 deletions(-)
> 
> diff --git a/converter/babeltrace.c b/converter/babeltrace.c
> index f74384e..ef783ed 100644
> --- a/converter/babeltrace.c
> +++ b/converter/babeltrace.c
> @@ -669,6 +669,7 @@ int convert_trace(struct bt_trace_descriptor *td_write,
>  	struct bt_iter_pos *begin_pos = NULL, *end_pos = NULL;
>  	struct bt_ctf_event *ctf_event;
>  	int ret;
> +	int error_holder = 0;
>  
>  	sout = container_of(td_write, struct ctf_text_stream_pos,
>  			trace_descriptor);
> @@ -695,11 +696,18 @@ int convert_trace(struct bt_trace_descriptor *td_write,
>  			goto end;
>  		}
>  		ret = bt_iter_next(bt_ctf_get_iter(iter));
> -		if (ret < 0) {
> +		if (ret == -ERANGE) {
> +			/*
> +			 * Remember that a range (truncated packet)
> +			 * error occurred and continue.
> +			 */
> +			error_holder = 1;
> +			continue;
> +		} else if (ret < 0) {
>  			goto end;
>  		}
>  	}
> -	ret = 0;
> +	ret = error_holder;
>  
>  end:
>  	bt_ctf_iter_destroy(iter);
> diff --git a/formats/ctf/ctf.c b/formats/ctf/ctf.c
> index b74cddd..9acaa2a 100644
> --- a/formats/ctf/ctf.c
> +++ b/formats/ctf/ctf.c
> @@ -476,6 +476,25 @@ void ctf_print_discarded_lost(FILE *fp, struct ctf_stream_definition *stream)
>  		"buffers or with fewer events enabled.\n");
>  	fflush(fp);
>  }
> +static
> +void ctf_print_truncated_packet(FILE *fp, struct ctf_stream_definition *stream,
> +		uint64_t packet_size, uint64_t remaining_file_size)
> +{
> +	fflush(stdout);
> +	fprintf(fp, "[error] Packet size (%" PRIu64 " bits) is larger than remaining file size (%" PRIu64 " bits) ",
> +			packet_size, remaining_file_size);
> +	fprintf(fp, "in trace UUID ");
> +	print_uuid(fp, stream->stream_class->trace->uuid);
> +	if (stream->stream_class->trace->parent.path[0])
> +		fprintf(fp, ", at path: \"%s\"",
> +			stream->stream_class->trace->parent.path);
> +
> +	fprintf(fp, ", within stream id %" PRIu64, stream->stream_id);
> +	if (stream->path[0])
> +		fprintf(fp, ", at relative path: \"%s\"", stream->path);
> +	fprintf(fp, ".\n");
> +	fflush(fp);
> +}
>  
>  static
>  int ctf_read_event(struct bt_stream_pos *ppos, struct ctf_stream_definition *stream)
> @@ -491,8 +510,12 @@ int ctf_read_event(struct bt_stream_pos *ppos, struct ctf_stream_definition *str
>  	if (unlikely(pos->offset == EOF))
>  		return EOF;
>  
> -	if (ctf_pos_get_event(pos))
> +	ret = ctf_pos_get_event(pos);
> +	if (ret == -BT_PACKET_SEEK_ERROR_TRUNCATED_PACKET) {
> +		return -ERANGE;
> +	} else if (ret) {
>  		return EOF;
> +	}
>  
>  	/* save the current position as a restore point */
>  	pos->last_offset = pos->offset;
> @@ -1059,7 +1082,7 @@ void ctf_packet_seek(struct bt_stream_pos *stream_pos, size_t index, int whence)
>  	case SEEK_SET:	/* Fall-through */
>  		break;	/* OK */
>  	default:
> -		ret = -1;
> +		ret = -BT_PACKET_SEEK_ERROR;
>  		goto end;
>  	}
>  
> @@ -1072,7 +1095,7 @@ void ctf_packet_seek(struct bt_stream_pos *stream_pos, size_t index, int whence)
>  		if (ret) {
>  			fprintf(stderr, "[error] Unable to unmap old base: %s.\n",
>  				strerror(errno));
> -			ret = -1;
> +			ret = -BT_PACKET_SEEK_ERROR;
>  			goto end;
>  		}
>  		pos->base_mma = NULL;
> @@ -1093,7 +1116,7 @@ void ctf_packet_seek(struct bt_stream_pos *stream_pos, size_t index, int whence)
>  			pos->cur_index = 0;
>  			break;
>  		default:
> -			ret = -1;
> +			ret = -BT_PACKET_SEEK_ERROR;
>  			goto end;
>  		}
>  		pos->content_size = -1U;	/* Unknown at this point */
> @@ -1127,7 +1150,7 @@ void ctf_packet_seek(struct bt_stream_pos *stream_pos, size_t index, int whence)
>  			pos->cur_index = index;
>  			break;
>  		default:
> -			ret = -1;
> +			ret = -BT_PACKET_SEEK_ERROR;
>  			goto end;
>  		}
>  
> @@ -1176,15 +1199,18 @@ void ctf_packet_seek(struct bt_stream_pos *stream_pos, size_t index, int whence)
>  		if (packet_index->data_offset == -1) {
>  			ret = find_data_offset(pos, file_stream, packet_index);
>  			if (ret < 0) {
> -				ret = -1;
> +				ret = -BT_PACKET_SEEK_ERROR;
>  				goto end;
>  			}
>  		}
>  
>  		if (packet_index->packet_size > ((uint64_t) pos->file_length - packet_index->offset) * CHAR_BIT) {
> -			fprintf(stderr, "[error] Packet size (%" PRIu64 " bits) is larger than remaining file size (%" PRIu64 " bits).\n",
> -				packet_index->packet_size, ((uint64_t) pos->file_length - packet_index->offset) * CHAR_BIT);
> -			ret = -1;
> +			uint64_t remaining_file_size = ((uint64_t) pos->file_length - packet_index->offset) * CHAR_BIT;
> +			ctf_print_truncated_packet(stderr, &file_stream->parent,
> +					packet_index->packet_size,
> +					remaining_file_size);
> +			pos->offset = EOF;
> +			ret = -BT_PACKET_SEEK_ERROR_TRUNCATED_PACKET;
>  			goto end;
>  		}
>  
> diff --git a/formats/lttng-live/lttng-live-comm.c b/formats/lttng-live/lttng-live-comm.c
> index cb871a1..055b1c3 100644
> --- a/formats/lttng-live/lttng-live-comm.c
> +++ b/formats/lttng-live/lttng-live-comm.c
> @@ -1223,7 +1223,7 @@ void ctf_live_packet_seek(struct bt_stream_pos *stream_pos, size_t index,
>  	ret = handle_seek_position(index, whence, viewer_stream, pos,
>  			file_stream);
>  	if (ret != 0) {
> -		ret = -1;
> +		ret = -BT_PACKET_SEEK_ERROR;
>  		goto end;
>  	}
>  
> @@ -1266,7 +1266,7 @@ retry:
>  			if (!lttng_live_should_quit()) {
>  				fprintf(stderr, "[error] get_next_index failed\n");
>  			}
> -			ret = -1;
> +			ret = -BT_PACKET_SEEK_ERROR;
>  			goto end;
>  		}
>  		printf_verbose("Index received : packet_size : %" PRIu64
> @@ -1389,7 +1389,7 @@ retry:
>  		pos->offset = EOF;
>  		if (!lttng_live_should_quit()) {
>  			fprintf(stderr, "[error] get_data_packet failed\n");
> -			ret = -1;
> +			ret = -BT_PACKET_SEEK_ERROR;
>  		} else {
>  			ret = 0;
>  		}
> diff --git a/include/babeltrace/error.h b/include/babeltrace/error.h
> index 2f6a6d7..8b4f323 100644
> --- a/include/babeltrace/error.h
> +++ b/include/babeltrace/error.h
> @@ -30,6 +30,11 @@
>   * SOFTWARE.
>   */
>  
> +enum bt_packet_seek_error {
> +	BT_PACKET_SEEK_ERROR = 1,
> +	BT_PACKET_SEEK_ERROR_TRUNCATED_PACKET = 2,
> +};
> +
>  /*
>   * bt_packet_seek_get_error: get the return code of the last packet_seek use.
>   *
> diff --git a/lib/iterator.c b/lib/iterator.c
> index 30a4542..639a2d2 100644
> --- a/lib/iterator.c
> +++ b/lib/iterator.c
> @@ -864,6 +864,10 @@ int bt_iter_next(struct bt_iter *iter)
>  		 */
>  		ret = 0;
>  		goto reinsert;
> +	} else if (ret == -ERANGE) {
> +		removed = bt_heap_remove(iter->stream_heap);
> +		assert(removed == file_stream);
> +		goto end;
>  	} else if (ret) {
>  		goto end;
>  	}
> -- 
> 2.7.4
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev


More information about the lttng-dev mailing list