From: Eric Biggers <ebiggers@kernel.org>
To: Jes Sorensen <jes@trained-monkey.org>
Cc: linux-fscrypt@vger.kernel.org, kernel-team@fb.com,
Jes Sorensen <jsorensen@fb.com>
Subject: Re: [PATCH v2 0/6] Split fsverity-utils into a shared library
Date: Tue, 10 Mar 2020 14:10:02 -0700 [thread overview]
Message-ID: <20200310211002.GA46757@gmail.com> (raw)
In-Reply-To: <6486476e-2109-cbd5-07d0-4c310d2c9f06@trained-monkey.org>
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@fb.com>
> >
> > 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
next prev parent reply other threads:[~2020-03-10 21:10 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-28 21:28 [PATCH v2 0/6] Split fsverity-utils into a shared library Jes Sorensen
2020-02-28 21:28 ` [PATCH 1/6] Build basic shared library framework Jes Sorensen
2020-02-28 21:28 ` [PATCH 2/6] Change compute_file_measurement() to take a file descriptor as argument Jes Sorensen
2020-02-28 21:28 ` [PATCH 3/6] Move fsverity_descriptor definition to libfsverity.h Jes Sorensen
2020-02-28 21:28 ` [PATCH 4/6] Move hash algorithm code to shared library Jes Sorensen
2020-02-28 21:28 ` [PATCH 5/6] Create libfsverity_compute_digest() and adapt cmd_sign to use it Jes Sorensen
2020-02-28 21:28 ` [PATCH 6/6] Introduce libfsverity_sign_digest() Jes Sorensen
2020-03-10 20:32 ` [PATCH v2 0/6] Split fsverity-utils into a shared library Jes Sorensen
2020-03-10 21:10 ` Eric Biggers [this message]
2020-03-10 21:53 ` Jes Sorensen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200310211002.GA46757@gmail.com \
--to=ebiggers@kernel.org \
--cc=jes@trained-monkey.org \
--cc=jsorensen@fb.com \
--cc=kernel-team@fb.com \
--cc=linux-fscrypt@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox