Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More misc fixes #185

Merged
merged 13 commits into from
Nov 22, 2021
Merged

More misc fixes #185

merged 13 commits into from
Nov 22, 2021

Conversation

vathpela
Copy link
Contributor

These are mostly minor fixes, plus some more mostly standalone prep-work for #184 .

Copy link
Member

@frozencemetery frozencemetery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll want to re-review the hexdump patch, but am done with the rest.

As usual, fussy style-related changes I can make on merge, including to commit messages. I didn't call them out here, but did want to explain the things I look for in commit messages:

  • commit subjects shouldn't end in a period, and should contain the "what" of the change (not the "how"). Ideally they're also <= 50 chars
  • bodies should focus on the "why" of a change - this will involve some "what" and "how", but shouldn't be the focus. Line wrapping at 72 characters where reasonable

(Full disclosure, I didn't come up with these criteria.)

While I generally dislike having separate "prep work" PRs from "now do the thing I want" PRs, having them simultaneously as you've done with 184+185 is fine (though if you wanted to combine them, that's also fine by me).

src/thread-test.c Outdated Show resolved Hide resolved
tests/Makefile Outdated Show resolved Hide resolved
tests/test.parse.db.var.goal.txt Outdated Show resolved Hide resolved
src/hexdump.h Outdated Show resolved Hide resolved
src/hexdump.h Outdated Show resolved Hide resolved
src/util.h Outdated Show resolved Hide resolved
src/hexdump.h Outdated Show resolved Hide resolved
src/util.h Outdated Show resolved Hide resolved
src/guid.c Show resolved Hide resolved
src/hexdump.h Show resolved Hide resolved
@vathpela vathpela force-pushed the more-misc-fixes branch 2 times, most recently from 1f62005 to a039d5d Compare November 16, 2021 22:02
@vathpela
Copy link
Contributor Author

Note that this updated added 6307b21 and 8767928.

@vathpela vathpela force-pushed the more-misc-fixes branch 2 times, most recently from 6da695c to 06c2387 Compare November 16, 2021 22:56
Copy link
Member

@frozencemetery frozencemetery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor thing inline. Otherwise probably okay to merge with #184 once tests pass on that.

src/util.h Outdated Show resolved Hide resolved
@vathpela vathpela force-pushed the more-misc-fixes branch 3 times, most recently from 632a627 to eb8b43a Compare November 17, 2021 22:44
We have several tests that use saved variables packed with data
generated by grubenv.  Unfortunately, newer versions of grub
(rather counterproductively) changed the pointless stub header from
"# GRUB Environment Block\n" to "# GRUB Environment Block\n# WARNING: Do
not edit this file by tools other than grub-editenv!!!\n", which broke
the test cases when they actually try to re-generate the data.

This patch changes our test goals to have the new garbage.  I'll
probably regret this later.

Signed-off-by: Peter Jones <pjones@redhat.com>
list_sort() as written always adds to head on its new list, which is
wrong.

This patch updates head each time so that it's actually appending.

Signed-off-by: Peter Jones <pjones@redhat.com>
In the efivar utility, we're accidentally printing '-t' in the error
message for the '-A' argument.  This is clearly less than good.  We're
also printing the error code /twice/, presumably due to missing it in a
switch from fprintf() to err().

This fixes it to say '-A' and removes the extra output.

Signed-off-by: Peter Jones <pjones@redhat.com>
thread-test produces a lot of output even when nothing has failed, and
there's no real reason for that to need to be the case.

This adds verbosity options, and also some minor cleanups for code I
encountered while trying to figure out what it prints and when.

Signed-off-by: Peter Jones <pjones@redhat.com>
This makes "make test V=1" print what commands are being run, and
otherwise makes it silence those.

Signed-off-by: Peter Jones <pjones@redhat.com>
Several times I've wanted to inject new tests before old ones, and it's
downright impossible because of the numbering.

This patch abandons the numbering.

Signed-off-by: Peter Jones <pjones@redhat.com>
When we're doing a hexdump, most of the time people don't care that much
about the memory address of the data in program memory, especially if
it's a hexdump of a file on disk similar data.

This adds hexudmpat(), which allows us to skew the position reported in
the dump by a constant factor, so that we can show positions relative to
the structure in memory rather than the address in ram itself.

Signed-off-by: Peter Jones <pjones@redhat.com>
Some of the descriptions of what GUIDs are used for were pretty
ambiguous; this hopefully makes them more clear.

Signed-off-by: Peter Jones <pjones@redhat.com>
This adds some guids I see in the wild, as well as making
efi_guid_redhat meaningful again.

Since 054193d, what was once
efi_guid_redhat has been efi_guid_fwupdate.  Since that's used at places
that aren't exclusively from Red Hat, this is sub-optimal.
Additionally, nothing currently uses (and it's possible nothing ever
did) the invalid guid that is efi_redhat_2, though we can't remove the
related symbols.

This patch allocates a new guid value for efi_guid_redhat and
makes efi_guid_redhat_2 an alias for it, as well as adding several more
guids found in the security databases of machines found in the wild.

Signed-off-by: Peter Jones <pjones@redhat.com>
We've always had a helper macro, GUID_FORMAT, for the format specifier
for efi_guid_t, but for no good reason we've never had a helper for the
following arguments, which are easy to get wrong due to the completely
pointless byte swapping.

This patch adds GUID_FORMAT_ARGS(), which takes a pointer to an
efi_guid_t and expands as all the printf arguments that match
GUID_FORMAT.

Signed-off-by: Peter Jones <pjones@redhat.com>
Not actually using this for anything right now, but it seems useful to
have real-world data sitting around.

Signed-off-by: Peter Jones <pjones@redhat.com>
hexdump.h and util.h are both normally included from efivar.h, which
also pulls in all of the system headers that are normally needed for it.
Unfortunately, if you're editing the headers themselves, validation
tools don't know that's going on.

This change adds header includes for the things needed in those files.
This shouldn't change anything at all for this code when it's being used
as it always had been, because those headers' guard defines are already
there, but it makes it possible to run checkers on them individually.

Signed-off-by: Peter Jones <pjones@redhat.com>
At a couple of places, such as in our hexdump() functions, we use
isprint() to determine if something is reasonable to print, but
isprint('\n') returns nonzero, and we really don't want to print that
sort of thing there.

This adds a utility function safe_to_print(), which is an isprint()
wrapper that filters those out.

Signed-off-by: Peter Jones <pjones@redhat.com>
@vathpela vathpela mentioned this pull request Nov 17, 2021
@frozencemetery frozencemetery merged commit 8c8ee53 into rhboot:main Nov 22, 2021
@vathpela vathpela deleted the more-misc-fixes branch December 9, 2021 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants