[coreboot-gerrit] [M] Change in coreboot[main]: util/cbfstool/cbfs-payload-linux: Add error handling

[Maximilian Brune (Code Review)]

Maximilian Brune has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/83617?usp=email )

Change subject: util/cbfstool/cbfs-payload-linux: Add error handling
......................................................................

util/cbfstool/cbfs-payload-linux: Add error handling

This adds some error handling to the code that adds the input segments
(e.g. kernel, cmdline, initrd...) to the output file.

Currently the compress function can fail and coreboot will still
build "successfully" leaving whoever build coreboot puzzled.

Change-Id: Ie36ad469c73cb3ff9360acc9bbe66c245e8b4a1e
Signed-off-by: Maximilian Brune 
---
M util/cbfstool/cbfs-payload-linux.c
1 file changed, 66 insertions(+), 21 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/83617/1

diff --git a/util/cbfstool/cbfs-payload-linux.c b/util/cbfstool/cbfs-payload-linux.c
index 6b4ed2b..94f6161 100644
--- a/util/cbfstool/cbfs-payload-linux.c
+++ b/util/cbfstool/cbfs-payload-linux.c
@@ -138,17 +138,19 @@
    return 0;
 }

-static void bzp_output_segment(struct bzpayload *bzp, struct buffer *b,
-                               uint32_t type, uint64_t load_addr,
-                               comp_func_ptr compress_func)
+static int bzp_output_segment(struct bzpayload *bzp, struct buffer *b,
+                              uint32_t type, uint64_t load_addr,
+                              comp_func_ptr compress_func)
 {
    struct buffer out;
    struct cbfs_payload_segment *seg;
    int len = 0;

/* Don't process empty buffers. */
-	if (b != NULL && buffer_size(b) == 0)
-		return;
+	if (b != NULL && buffer_size(b) == 0) {
+		ERROR("%s(): Input buffer is empty\n", __func__);
+		return -1;
+	}

seg = bzp->out_seg;
    seg->type = type;
@@ -156,20 +158,32 @@
    bzp->out_seg++;

/* No buffer associated with segment. */
-	if (b == NULL)
-		return;
+	if (b == NULL) {
+		if (type == PAYLOAD_SEGMENT_ENTRY)
+			return 0; // entry segments don't have a buffer attached
+		else
+			return -2;
+	}

/* Use a temp buffer for easier management. */
    buffer_splice(&out, &bzp->output, bzp->offset, buffer_size(b));

seg->mem_len = buffer_size(b);
    seg->offset = bzp->offset;
-	compress_func(buffer_get(b), buffer_size(b), buffer_get(&out), &len);
+
+	int ret = compress_func(buffer_get(b), buffer_size(b), buffer_get(&out), &len);
+	if (ret) {
+		ERROR("%s(): Compression failed\n", __func__);
+		return ret;
+	}
+
    seg->compression = bzp->algo;
    seg->len = len;

/* Update output offset. */
    bzp->offset += len;
+
+	return 0; // success
 }

/* TODO:
@@ -292,30 +306,61 @@
    if (bzp_init_output(&bzp, input->name) != 0)
    	return -1;

+	int ret;
+
    /* parameter block */
-	bzp_output_segment(&bzp, &bzp.parameters,
-	                   PAYLOAD_SEGMENT_DATA, LINUX_PARAM_LOC, bzp.compress);
+	ret = bzp_output_segment(&bzp, &bzp.parameters,
+	                         PAYLOAD_SEGMENT_DATA, LINUX_PARAM_LOC, bzp.compress);
+	if (ret) {
+		ERROR("%s(): Failed to write Linux zero page into segment\n", __func__);
+		return ret;
+	}

/* code block */
    // There is no point in compressing the bzImage (it is already compressed)
-	bzp_output_segment(&bzp, &bzp.kernel,
-	                   PAYLOAD_SEGMENT_CODE, kernel_base,
-	                   compression_function(CBFS_COMPRESS_NONE));
+	ret = bzp_output_segment(&bzp, &bzp.kernel,
+	                         PAYLOAD_SEGMENT_CODE, kernel_base,
+	                         compression_function(CBFS_COMPRESS_NONE));
+	if (ret) {
+		ERROR("%s(): Failed to write Linux kernel into segment\n", __func__);
+		return ret;
+	}

/* trampoline */
-	bzp_output_segment(&bzp, &bzp.trampoline,
-	                   PAYLOAD_SEGMENT_CODE, TRAMPOLINE_ENTRY_LOC, bzp.compress);
+	ret = bzp_output_segment(&bzp, &bzp.trampoline,
+	                         PAYLOAD_SEGMENT_CODE, TRAMPOLINE_ENTRY_LOC, bzp.compress);
+	if (ret) {
+		ERROR("%s(): Failed to write Linux trampoline into segment\n", __func__);
+		return ret;
+	}

/* cmdline */
-	bzp_output_segment(&bzp, &bzp.cmdline,
-		           PAYLOAD_SEGMENT_DATA, COMMAND_LINE_LOC, bzp.compress);
+	if (buffer_size(&bzp.cmdline) != 0) {
+		ret = bzp_output_segment(&bzp, &bzp.cmdline,
+		                         PAYLOAD_SEGMENT_DATA, COMMAND_LINE_LOC, bzp.compress);
+		if (ret) {
+			ERROR("%s(): Failed to write Linux cmdline into segment\n", __func__);
+			return ret;
+		}
+	}

-	/* initrd */
-	bzp_output_segment(&bzp, &bzp.initrd,
-		           PAYLOAD_SEGMENT_DATA, initrd_base, bzp.compress);
+	if (buffer_size(&bzp.initrd) != 0) {
+		/* initrd */
+		ret = bzp_output_segment(&bzp, &bzp.initrd,
+		                         PAYLOAD_SEGMENT_DATA, initrd_base, bzp.compress);
+		if (ret) {
+			ERROR("%s(): Failed to write Linux initrd into segment\n", __func__);
+			return ret;
+		}
+	}

/* Terminating entry segment. */
-	bzp_output_segment(&bzp, NULL, PAYLOAD_SEGMENT_ENTRY, TRAMPOLINE_ENTRY_LOC, bzp.compress);
+	ret = bzp_output_segment(&bzp, NULL, PAYLOAD_SEGMENT_ENTRY, TRAMPOLINE_ENTRY_LOC,
+	                         bzp.compress);
+	if (ret) {
+		ERROR("%s(): Failed to write entry segment\n", __func__);
+		return ret;
+	}

/* Set size of buffer taking into account potential compression. */
    buffer_set_size(&bzp.output, bzp.offset);