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(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs