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

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



On Aug 10, 2011, at 02:44 , Richard W.M. Jones wrote:

> On Tue, Aug 09, 2011 at 09:54:40PM -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).  Some of the code changes necessary include:
>> 
>> * Exposing the hive_h structure in the hivex header file (via
>>   generator.ml)
> 
> You can't make the hive_h internal structure visible in the header
> file.  I don't understand why you'd want callers to be delving into
> the internals of the handle anyway.  What's the reason for this
> change?
The reason for this change was to expose some of the handle's fields.  I wasn't sure how to detect being at the root node.  It also looked like a good place to stash the last-modified time of the root node.  With your ABI suggestions below, I think I have a better way that doesn't muck with the handle structure.

> 
>> * Adding an additional argument to the node_start function, which
>>   should cause no complications since the change is specific to the
>>   C API.
> 
> This is a change to the C ABI, which is not allowed.  Instead:
> 
> Add a new node_start field to the hivex_visitor struct.  (Obviously
> call it something different, like node_start2 or whatever, and it has
> to be added to the end of the current struct).
> 
> In the code you can differentiate between whether the caller supplied
> the old function or the new function:
> 
>  if (vtor->node_start2) {
>     // call new vtor->node_start2 with extra argument
>  } else if (vtor->node_start) {
>     // call old vtor->node_start
>  }

Thank you, I'll adjust my changes to use differently-named functions.  Is it acceptable style to you to have these become Java-like wrapper functions, e.g.:

static int
node_start_with_time (hive_h *h, void *writer_v, hive_node_h node, const char *name, const char *last_modified)
{
  //Body:  the node_start function as it is now, augmented with null-checking work on last_modified
}

static int
node_start (hive_h *h, void *writer_v, hive_node_h node, const char *name, const char *last_modified)
{
  return node_start_with_time (h, write_v, node, name, NULL);
}

This preserves the C ABI, and prevents code duplication.

> 
> The last_modified changes in themselves seem quite reasonable to me,
> but this patch cannot be applied as-is.

I should have patch version 2 out soon.  Thanks!

--Alex

> 
> Rich.


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