[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH] filesystem_walk: more information into tsk_dirent



On Monday 04 July 2016 21:57:31 Matteo Cafasso wrote:
> Access, modification, last status change and creation time in
> Unix format as for statns.
> 
> Number of links pointing to a given entry.
> 
> If the entry is a symbolic link, report the its target path.
> 
> A new flag (DIRENT_COMPRESSED 0x04) indicating whether the file is
> compressed using native filesystem compression support.
> 
> Signed-off-by: Matteo Cafasso <noxdafox gmail com>
> ---
>  daemon/tsk.c                      | 59 ++++++++++++++++++++++++++++++++++-----
>  generator/actions.ml              | 39 ++++++++++++++++++++++++--
>  generator/structs.ml              | 20 ++++++-------
>  tests/tsk/test-filesystem-walk.sh | 44 ++++++++++++++---------------
>  4 files changed, 121 insertions(+), 41 deletions(-)
> 
> diff --git a/daemon/tsk.c b/daemon/tsk.c
> index 446213e..b0fb3a7 100644
> --- a/daemon/tsk.c
> +++ b/daemon/tsk.c
> @@ -38,13 +38,15 @@
>  enum tsk_dirent_flags {
>    DIRENT_UNALLOC = 0x00,
>    DIRENT_ALLOC = 0x01,
> -  DIRENT_REALLOC = 0x02
> +  DIRENT_REALLOC = 0x02,
> +  DIRENT_COMPRESSED = 0x04
>  };
> 
>  static int open_filesystem (const char *, TSK_IMG_INFO **, TSK_FS_INFO **);
>  static TSK_WALK_RET_ENUM fswalk_callback (TSK_FS_FILE *, const char *, void *);
>  static char file_type (TSK_FS_FILE *);
>  static int file_flags (TSK_FS_FILE *fsfile);
> +static int file_metadata (TSK_FS_META *, guestfs_int_tsk_dirent *);
>  static int send_dirent_info (guestfs_int_tsk_dirent *);
>  static void reply_with_tsk_error (const char *);
> 
> @@ -122,19 +124,24 @@ fswalk_callback (TSK_FS_FILE *fsfile, const char *path, void *data)
>      return TSK_WALK_ERROR;
>    }
> 
> +  /* Set dirent fields */
> +  memset (&dirent, 0, sizeof dirent);
> +
>    dirent.tsk_inode = fsfile->name->meta_addr;
>    dirent.tsk_type = file_type (fsfile);
> -  dirent.tsk_size = (fsfile->meta != NULL) ? fsfile->meta->size : -1;
>    dirent.tsk_name = fname;
>    dirent.tsk_flags = file_flags (fsfile);
> -  dirent.tsk_spare1 = dirent.tsk_spare2 = dirent.tsk_spare3 =
> -    dirent.tsk_spare4 = dirent.tsk_spare5 = dirent.tsk_spare6 =
> -    dirent.tsk_spare7 = dirent.tsk_spare8 = dirent.tsk_spare9 =
> -    dirent.tsk_spare10 = dirent.tsk_spare11 = 0;
> +
> +  ret = file_metadata (fsfile->meta, &dirent);
> +  if (ret < 0)
> +    return TSK_WALK_ERROR;
> 
>    ret = send_dirent_info (&dirent);
>    ret = (ret == 0) ? TSK_WALK_CONT : TSK_WALK_ERROR;
> 
> +  if (strlen(dirent.tsk_link) == 0)
> +    free (dirent.tsk_link);

You don't need strlen to check whether an empty string is empty: any of
the variants:
  *s == 0
  *s == '\0'
  s[0] == 0
  s[0] == '\0'
does the job.

But in this case you don't even need this, see below.

> +
>    return ret;
>  }
> 
> @@ -175,7 +182,7 @@ file_type (TSK_FS_FILE *fsfile)
>    return 'u';
>  }
> 
> -/* Inspect fsfile to retrieve the file allocation state. */
> +/* Inspect fsfile to retrieve file flags. */
>  static int
>  file_flags (TSK_FS_FILE *fsfile)
>  {
> @@ -188,9 +195,47 @@ file_flags (TSK_FS_FILE *fsfile)
>    else
>      flags |= DIRENT_ALLOC;
> 
> +  if (fsfile->meta && fsfile->meta->flags & TSK_FS_META_FLAG_COMP)
> +    flags |= DIRENT_COMPRESSED;
> +
>    return flags;
>  }
> 
> +/* Inspect fsfile to retrieve file metadata. */
> +static int
> +file_metadata (TSK_FS_META *fsmeta, guestfs_int_tsk_dirent *dirent)
> +{
> +  if (fsmeta != NULL) {
> +    dirent->tsk_size = fsmeta->size;
> +    dirent->tsk_nlink = fsmeta->nlink;
> +    dirent->tsk_atime_sec = fsmeta->atime;
> +    dirent->tsk_atime_nsec = fsmeta->atime_nano;
> +    dirent->tsk_mtime_sec = fsmeta->mtime;
> +    dirent->tsk_mtime_nsec = fsmeta->mtime_nano;
> +    dirent->tsk_ctime_sec = fsmeta->ctime;
> +    dirent->tsk_ctime_nsec = fsmeta->ctime_nano;
> +    dirent->tsk_crtime_sec = fsmeta->crtime;
> +    dirent->tsk_crtime_nsec = fsmeta->crtime_nano;
> +
> +    dirent->tsk_link = (fsmeta->link != NULL) ? fsmeta->link : strdup ("");

No need to duplicate an empty string, just assign it to the string:

  dirent->tsk_link = (fsmeta->link != NULL) ? fsmeta->link : (char *) "";

"" is stored as static const char[], so you will need to cast away the
constness -- a bit ugly, but given that tsk_link is never changed
should be fine. (Add a comment there, so it is more clear.)

This avoids the need to free the empty string in fswalk_callback.

> +    if (dirent->tsk_link == NULL) {
> +      perror ("strdup");
> +      return -1;
> +    }
> +  }
> +  else {
> +    dirent->tsk_size = -1;
> +
> +    dirent->tsk_link = strdup ("");
> +    if (dirent->tsk_link == NULL) {
> +      perror ("strdup");
> +      return -1;
> +    }

Ditto -- you can also simplify this and the blocks above in a single
one:
- in the "if (fsmeta != NULL)", directly assign
  "dirent->tsk_link = fsmeta->link"
- after the if (so whichever value is fsmeta), assign an empty string
  to tsk_link if not set:
    if (dirent->tsk_link == NULL)
      dirent->tsk_link = (char *) ""; /* with the comment as above */

> +  }
> +
> +  return 0;
> +}
> +
>  /* Serialise dirent into XDR stream and send it to the appliance.
>   * Return 0 on success, -1 on error.
>   */
> diff --git a/generator/actions.ml b/generator/actions.ml
> index e0931b8..78d0a73 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -3612,11 +3612,46 @@ from the metadata structure.
>  The bit is set to C<1> when the file name is in an unallocated state
>  and the metadata structure is in an allocated one.
>  This generally implies the metadata has been reallocated to a new file.
> -Therefore, information such as file type and file size
> -might not correspond with the ones of the original deleted entry.
> +Therefore, information such as file type, file size, timestamps,
> +number of links and symlink target might not correspond
> +with the ones of the original deleted entry.
> +
> +=item 0x0004
> +
> +The bit is set to C<1> when the file is compressed using filesystem
> +native compression support (NTFS). The API is not able to detect
> +application level compression.

"... using support in the filesystem for native compression (e.g. on
NTFS).  The API is not able to detect compression at application level."

Not a native English speaker either, but IMHO this way it'd be slightly
better.

> 
>  =back
> 
> +=item 'tsk_atime_sec'
> +
> +=item 'tsk_atime_nsec'
> +
> +=item 'tsk_mtime_sec'
> +
> +=item 'tsk_mtime_nsec'
> +
> +=item 'tsk_ctime_sec'
> +
> +=item 'tsk_ctime_nsec'
> +
> +=item 'tsk_crtime_sec'
> +
> +=item 'tsk_crtime_nsec'
> +
> +Respectively, access, modification, last status change and creation
> +time in Unix format in seconds and nanoseconds.

"Respectively: ..."

Thanks,
-- 
Pino Toscano

Attachment: signature.asc
Description: This is a digitally signed message part.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]