Change in coreboot[master]: cbfstool: Don't add compression attribute for uncompressed files

[Julius Werner (Code Review)]

Hello Aaron Durbin,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/46835

to review the following change.

Change subject: cbfstool: Don't add compression attribute for uncompressed files
......................................................................

cbfstool: Don't add compression attribute for uncompressed files

Our current cbfstool has always added a compression attribute to the
CBFS file header for all files that used the cbfstool_convert_raw()
function (basically anything other than a stage or payload), even if the
compression type was NONE. This was likely some sort of oversight, since
coreboot CBFS reading code has always accepted the absence of a
compression attribute to mean "no compression". This patch fixes the
behavior to avoid adding the attribute in these cases.

Change-Id: Ic4a41152db9df66376fa26096d6f3a53baea51de
---
M util/cbfstool/cbfstool.c
1 file changed, 5 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/46835/1

diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index 454aff9..6b9f664 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -766,6 +766,9 @@
    		return -1;
    	memcpy(compressed, buffer->data + 8, compressed_size);
    } else {
+		if (param.compression == CBFS_COMPRESS_NONE)
+			goto out;
+
    	compress = compression_function(param.compression);
    	if (!compress)
    		return -1;
@@ -777,7 +780,7 @@
    		     compressed, &compressed_size)) {
    		WARN("Compression failed - disabled\n");
    		free(compressed);
-			return 0;
+			goto out;
    	}
    }

@@ -797,6 +800,7 @@
    buffer->data = compressed;
    buffer->size = compressed_size;

+out:
    header->len = htonl(buffer->size);
    return 0;
 }

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46835 )

Change subject: cbfstool: Don't add compression attribute for uncompressed files
......................................................................

Patch Set 1: Code-Review+2

[Julius Werner (Code Review)]

Hello build bot (Jenkins), Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/46835

to look at the new patch set (#6).

Change subject: cbfstool: Don't add compression attribute for uncompressed files
......................................................................

cbfstool: Don't add compression attribute for uncompressed files

Our current cbfstool has always added a compression attribute to the
CBFS file header for all files that used the cbfstool_convert_raw()
function (basically anything other than a stage or payload), even if the
compression type was NONE. This was likely some sort of oversight, since
coreboot CBFS reading code has always accepted the absence of a
compression attribute to mean "no compression". This patch fixes the
behavior to avoid adding the attribute in these cases.

Change-Id: Ic4a41152db9df66376fa26096d6f3a53baea51de
Signed-off-by: Julius Werner 
---
M util/cbfstool/cbfstool.c
1 file changed, 5 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/46835/6

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46835 )

Change subject: cbfstool: Don't add compression attribute for uncompressed files
......................................................................

cbfstool: Don't add compression attribute for uncompressed files

Our current cbfstool has always added a compression attribute to the
CBFS file header for all files that used the cbfstool_convert_raw()
function (basically anything other than a stage or payload), even if the
compression type was NONE. This was likely some sort of oversight, since
coreboot CBFS reading code has always accepted the absence of a
compression attribute to mean "no compression". This patch fixes the
behavior to avoid adding the attribute in these cases.

Change-Id: Ic4a41152db9df66376fa26096d6f3a53baea51de
Signed-off-by: Julius Werner 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46835
Tested-by: build bot (Jenkins) 
Reviewed-by: Aaron Durbin 
---
M util/cbfstool/cbfstool.c
1 file changed, 5 insertions(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Aaron Durbin: Looks good to me, approved

diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index d2df1cc..c7a6079 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -595,6 +595,9 @@
    		return -1;
    	memcpy(compressed, buffer->data + 8, compressed_size);
    } else {
+		if (param.compression == CBFS_COMPRESS_NONE)
+			goto out;
+
    	compress = compression_function(param.compression);
    	if (!compress)
    		return -1;
@@ -606,7 +609,7 @@
    		     compressed, &compressed_size)) {
    		WARN("Compression failed - disabled\n");
    		free(compressed);
-			return 0;
+			goto out;
    	}
    }

@@ -626,6 +629,7 @@
    buffer->data = compressed;
    buffer->size = compressed_size;

+out:
    header->len = htonl(buffer->size);
    return 0;
 }