Hi, thank you for the review! On 2019-08-05 13:14, Allan McRae wrote:
+ errors = alpm_list_add(errors, email); errors = alpm_list_add(errors, strdup(key));
I don't like this. Storing two strings as adjacent items in the list.
I'd prefer a small two item struct.
Any other opinions on this?
Done, it now uses a struct with two members "email" and "keyid". A bit more work because we need to free the strings manually now, but I agree it is much cleaner.
+/** Extract the email address from a User ID + * @param uid User ID to parse in the form "Example Name <email@address.invalid>" [...] + start = strrchr(uid, '<');
This makes a strong assumption that "<" is not used within an email address. The use of that character is technically valid, provided it is quoted.
I am happy with that assumption, but we need to add a check in libmakpkeg to reject emails containing it.
In fact, our PACKAGER variable has no enforced format at all...
I sent a separate patch for libmakepkg to issue a warning if PACKAGER doesn't have the expected format. I opted for a warning instead of a hard error because I don't know what other distributions using pacman do - for Arch the "Example Name <email@address.invalid>" is used consistently by all packagers (except for Xyne, who doesn't use an email address at all).
+int _alpm_email_from_uid(const char *uid, char **email);
Rename to:
_alpm_email_from_packager()
Done, this also affected "[PATCH 4/5] be_package: lookup missing keys in the WKD using the packager email" because the function is used there as well. I also published the updated patch series as the "wkd-v2" branch of https://gitlab.com/diabonas/pacman Kind regards, Jonas