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 protected]>" >> [...] >> + 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 protected]>" 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
signature.asc
Description: OpenPGP digital signature
