On Wednesday, September 8, 2021 4:48 PM, Eric Biggers wrote: > Regarding struct libfsverity_signature_params, I wrote "Please write a comment > that clearly explains which parameters must be specified and when.". Got it. I assumed that the detailed explanation in the manpage covering the same parameters would be sufficient, as repeating it in struct comments would make the information redundant and require reformatting that part to multi-line comments. I can add it to the struct comments, but this will mean I'll need to change them to multi-line comments (above each struct member) and add empty lines between members (following the same commenting style as in struct libfsverity_merkle_tree_params). Are you okay with that change? > Also I mentioned "The !OPENSSL_IS_BORINGSSL case no longer returns an error if > sig_params->keyfile or sig_params->certfile is unset". That wasn't addressed > for sig_params->certfile. Ah, I see. In my patch V2, after your suggestion, there's a new NULL check for certfile in lib/sign_digest.c:87 that I intended as a replacement for the previous check in lib/sign_digest.c:337. I think it's a better place for that check, as it's in the place of actual use. Do you want me to place that check back in the pre-check logic in libfsverity_sign_digest()?
|