Re: [Libguestfs] [PATCH v2 1/6] filesystem_walk: fixed root inode listing

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(a)gmail.com&gt; >> --- >> 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.

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

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

Are all matching the statement: fsfile->fs_info->root_inum != fsfile->name->meta_addr