All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: Victor Hsieh <victorhsieh@google.com>
Cc: linux-fscrypt@vger.kernel.org
Subject: Re: [fsverity-utils PATCH 3/4] programs/utils: add full_pwrite() and preallocate_file()
Date: Fri, 4 Jun 2021 09:55:37 -0700	[thread overview]
Message-ID: <YLpbCRnVVGGkjl19@sol.localdomain> (raw)
In-Reply-To: <CAFCauYNPvC=otUWXD5ytqM5rvoaMBLXXfdGHTUcCvWwFG4PEmw@mail.gmail.com>

On Fri, Jun 04, 2021 at 08:24:50AM -0700, Victor Hsieh wrote:
> On Thu, Jun 3, 2021 at 5:58 PM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > On Thu, Jun 03, 2021 at 05:33:18PM -0700, Victor Hsieh wrote:
> > > > +
> > > > +bool full_pwrite(struct filedes *file, const void *buf, size_t count,
> > > > +                u64 offset)
> > > > +{
> > > > +       while (count) {
> > > > +               int n = raw_pwrite(file->fd, buf, min(count, INT_MAX), offset);
> > > > +
> > > > +               if (n < 0) {
> > > > +                       error_msg_errno("writing to '%s'", file->name);
> > > > +                       return false;
> > > > +               }
> > > > +               buf += n;
> > > I think this pointer arithmetic is not portable?  Consider changing
> > > the type of buf to "const char*".
> > >
> >
> > fsverity-utils is already using void pointer arithmetic elsewhere, for example
> > in full_read() and full_write().
> >
> > I am allowing the use of some gcc/clang extensions which are widely used,
> > including in the Linux kernel (which fsverity-utils is generally trying to
> > follow the coding style of), and are annoying to do without.  Void pointer
> > arithmetic is one of these.
> >
> > If we really needed to support someone compiling fsverity-utils with e.g.
> > Visual Studio, we could add -Wpedantic to the compiler flags and get rid of all
> > the gcc/clang extensions.  But I don't see a reason to do that now.
> 
> Yeah, that's what I was thinking since the code has #ifdef _WIN32.
> I'd think the
> "host" side program should be more portable than the kernel itself.
> But if this is
> already used elsewhere, no objection to keeping assuming so.
> 
> Reviewed-by: Victor Hsieh <victorhsieh@google.com>
> 

Windows builds are supported with Mingw-w64, not with Visual Studio.  So that
isn't an issue.

- Eric

  reply	other threads:[~2021-06-04 16:55 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-03 19:58 [fsverity-utils PATCH 0/4] Add option to write Merkle tree to a file Eric Biggers
2021-06-03 19:58 ` [fsverity-utils PATCH 1/4] lib/compute_digest: add callbacks for getting the verity metadata Eric Biggers
2021-06-03 19:58 ` [fsverity-utils PATCH 2/4] programs/test_compute_digest: test the metadata callbacks Eric Biggers
2021-06-03 19:58 ` [fsverity-utils PATCH 3/4] programs/utils: add full_pwrite() and preallocate_file() Eric Biggers
2021-06-04  0:33   ` Victor Hsieh
2021-06-04  0:57     ` Eric Biggers
2021-06-04 15:24       ` Victor Hsieh
2021-06-04 16:55         ` Eric Biggers [this message]
2021-06-03 19:58 ` [fsverity-utils PATCH 4/4] programs/fsverity: add --out-merkle-tree and --out-descriptor options Eric Biggers
2021-06-04 15:25 ` [fsverity-utils PATCH 0/4] Add option to write Merkle tree to a file Victor Hsieh
2021-06-09  6:48 ` Eric Biggers

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=YLpbCRnVVGGkjl19@sol.localdomain \
    --to=ebiggers@kernel.org \
    --cc=linux-fscrypt@vger.kernel.org \
    --cc=victorhsieh@google.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.