<p>Julius Werner <strong>posted comments</strong> on this change.</p><p><a href="https://review.coreboot.org/21608">View Change</a></p><p>Patch set 1:</p><p>(3 comments)</p><ul style="list-style: none; padding-left: 20px;"><li><p><a href="https://review.coreboot.org/#/c/21608/1/util/cbfstool/cbfs_image.c">File util/cbfstool/cbfs_image.c:</a></p><ul style="list-style: none; padding-left: 20px;"><li><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/21608/1/util/cbfstool/cbfs_image.c@507">Patch Set #1, Line 507:</a> <code style="font-family:monospace,monospace">        * empty, we still want to keep it or we remove the entire filesystem.</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I'd say this (removing the entire filesystem) would be the more natural thing to do here. It might make sense in edge cases, e.g. imagine a situation where you have an extra CBFS FMAP section somewhere that may optionally contain something (we actually have that with RW_LEGACY already, although it's not verified). If the build system decides (from config options or whatever) not to add any files, you would still want it truncated so that it doesn't waste bytes on verification.</p></li><li><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/21608/1/util/cbfstool/cbfs_image.c@509">Patch Set #1, Line 509:</a> <code style="font-family:monospace,monospace">       * If that file is empty, remove it and report its header's offset as</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">nit: Put this comment in front of the next if() so it's clear which comment belongs to what code section.</p></li><li><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/21608/1/util/cbfstool/cbfs_image.c@519">Patch Set #1, Line 519:</a> <code style="font-family:monospace,monospace">           /* nothing to truncate */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">In both of the return 1 cases, should size be adjusted to the actual end of the CBFS? It might be different from buffer_size if the CBFS had already been truncated previously.</p></li></ul></li></ul><p>To view, visit <a href="https://review.coreboot.org/21608">change 21608</a>. To unsubscribe, visit <a href="https://review.coreboot.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://review.coreboot.org/21608"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: coreboot </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I0c747090813898539f3428936afa9d8459adee9c </div>
<div style="display:none"> Gerrit-Change-Number: 21608 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Patrick Georgi <pgeorgi@google.com> </div>
<div style="display:none"> Gerrit-Reviewer: Julius Werner <jwerner@chromium.org> </div>
<div style="display:none"> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 20 Sep 2017 22:40:55 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>