On Tue, Mar 10, 2020 at 04:32:12PM -0400, Jes Sorensen wrote: > On 2/28/20 4:28 PM, Jes Sorensen wrote: > > From: Jes Sorensen <jsorensen@xxxxxx> > > > > Hi, > > > > Here is a reworked version of the patches to split fsverity-utils into > > a shared library, based on the feedback for the original version. Note > > this doesn't yet address setting the soname, and doesn't have the > > client (rpm) changes yet, so there is more work to do. > > > > Comments appreciated. > > Hi, > > Any thoughts on this patchset? > > Thanks, > Jes > It's been on my list of things to review but I've been pretty busy. But a few quick comments now: The API needs documentation. It doesn't have to be too formal; comments in libfsverity.h would be fine. Did you check that the fs-verity xfstests still pass? They use fsverity-utils. See: https://www.kernel.org/doc/html/latest/filesystems/fsverity.html#tests struct fsverity_descriptor and struct fsverity_hash_alg are still part of the API. But there doesn't seem to be any point in it. Why aren't they internal to libfsverity? Can you make sure that the set of error codes for each API function is clearly defined? Can you make sure all API functions return an error if any reserved fields are set? Do you have a pointer to the corresponding RPM patches that will use this? Also, it would be nice if you could also add some tests of the API to fsverity-utils itself :-) - Eric
|