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

[Alex‎ Nelson]

On Aug 10, 2011, at 12:55 , Richard W.M. Jones wrote:

> 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.
There is a spot where it is printed:  in lib/hivex.c:hivex__visit_node, "Report extra for hive's root node".

> 
>> +#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.

These changes are bringing the hivexml program into a file system analysis suite that deals with many different file system types, each with their own timestamp recording quirks, and even some file formats which have yet more quirks.  We think that ISO 8601 is the best umbrella output format, with an additional XML attribute noting the time granularity (like FAT's 2-second and 1-day granularities).  That's why we're outputting strings in C, which, yes, feels wrong, but simplifies parsing outside of the scope of hivexml.  We're dealing with the time presentation proactively.

To prevent information loss, I could add decimal points to get the time reporting accurate to the 100-nanosecond level.

> 
>> +/* 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.
I hope that adding that function only entailed copy-pasting the list entry for node_name, assuming you accept the string return type.  I have the next patch version formatted, will send after the above discussion's resolved.

--Alex


> 
> 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