Re: [PATCH 4/6] tools/libfsimage: Add an fdopen() interface to libfsimage

[Alejandro Vallejo]

On 22/11/2023 22:29, Andrew Cooper wrote:
On 06/11/2023 3:05 pm, Alejandro Vallejo wrote:
diff --git a/tools/libfsimage/common/fsimage_priv.h 
b/tools/libfsimage/common/fsimage_priv.h
index 2274403557..779e433b37 100644
--- a/tools/libfsimage/common/fsimage_priv.h
+++ b/tools/libfsimage/common/fsimage_priv.h
@@ -29,6 +29,7 @@ extern C {
  #endif
  
  #include <sys/types.h>
+#include <stdbool.h>
  
  #include "xenfsimage.h"
  #include "xenfsimage_plugin.h"
@@ -54,7 +55,7 @@ struct fsi_file {
        void *ff_data;
  };
  
-int find_plugin(fsi_t *, const char *, const char *);
+int find_plugin(fsi_t *, const char *);
  
  #ifdef __cplusplus
  };

These are the only two hunks in this file.  Is the stdbool include stale?

It seems to compile fine with it removed.
I think it's leftover from code I needed initially and then didn't 
anymore. Safe to get rid of.


diff --git a/tools/libfsimage/ext2fs-lib/ext2fs-lib.c 
b/tools/libfsimage/ext2fs-lib/ext2fs-lib.c
index 864a15b349..9f07ea288f 100644
--- a/tools/libfsimage/ext2fs-lib/ext2fs-lib.c
+++ b/tools/libfsimage/ext2fs-lib/ext2fs-lib.c
@@ -25,15 +25,25 @@
  #include INCLUDE_EXTFS_H
  #include <errno.h>
  #include <inttypes.h>
+#include <stdio.h>
  
  static int
-ext2lib_mount(fsi_t *fsi, const char *name, const char *options)
+ext2lib_mount(fsi_t *fsi, const char *options)
  {
        int err;
        char opts[30] = "";
        ext2_filsys *fs;
        uint64_t offset = fsip_fs_offset(fsi);
  
+	/*
+        * We must choose unixfd_io_manager rather than unix_io_manager in
+        * order for the library to accept fd strings instead of paths. It
+        * still means we must pass a string representing an fd rather than
+        * an int, but at least this way we don't need to pass paths around
+        */
+       char name[32] = {0};

For an int?  12 will do fine including a terminator, and practical
system limits leave it far smaller than that generally.
Maybe. I admit optimising that line went pretty low on the list of 
things I cared terribly about. I'm happy to reduce it, but it is 
inconsequential.

Given that it is guaranteed long enough, you don't need to zero it just
to have snprintf() write a well-formed string in.

~Andrew
For wellformed fd's that's true. Through bugs or malice that may not be 
the case. I'm happier knowing at least the last zero remains.

Cheers,
Alejandro