Libguestfs - Feb 2013 - [libhivex] Patch implementing hivex_node_get_child_deep

Hello,

First and foremost - awesome library, beautiful code. Thank you!

If I may be so bold as to make my first post to the mailing list a request
for an API change, I have attached a patch for a new function in the hivex
library that implements obtaining a handle to a "deep" node, allowing
the
user to enter a path like "SOFTWARE\Intel\Infinst\Uninstall" with only
a
previous call to load the root of, say, HKLM. When I first used libhivex, I
assumed hivex_node_get_child would take such a path, but then learned that
it only scans the children of the current node for a match.

I do realize that hivex_node_get_child has a O(n) where n is the number of
keys in the node and this means hivex_node_get_child_deep has a O(mn) where
m is the number of \-separated paths; but the extra *m overhead is always
going to be there when iterating through a registry hive.

I tried to stick to the coding conventions as close as possible, and to
keep the changes to a minimal. Please note that I was not sure where the
function prototype should be declared so as to get it exported in hivex.h,
that is something that will need to be added to my patch (please).

I apologize for not taking the time to ask whether or not such a new API
function would be welcome. As it was, I needed this code for my own
purposes, so I am sharing it here on the mailing list in case it proves to
be helpful to anyone else.

Thank you,

Mahmoud Al-Qudsi
NeoSmart Technologies
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://listman.redhat.com/archives/libguestfs/attachments/20130207/b6f783ce/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Implemented-convenience-function-hivex_node_get_chil.patch
Type: application/octet-stream
Size: 1534 bytes
Desc: not available
URL:
<http://listman.redhat.com/archives/libguestfs/attachments/20130207/b6f783ce/attachment.obj>

On Thu, Feb 07, 2013 at 12:05:39PM -0600, Mahmoud Al-Qudsi
wrote:
> If I may be so bold as to make my first post to the mailing list a request
> for an API change, I have attached a patch for a new function in the hivex
> library that implements obtaining a handle to a "deep" node,
allowing the
> user to enter a path like "SOFTWARE\Intel\Infinst\Uninstall" with
only a
> previous call to load the root of, say, HKLM. When I first used libhivex, I
> assumed hivex_node_get_child would take such a path, but then learned that
> it only scans the children of the current node for a match.
> 
> I do realize that hivex_node_get_child has a O(n) where n is the number of
> keys in the node and this means hivex_node_get_child_deep has a O(mn) where
> m is the number of \-separated paths; but the extra *m overhead is always
> going to be there when iterating through a registry hive.
> 
> I tried to stick to the coding conventions as close as possible, and to
> keep the changes to a minimal. Please note that I was not sure where the
> function prototype should be declared so as to get it exported in hivex.h,
> that is something that will need to be added to my patch (please).
> 
> I apologize for not taking the time to ask whether or not such a new API
> function would be welcome. As it was, I needed this code for my own
> purposes, so I am sharing it here on the mailing list in case it proves to
> be helpful to anyone else.
There are a number of issues with this proposed API:

(1) It's assuming that path elements are separated by \ characters,
which is true on Windows in the REGEDIT tool but not something that
hivex knows or cares about right now.

(2) The patch is incomplete: it needs to add the new API to the
generator.

(3) The patch needs tests.

(4) There is one problem with the code (see my review below).

(5) As an API it would be kind of useful, but I think a better API
    would be something where you passed a list of strings (path
    elements), eg:

      char **elements = { "foo", "bar", "baz",
NULL };
      hivex_node_get_child_deep (h, node, elements);

    There are places in the code currently where we iterate over a
    list of strings, and those would be simpler with a new API that
    worked on a list of strings.  eg. random example:

   
http://git.fedorahosted.org/cgit/virt-v2v.git/tree/lib/Sys/VirtConvert/Converter/Windows.pm#n374
> +/* Convenience function to iteratively obtain node from \-separated path
> + * Error-checking is simply passed-through
> + */
> +hive_node_h
> +hivex_node_get_child_deep (hive_h *h, hive_node_h node, const char *nname)
> +{
> +  if (!nname) return NULL;
> +
> +  int done = 0;
> +  char *pathBuffer = strdup (nname);
The return value of functions like strdup must always be checked.
> +  char *stub;
> +  char *letter;
> +
> +  for (letter = stub = pathBuffer; !done && node; ++letter)
> +  {
> +    if ((*letter == '\\') || (done = *letter == '\0'))
> +    {
> +      *letter = '\0';
> +      node = hivex_node_get_child (h, node, stub);
> +      stub = letter + 1;
> +    }
> +  }
> +
> +  free (pathBuffer);
> +  return node;
> +}
> +
>  hive_node_h
>  hivex_node_parent (hive_h *h, hive_node_h node)
>  {
> -- 
> 1.7.10.4
Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top