Re: [Libguestfs] [PATCH 4/4] TSK: small refactoring

[Pino Toscano]

On Saturday, 17 September 2016 18:18:56 CEST Matteo Cafasso wrote:
> Removed duplicated code.
> 
> Signed-off-by: Matteo Cafasso 
> ---

As mentioned in another email, I'd like this to be a single change
with the return parse_filesystem_walk -> parse_dirent_file rename.

>  struct guestfs_tsk_dirent_list *
>  guestfs_impl_filesystem_walk (guestfs_h *g, const char *mountable)
>  {
>int ret = 0;
> -  CLEANUP_FCLOSE FILE *fp = NULL;
>CLEANUP_UNLINK_FREE char *tmpfile = NULL;
> 
> -  ret = guestfs_int_lazy_make_tmpdir (g);
> -  if (ret < 0)
> -return NULL;
> -
> -  tmpfile = safe_asprintf (g, "%s/filesystem_walk%d", g->tmpdir, 
> ++g->unique);
> +  tmpfile = make_temp_file (g, "filesystem_walk");

Note you are not checking the return value of make_temp_file, which can
still fail (when guestfs_int_lazy_make_tmpdir fails, for example).
So you still need to check tmpfile, otherwise tmpfile may be used as
null pointer.

Thanks,
-- 
Pino Toscano

signature.asc
Description: This is a digitally signed message part.
___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [PATCH 4/4] TSK: small refactoring

[Matteo Cafasso]

Removed duplicated code.

Signed-off-by: Matteo Cafasso 
---
 src/tsk.c | 69 ++-
 1 file changed, 28 insertions(+), 41 deletions(-)

diff --git a/src/tsk.c b/src/tsk.c
index 6b1b29c..8e6d266 100644
--- a/src/tsk.c
+++ b/src/tsk.c
@@ -34,96 +34,71 @@
 #include "guestfs-internal-all.h"
 #include "guestfs-internal-actions.h"

-static struct guestfs_tsk_dirent_list *parse_dirent_file (guestfs_h *, FILE *);
+static struct guestfs_tsk_dirent_list *parse_dirent_file (guestfs_h *, const 
char *);
 static int deserialise_dirent_list (guestfs_h *, FILE *, struct 
guestfs_tsk_dirent_list *);
+static char *make_temp_file (guestfs_h *, const char *);

 struct guestfs_tsk_dirent_list *
 guestfs_impl_filesystem_walk (guestfs_h *g, const char *mountable)
 {
   int ret = 0;
-  CLEANUP_FCLOSE FILE *fp = NULL;
   CLEANUP_UNLINK_FREE char *tmpfile = NULL;

-  ret = guestfs_int_lazy_make_tmpdir (g);
-  if (ret < 0)
-return NULL;
-
-  tmpfile = safe_asprintf (g, "%s/filesystem_walk%d", g->tmpdir, ++g->unique);
+  tmpfile = make_temp_file (g, "filesystem_walk");

   ret = guestfs_internal_filesystem_walk (g, mountable, tmpfile);
   if (ret < 0)
 return NULL;

-  fp = fopen (tmpfile, "r");
-  if (fp == NULL) {
-perrorf (g, "fopen: %s", tmpfile);
-return NULL;
-  }
-
-  return parse_dirent_file (g, fp);  /* caller frees */
+  return parse_dirent_file (g, tmpfile);  /* caller frees */
 }

 struct guestfs_tsk_dirent_list *
 guestfs_impl_find_inode (guestfs_h *g, const char *mountable, int64_t inode)
 {
   int ret = 0;
-  CLEANUP_FCLOSE FILE *fp = NULL;
   CLEANUP_UNLINK_FREE char *tmpfile = NULL;

-  ret = guestfs_int_lazy_make_tmpdir (g);
-  if (ret < 0)
-return NULL;
-
-  tmpfile = safe_asprintf (g, "%s/find_inode%d", g->tmpdir, ++g->unique);
+  tmpfile = make_temp_file (g, "find_inode");

   ret = guestfs_internal_find_inode (g, mountable, inode, tmpfile);
   if (ret < 0)
 return NULL;

-  fp = fopen (tmpfile, "r");
-  if (fp == NULL) {
-perrorf (g, "fopen: %s", tmpfile);
-return NULL;
-  }
-
-  return parse_dirent_file (g, fp);  /* caller frees */
+  return parse_dirent_file (g, tmpfile);  /* caller frees */
 }

 struct guestfs_tsk_dirent_list *
 guestfs_impl_find_block (guestfs_h *g, const char *mountable, int64_t block)
 {
   int ret = 0;
-  CLEANUP_FCLOSE FILE *fp = NULL;
   CLEANUP_UNLINK_FREE char *tmpfile = NULL;

-  ret = guestfs_int_lazy_make_tmpdir (g);
-  if (ret < 0)
-return NULL;
-
-  tmpfile = safe_asprintf (g, "%s/find_block%d", g->tmpdir, ++g->unique);
+  tmpfile = make_temp_file (g, "find_block");

   ret = guestfs_internal_find_block (g, mountable, block, tmpfile);
   if (ret < 0)
 return NULL;

-  fp = fopen (tmpfile, "r");
-  if (fp == NULL) {
-perrorf (g, "fopen: %s", tmpfile);
-return NULL;
-  }
-
-  return parse_dirent_file (g, fp);  /* caller frees */
+  return parse_dirent_file (g, tmpfile);  /* caller frees */
 }

 /* Parse the file content and return dirents list.
  * Return a list of tsk_dirent on success, NULL on error.
  */
 static struct guestfs_tsk_dirent_list *
-parse_dirent_file (guestfs_h *g, FILE *fp)
+parse_dirent_file (guestfs_h *g, const char *tmpfile)
 {
   int ret = 0;
+  CLEANUP_FCLOSE FILE *fp = NULL;
   struct guestfs_tsk_dirent_list *dirents = NULL;

+  fp = fopen (tmpfile, "r");
+  if (fp == NULL) {
+perrorf (g, "fopen: %s", tmpfile);
+return NULL;
+  }
+
   /* Initialise results array. */
   dirents = safe_malloc (g, sizeof (*dirents));
   dirents->len = 8;
@@ -178,3 +153,15 @@ deserialise_dirent_list (guestfs_h *g, FILE *fp,

   return ret ? 0 : -1;
 }
+
+static char *
+make_temp_file (guestfs_h *g, const char *name)
+{
+  int ret = 0;
+
+  ret = guestfs_int_lazy_make_tmpdir (g);
+  if (ret < 0)
+return NULL;
+
+  return safe_asprintf (g, "%s/%s%d", g->tmpdir, name, ++g->unique);
+}
--
2.9.3

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs