Attention is currently required from: Nico Huber, Martin L Roth, Maximilian Brune, Arthur Heymans. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63639 )
Change subject: Add SBOM (Software Bill of Materials) Generation ......................................................................
Patch Set 14:
(4 comments)
File util/goswid/cmd/main.go:
https://review.coreboot.org/c/coreboot/+/63639/comment/db51514d_dae4b73c PS12, Line 54: //if (output_format != uswid) && (len(input_file_paths) > 1) { : // ErrorOut("multiple input files are only supported in conjunction with the .uswid output file extension\n") : //}
since this corresponds to a different project/repo, I will make the changes there and pull them at a […]
Ack
https://review.coreboot.org/c/coreboot/+/63639/comment/84f5ab8b_58e6ca5e PS12, Line 68: if if_len >= 5 && input_file_path[if_len-5:if_len] == ".json" { : err = input_tag.FromJSON(input_file) : } else if if_len >= 4 && input_file_path[if_len-4:if_len] == ".xml" { : err = input_tag.FromXML(input_file) : } else if if_len >= 5 && input_file_path[if_len-5:if_len] == ".cbor" { : err = input_tag.FromCBOR(input_file) : } else if if_len >= 6 && input_file_path[if_len-6:if_len] == ".uswid" { : _, err = uswid_input_tag.FromUSWID(input_file) : isUSWID = true : } else { : fmt.Printf("input file extension not recognized, assuming USWID: %s\n", input_file_path) : _, err = uswid_input_tag.FromUSWID(input_file) : isUSWID = true : }
I guess yours is a bit more explicit. […]
Ack. My approach avoids having the same information in three different places on each line:
if if_len >= 5 && input_file_path[if_len-5:if_len] == ".json" {
Here, the length of the string to compare appears twice in the form of `5` and is also implicit in the string literal.
File util/goswid/pkg/uswid/uswid.go:
https://review.coreboot.org/c/coreboot/+/63639/comment/c740532f_8d4845f3 PS12, Line 14: // can't be const...
to be honest I was kind of confused with this one. […]
Huh, weird.
https://review.coreboot.org/c/coreboot/+/63639/comment/1774aa64_a03343d6 PS12, Line 32: 16
in my opinion this is the most explicit and readable version of doing protocol/encoding stuff. An abstraction would only make it worse (at least in my opinion)
Hmmm, my main complaint is that having magic offsets makes it hard to know what's being accessed (one would need to check some spec/description of the binary data), that's why I'd try to define constants for the numbers (e.g. something like `offset_header_version := 16`).
By abstraction I was thinking of something that can convert the binary data into a structure or an object and viceversa. Does something like this still exist? https://www.jonathan-petitcolas.com/2014/09/25/parsing-binary-files-in-go.ht...
In any case, this is not too critical, as the tool is a separate project.