Re: [fsverity-utils PATCH 3/4] programs/utils: add full_pwrite() and preallocate_file()

[Victor Hsieh <victorhsieh@google.com>]

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>

>
> - Eric