diff mbox series

[RFC,babeltrace-1.5] Propagate error from packet_seek in case of truncated packet

Message ID 1518043925-13229-1-git-send-email-jonathan.rajotte-julien@efficios.com
State Accepted, archived
Delegated to: Jérémie Galarneau
Headers show
Series [RFC,babeltrace-1.5] Propagate error from packet_seek in case of truncated packet | expand

Commit Message

Jonathan Rajotte Feb. 7, 2018, 10:52 p.m. UTC
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(-)

Comments

Jérémie Galarneau Feb. 20, 2018, 8:23 p.m. UTC | #1
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
diff mbox series

Patch

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;
 	}