File device paths: don't print "File(" or ")" #232
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ardb poked me and said:
<ardb> pjones: were you aware that EDK2 does not actually use File() for
EFIDP_MEDIA_FILE device path nodes?
<ardb> e.g., parsing the following using the text-to-device-path
protocol
<ardb> HD(1,GPT,400774eb-3297-4c12-906a-f913b4c4de18,0x800,0x100000)/File(\EFI\debian\grubaa64.efi)
<ardb> results in a filename in the EFIDP_MEDIA_FILE device node that
includes the characters that make up File( and )
I checked in the spec, and all it has to say about it is in the examples
of printing a DP section, which says:
+------------------------------+-----------------------------+
| Type: 4 (Media Device Path) | String |
| SubType: 4 (File Path)String | The String is the file path |
| | and is a string. |
+------------------------------+-----------------------------+
I've seen little evidence of anything ever using the output of efivar
with the EFI Text to Device Path protocol, so I don't think it's a
problem to make the output here consistent with the spec's big
inconsistency.
This patch removes the "File(" and ")" text from the output.
Signed-off-by: Peter Jones <pjones@redhat.com>
b39e77a to
4fbe060
Compare
format() takes a device path type to print in the error output. Unfortunately, in several cases format_ipv6_addr_helper() passes in the /string/ "dp_type" instead of the dp_type that's passed in, which makes the error output thoroughly useless. This patch passes through the data instead. Signed-off-by: Peter Jones <pjones@redhat.com>
IPv6 addresses are supposed to append the port if it's nonzero, but we've accidentally checked for >= 0, which is all possible values. This patch fixes the check to be > 0. Signed-off-by: Peter Jones <pjones@redhat.com>
Unfortunately, the ipv6 address device path type has gateway_ip6_addr defined as a uint8_t rather than 16 of them. This patch fixes it to be an array. While this is theoretically ABI breaking, it'll only affect things that take a sizeof() on that field or incorrectly take a sizeof() on the struct, both of which are bad to do in any case, as anything iterating device paths or taking the size of the path node should be using the size field in the header. Signed-off-by: Peter Jones <pjones@redhat.com>
ardb poked me saying: <ardb> pjones: now that you're fixing stuff: <ardb> MAC(00001a1bb328,1)/IPv4(0.0.0.00.0.0.0,0,0) Lo and behold, that is indeed wrong and bad. This patch fixes it, and takes the liberty of making the IPv6 formatting also match the spec, with the caveat that the spec says the last field in IPv6() is a netmask, but all the examples show the prefix length, which is the field we actually have in the struct. Before: Boot0007* UEFI HTTPv4 (MAC:525400CA2EE0) MAC(525400ca2ee0,1)/IPv4(0.0.0.00.0.0.0,0,0)/Uri() Boot0008* UEFI HTTPv6 (MAC:525400CA2EE0) MAC(525400ca2ee0,1)/IPv6([::]:<->[::]:,0,0)/Uri() After: Boot0007* UEFI HTTPv4 (MAC:525400CA2EE0) MAC(525400ca2ee0,1)/IPv4(0.0.0.0,0,DHCP,0.0.0.0,0.0.0.0,0.0.0.0)/Uri() Boot0008* UEFI HTTPv6 (MAC:525400CA2EE0) MAC(525400ca2ee0,1)/IPv6([::],0,Static,[::],[::],64)/Uri() Signed-off-by: Peter Jones <pjones@redhat.com>
4fbe060 to
dd901c1
Compare
frozencemetery
approved these changes
Sep 27, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ardb poked me about a few efivar device path issues: