On 26/08/16 15:58, Pino Toscano wrote:
On Friday, 26 August 2016 15:15:17 CEST noxdafox wrote:

On 26/08/16 14:15, Pino Toscano wrote:

On Thursday, 25 August 2016 23:53:51 CEST Matteo Cafasso wrote:

With the current implementation, the root inode of the given partition
is ignored.

The root inode is now reported. Its name will be a single dot '.'
reproducing the TSK API.

Signed-off-by: Matteo Cafasso <noxdafox@gmail.com>
---
  daemon/tsk.c | 17 ++++++++++++++---
  1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/daemon/tsk.c b/daemon/tsk.c
index dd368d7..6e6df6d 100644
--- a/daemon/tsk.c
+++ b/daemon/tsk.c
@@ -48,6 +48,7 @@ static char file_type (TSK_FS_FILE *);
  static int file_flags (TSK_FS_FILE *fsfile);
  static void file_metadata (TSK_FS_META *, guestfs_int_tsk_dirent *);
  static int send_dirent_info (guestfs_int_tsk_dirent *);
+static int entry_is_dot(TSK_FS_FILE *);
  static void reply_with_tsk_error (const char *);

Since in patch #2 this forward declaration is moved, put it at the
right place already in this patch.


  int
@@ -113,9 +114,7 @@ fswalk_callback (TSK_FS_FILE *fsfile, const char *path, void *data)
    CLEANUP_FREE char *fname = NULL;
    struct guestfs_int_tsk_dirent dirent;
  
-  /* Ignore ./ and ../ */
-  ret = TSK_FS_ISDOT (fsfile->name->name);
-  if (ret != 0)
+  if (entry_is_dot(fsfile))
      return TSK_WALK_CONT;

Nitpick: add a space between the function name and the opening round
bracket.


    /* Build the full relative path of the entry */
@@ -271,6 +270,18 @@ reply_with_tsk_error (const char *funcname)
      reply_with_error ("%s: unknown error", funcname);
  }
  
+/* Check whether the entry is dot and is not Root. */
+static int
+entry_is_dot(TSK_FS_FILE *fsfile)

Ditto.


+{
+  if (TSK_FS_ISDOT (fsfile->name->name))
+    if (fsfile->fs_info->root_inum != fsfile->name->meta_addr ||
+        strcmp (fsfile->name->name, "."))

Simply merge the two if's into a single if (A && B) condition?
Also, the strcmp is already done by TSK_FS_ISDOT, isn't it?
So this should be like:

   if (TSK_FS_ISDOT (fsfile->name->name)
       && (fsfile->fs_info->root_inum != fsfile->name->meta_addr))

It's a bit more complicated, that's why I preferred to keep the two if 
statements separated.

Most probably I expressed myself in an unclear way:

* TSK_FS_ISDOT (foo) returns truen when foo is "." or ".."

* strcmp (foo, ".") -- btw, please use STREQ/STRNEQ -- returns true
  when foo is not "."
Didn't know about STREQ, it indeed makes the code more readable.

The logic was:

if IS_DOT
. if (IS_NOT_ROOT or IS_NOT_SINGLE_DOT)
.. SKIP_THE_ENTRY
else
. PARSE_THE_ENTRY

I changed it in something more readable:

if IS_DOT
. if (IS_ROOT and IS_SINGLE_DOT)
.. PARSE_THE_ENTRY
. else
.. SKIP_THE_ENTRY
else
. PARSE_THE_ENTRY


TSK_FS_ISDOT returns whether the string is either '.' or '..'.
Yet we want to return Root so we check if that's the case.

OK.


Problem is that we'd return multiple entries for Root because:
.
..
etc/..
bin/..

fsfile->name->name is a filename only, isn't it?
Yes, the callback returns the fsfile struct and the path in two separate parameters.


Are all matching the statement:

fsfile->fs_info->root_inum != fsfile->name->meta_addr

I don't understand this: if the statement will match all the dot and
dot-dot entries, why is it checked at all?
Or it will be valid for all the dot and dot-dot inodes different than
the ones in the root, right?
As I explained in the last patch series, the callback will be called for every entry.

. <-- the Root entry
etc/.
etc/.. <-- again the Root entry
etc/systemd/.
etc/systemd/..
bin/.
bin/.. <-- again the Root entry
Therefore we need to make sure we return the Root entry only once.

      

      

      

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