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

Commit Message

Eric Biggers June 3, 2021, 7:58 p.m. UTC

From: Eric Biggers <[e-mail]>

These helper functions will be used by the implementation of the
--out-merkle-tree option for 'fsverity digest' and 'fsverity sign'.

Signed-off-by: Eric Biggers <[e-mail]>
---
 programs/utils.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++
 programs/utils.h |  3 +++
 2 files changed, 62 insertions(+)

Comments

Victor Hsieh June 4, 2021, 12:33 a.m. UTC | #1

On Thu, Jun 3, 2021 at 1:00 PM Eric Biggers <[e-mail]> wrote:
>
> From: Eric Biggers <[e-mail]>
>
> These helper functions will be used by the implementation of the
> --out-merkle-tree option for 'fsverity digest' and 'fsverity sign'.
>
> Signed-off-by: Eric Biggers <[e-mail]>
> ---
>  programs/utils.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++
>  programs/utils.h |  3 +++
>  2 files changed, 62 insertions(+)
>
> diff --git a/programs/utils.c b/programs/utils.c
> index ce19b57..116eb95 100644
> --- a/programs/utils.c
> +++ b/programs/utils.c
> @@ -13,10 +13,14 @@
>
>  #include <errno.h>
>  #include <fcntl.h>
> +#include <inttypes.h>
>  #include <limits.h>
>  #include <stdarg.h>
>  #include <sys/stat.h>
>  #include <unistd.h>
> +#ifdef _WIN32
> +#  include <windows.h>
> +#endif
>
>  /* ========== Memory allocation ========== */
>
> @@ -126,6 +130,26 @@ bool get_file_size(struct filedes *file, u64 *size_ret)
>         return true;
>  }
>
> +bool preallocate_file(struct filedes *file, u64 size)
> +{
> +       int res;
> +
> +       if (size == 0)
> +               return true;
> +#ifdef _WIN32
> +       /* Not exactly the same as posix_fallocate(), but good enough... */
> +       res = _chsize_s(file->fd, size);
> +#else
> +       res = posix_fallocate(file->fd, 0, size);
> +#endif
> +       if (res != 0) {
> +               error_msg_errno("preallocating %" PRIu64 "-byte file '%s'",
> +                               size, file->name);
> +               return false;
> +       }
> +       return true;
> +}
> +
>  bool full_read(struct filedes *file, void *buf, size_t count)
>  {
>         while (count) {
> @@ -160,6 +184,41 @@ bool full_write(struct filedes *file, const void *buf, size_t count)
>         return true;
>  }
>
> +static int raw_pwrite(int fd, const void *buf, int count, u64 offset)
> +{
> +#ifdef _WIN32
> +       HANDLE h = (HANDLE)_get_osfhandle(fd);
> +       OVERLAPPED pos = { .Offset = offset, .OffsetHigh = offset >> 32 };
> +       DWORD written = 0;
> +
> +       /* Not exactly the same as pwrite(), but good enough... */
> +       if (!WriteFile(h, buf, count, &written, &pos)) {
> +               errno = EIO;
> +               return -1;
> +       }
> +       return written;
> +#else
> +       return pwrite(fd, buf, count, offset);
> +#endif
> +}
> +
> +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*".

> +               count -= n;
> +               offset += n;
> +       }
> +       return true;
> +}
> +
>  bool filedes_close(struct filedes *file)
>  {
>         int res;
> diff --git a/programs/utils.h b/programs/utils.h
> index ab5005f..9a5c97a 100644
> --- a/programs/utils.h
> +++ b/programs/utils.h
> @@ -40,8 +40,11 @@ struct filedes {
>
>  bool open_file(struct filedes *file, const char *filename, int flags, int mode);
>  bool get_file_size(struct filedes *file, u64 *size_ret);
> +bool preallocate_file(struct filedes *file, u64 size);
>  bool full_read(struct filedes *file, void *buf, size_t count);
>  bool full_write(struct filedes *file, const void *buf, size_t count);
> +bool full_pwrite(struct filedes *file, const void *buf, size_t count,
> +                u64 offset);
>  bool filedes_close(struct filedes *file);
>  int read_callback(void *file, void *buf, size_t count);
>
> --
> 2.31.1
>

Eric Biggers June 4, 2021, 12:57 a.m. UTC | #2

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.

- Eric

Victor Hsieh June 4, 2021, 3:24 p.m. UTC | #3

On Thu, Jun 3, 2021 at 5:58 PM Eric Biggers <[e-mail]> 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 <[e-mail]>

>
> - Eric

Eric Biggers June 4, 2021, 4:55 p.m. UTC | #4

On Fri, Jun 04, 2021 at 08:24:50AM -0700, Victor Hsieh wrote:
> On Thu, Jun 3, 2021 at 5:58 PM Eric Biggers <[e-mail]> 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 <[e-mail]>
> 

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

- Eric

Patch

diff --git a/programs/utils.c b/programs/utils.c
index ce19b57..116eb95 100644
--- a/programs/utils.c
+++ b/programs/utils.c
@@ -13,10 +13,14 @@ 
 
 #include <errno.h>
 #include <fcntl.h>
+#include <inttypes.h>
 #include <limits.h>
 #include <stdarg.h>
 #include <sys/stat.h>
 #include <unistd.h>
+#ifdef _WIN32
+#  include <windows.h>
+#endif
 
 /* ========== Memory allocation ========== */
 
@@ -126,6 +130,26 @@  bool get_file_size(struct filedes *file, u64 *size_ret)
 	return true;
 }
 
+bool preallocate_file(struct filedes *file, u64 size)
+{
+	int res;
+
+	if (size == 0)
+		return true;
+#ifdef _WIN32
+	/* Not exactly the same as posix_fallocate(), but good enough... */
+	res = _chsize_s(file->fd, size);
+#else
+	res = posix_fallocate(file->fd, 0, size);
+#endif
+	if (res != 0) {
+		error_msg_errno("preallocating %" PRIu64 "-byte file '%s'",
+				size, file->name);
+		return false;
+	}
+	return true;
+}
+
 bool full_read(struct filedes *file, void *buf, size_t count)
 {
 	while (count) {
@@ -160,6 +184,41 @@  bool full_write(struct filedes *file, const void *buf, size_t count)
 	return true;
 }
 
+static int raw_pwrite(int fd, const void *buf, int count, u64 offset)
+{
+#ifdef _WIN32
+	HANDLE h = (HANDLE)_get_osfhandle(fd);
+	OVERLAPPED pos = { .Offset = offset, .OffsetHigh = offset >> 32 };
+	DWORD written = 0;
+
+	/* Not exactly the same as pwrite(), but good enough... */
+	if (!WriteFile(h, buf, count, &written, &pos)) {
+		errno = EIO;
+		return -1;
+	}
+	return written;
+#else
+	return pwrite(fd, buf, count, offset);
+#endif
+}
+
+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;
+		count -= n;
+		offset += n;
+	}
+	return true;
+}
+
 bool filedes_close(struct filedes *file)
 {
 	int res;
diff --git a/programs/utils.h b/programs/utils.h
index ab5005f..9a5c97a 100644
--- a/programs/utils.h
+++ b/programs/utils.h
@@ -40,8 +40,11 @@  struct filedes {
 
 bool open_file(struct filedes *file, const char *filename, int flags, int mode);
 bool get_file_size(struct filedes *file, u64 *size_ret);
+bool preallocate_file(struct filedes *file, u64 size);
 bool full_read(struct filedes *file, void *buf, size_t count);
 bool full_write(struct filedes *file, const void *buf, size_t count);
+bool full_pwrite(struct filedes *file, const void *buf, size_t count,
+		 u64 offset);
 bool filedes_close(struct filedes *file);
 int read_callback(void *file, void *buf, size_t count);