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

Re: [Libguestfs] [Hivex][PATCH v2] Report last-modified time of hive root and nodes



On Wed, Aug 10, 2011 at 12:07:23PM -0700, Alex‎ Nelson wrote:
> The infrastructure for modified-time reporting has been essentially
> unused.  These changes report the registry time by treating the
> time fields as Windows filetime fields stored in little-Endian
> (which means they can be treated as a single 64-bit little-Endian
> integer).
> 
> This patch adds the node_mtime function to the visitor API.

Nearly there.  See my comments below.

> @@ -93,6 +94,7 @@ struct hive_h {
>    /* Fields from the header, extracted from little-endianness hell. */
>    size_t rootoffs;              /* Root key offset (always an nk-block). */
>    size_t endpages;              /* Offset of end of pages. */
> +  char *last_modified;          /* mtime of base block. */

This field seems to be unused except in debugging messages?  Unless
you're going to use it, I'd just omit this field and all the code that
generates and prints last_modified.

> +#define WINDOWS_TICK 10000000LL
> +#define SEC_TO_UNIX_EPOCH 11644473600LL
> +/**
> + * Convert Windows filetime to ISO 8601 format.
> + * Source for filetime->time_t conversion:  http://stackoverflow.com/questions/6161776/convert-windows-filetime-to-second-in-unix-linux/6161842#6161842
> + * Source for time_t->char* conversion:  Fiwalk version 0.6.14's fiwalk.cpp.
> + * @param windows_ticks Expected to not have any remaining Endian issues.
> + */
> +int
> +filetime_to_8601 (char *buf, int bufsize, uint64_t windows_ticks)

This function is an oddity.  You're taking a useful thing (64 bit
Windows filetime) and converting it to a string.  So callers are going
to need to parse this string if they want to do anything useful with
it.  I think it's best just to return the 64 bit Windows filetime (but
with the correct endianness) and let callers deal with it.  Passing
back strings from a C API just seems wrong.

> +/* Caller responsible for freeing returned char* if non-null. */
> +char *
> +hivex_node_mtime (hive_h *h, hive_node_h node)

Add this function to the API (in generator/generator.ml, 'functions').
This will ensure that Perl, OCaml etc bindings + documentation are
generated automatically for the function.

Don't make it return a string.  Have it return the original 64 bit
Windows filetime instead (ie. RInt64) with the correct endianness, and
let callers deal with it.

The rest seems fine to me, except that you'll need to push the
filetime_to_8601 function into hivexml.c since it will now be getting
a 64 bit filetime.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org


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