[coreboot-gerrit] Change in coreboot[master]: [WIP]cbfstool: Always add compression attribute

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/27115 )

Change subject: [WIP]cbfstool: Always add compression attribute
......................................................................


Patch Set 2: Code-Review-1

I am not sure what you want to do with this, so I'm gonna go with cautious disagreement unless you convince me that this is really needed for some reason.

Stage compression and full content compression are two different things. They use the same algorithms under the hood, but the way they're packaged is different and we'd better not confuse the two. The compression attribute is only used for the full content version, and it's more than informational... it actually tells coreboot how to treat the whole contents of that CBFS file. So if you set that on a stage or payload, that would mean that coreboot should try to decompress that full file content (including the stage or payload header) with the given algorithm, which will obviously not work. (Whether the current CBFS code would actually try to do that or not I'm not sure, but regardless I think that would be the most logical interpretation of the data.)

cbfstool can already show you the stage and payload compression algorithms, you just have to call print with -v so it will show you individual stage/payload components. If your only gripe here is that cbfstool print without -v shows 'none' on those files, I'd be fine with just changing the output to use the algorithm from the stage or the biggest payload section there. (Still, technically, 'none' is correct because the stage or payload header is always uncompressed. Stages and especially payloads are essentially container formats. Parts of the container may be internally compressed, but we never compress the whole thing.)


-- 
To view, visit https://review.coreboot.org/27115
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I03927dcbd486602f54210a344d5cc7af2265c965
Gerrit-Change-Number: 27115
Gerrit-PatchSet: 2
Gerrit-Owner: Patrick Rudolph <siro at das-labor.org>
Gerrit-Reviewer: Julius Werner <jwerner at chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth at google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi at google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-Comment-Date: Mon, 18 Jun 2018 23:43:32 +0000
Gerrit-HasComments: No
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180618/c3e7d957/attachment.html>