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@xxxxxxxxxx> 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@xxxxxxxxxx>
>
Windows builds are supported with Mingw-w64, not with Visual Studio. So that
isn't an issue.
- Eric
|