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>
>
> - Eric
|