WIP: Add Position() to Firmware interface

[karlodwyer]

Original Issue here

@xaionaro asked on the opensource firmware slack for suggestions around implementing this idea. The approach suggested was to publish a PR and work it out form there, so he and I talked a bit about this.

Some things to note:

• We understand it is not good to export a visitor PositionUpdater from uefi, but we are open for discussion on how to fix it.
• This change and Assemble probably need more testing to give us more assurance that we aren't regressing anything.

[JulienVdG]

Hello, first I've not looked into the code yet, but regarding your point about PositionUpdater above you can place it in the visitor package along with other visitors. There is already coupling between the two package anyway (assemble or table cannot work without knowing about each uefi type). A visitor is not required to provide cli (again assemble is a good example, there is no RegisterCLI in assemble.go) hope this helps.

[xaionaro]

Hello, first I've not looked into the code yet, but regarding your point about PositionUpdater above you can place it in the visitor package along with other visitors.

The problem is: We want to get objects with updated offsets from uefi.Parse. So it is required to call (*PositionUpdater{}).Run() inside uefi.Parse. And if we put this visitor to visitors we get a circular dependency: uefi -> visitor -> uefi.

This circular dependency could be broken if we will move types (like BIOSPadding) to a separate package (like ./pkg/uefi/types). In this case the new visitor could be moved into ./internal/visitors and will be used from both packages: ./pkg/uefi and ./pkg/visitors. But since it requires changes in the skeleton of the project it would be nice to understand if this approach is approved :)

Another obvious approach is just to duplicate the visitor between uefi and visitors (and make it private in uefi) =\

[codecov-io]

Codecov Report

Merging #317 into master will increase coverage by 2.26%. The diff coverage is 36.08%.

@@            Coverage Diff             @@
##           master     #317      +/-   ##
==========================================
+ Coverage   39.21%   41.47%   +2.26%     
==========================================
  Files          47       48       +1     
  Lines        3501     3597      +96     
==========================================
+ Hits         1373     1492     +119     
+ Misses       1912     1873      -39     
- Partials      216      232      +16     

Impacted Files	Coverage Δ
pkg/uefi/biosregion.go	41.53% <0.00%> (+34.98%)	:arrow_up:
pkg/uefi/file.go	24.83% <0.00%> (+7.61%)	:arrow_up:
pkg/uefi/flash.go	27.06% <0.00%> (-0.84%)	:arrow_down:
pkg/uefi/meregion.go	9.89% <0.00%> (-0.46%)	:arrow_down:
pkg/uefi/nvram.go	59.29% <0.00%> (-0.85%)	:arrow_down:
pkg/uefi/rawregion.go	19.04% <0.00%> (-2.01%)	:arrow_down:
pkg/uefi/section.go	41.37% <0.00%> (+14.05%)	:arrow_up:
pkg/uefi/update_positions.go	40.98% <40.98%> (ø)
pkg/uefi/uefi.go	62.13% <50.00%> (+1.08%)	:arrow_up:
pkg/uefi/firmwarevolume.go	61.16% <100.00%> (+23.54%)	:arrow_up:
... and 7 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d73b5b5...f55ff01. Read the comment docs.