getNEVRA refactor and cleanup #32
Open
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
The main package loop is busy, especially the massive switch statement, is busy and difficult to read. This commit pulls the PGP parsing code out into a separate helper. Signed-off-by: Jeff Mahoney <jeffm@suse.com>
This commit factors out a parseString helper to eliminate all the open coding of the same thing. Signed-off-by: Jeff Mahoney <jeffm@suse.com>
We open code error messages everywhere and we can instead do it in a generic way by adding the ability to pull printable tag names and types from the indexEntry. We use a generator to parse rpmtags.go and create a map from the integer values to the names. This also leads to tag names and type names that are more consistent with the C RPM library. This commit just adds the infrastructure for generic error messages. Signed-off-by: Jeff Mahoney <jeffm@suse.com>
In preparation for generic error messages, add a handler for errors that happen inside the switch statement after the switch statement. The PGP handling code can use it immediately. Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Every caller of parseString uses the indexEntry's Data field as an argument. If we pass the entire indexEntry, we could pull the tag type checking from the caller making the main loop easier to read. Rather than just do a helper, we can add a member function to do it directly. Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Every caller of parseString uses the indexEntry's Data and length fields as arguments. If we pass the entire indexEntry, we could pull the tag type checking from the caller making the main loop easier to read. Rather than just do a helper, we can add a member function to do it directly. Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Every caller of uint16Array uses the indexEntry's Data field as an argument. If we pass the entire indexEntry, we could pull the tag type checking from the caller making the main loop easier to read. Rather than just do a helper, we can add a member function to do it directly. Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Every caller of parseInt32 uses the indexEntry's Data field as an argument. If we pass the entire indexEntry, we could pull the tag type checking from the caller making the main loop easier to read. Rather than just do a helper, we can add a member function to do it directly. Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Every caller of parseString uses the indexEntry's Data and Length fields as arguments. If we pass the entire indexEntry, we could pull the tag type checking from the caller making the main loop easier to read. Rather than just do a helper, we can add a member function to do it directly. Signed-off-by: Jeff Mahoney <jeffm@suse.com>
There is only one caller now but the description and group tags that are implemented later in this series will use it. Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Now that the switch statement is easier to read, let's sort the case statements by type being handled so it's obvious where to add new tags. Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Now that there are no callers of the original parsing helpers, just pull that code into the indexEntry members. Signed-off-by: Jeff Mahoney <jeffm@suse.com>
The tags in rpmtags.go are out of order. Sorting them makes it easier to compare to the RPM C library headers. Signed-off-by: Jeff Mahoney <jeffm@suse.com>
bcrochet
approved these changes
May 18, 2023
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.
Hello -
I came across your project while looking for a way to pull package info out of a system that didn't involve calling RPM. Thanks for all the effort you've put into this. While looking at adding handling for a few new tags, I found that the main loop in getNEVRA has a bunch of open coded boilerplate that could be cleaned up and made easier to read.
This PR contains a series of cleanups that remove duplicate code and simplify the main switch statement so that it contains only what's strictly necessary. This makes adding support for new tags pretty straightforward. It reports parser/reader errors in one place and uses names for tags and types that are consistent with the RPM C API.
I have another branch that adds a bunch of tags using this one as a base. I wanted to keep that separate as I still need to update the test cases for the new fields.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.