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

Re: [Libguestfs] [PATCH 10/12] hivexsh: Add 'setval' and 'commit' commands.



On 03/02/10 18:35, Richard W.M. Jones wrote:
>>From c6fa2b912e47df21bc6a64b2f1b0bcf17cbd0a0f Mon Sep 17 00:00:00 2001
> From: Richard Jones <rjones redhat com>
> Date: Wed, 3 Feb 2010 18:04:31 +0000
> Subject: [PATCH 10/12] hivexsh: Add 'setval' and 'commit' commands.
> 
> This adds the 'setval' and 'commit' commands to the hivex shell.
> 
> Also adds some example scripts showing use of these.
> ---
>  hivex/Makefile.am |    5 +-
>  hivex/example1    |   40 ++++++++
>  hivex/example2    |   47 +++++++++
>  hivex/example3    |   53 +++++++++++
>  hivex/hivexsh.c   |  271 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  hivex/hivexsh.pod |   79 ++++++++++++++++
>  6 files changed, 491 insertions(+), 4 deletions(-)
>  create mode 100755 hivex/example1
>  create mode 100755 hivex/example2
>  create mode 100755 hivex/example3
> 

A few nits inline.

Matt

> diff --git a/hivex/hivexsh.c b/hivex/hivexsh.c
> index 01a5ddc..6f33f41 100644
> --- a/hivex/hivexsh.c
> +++ b/hivex/hivexsh.c
> @@ -779,3 +816,231 @@ cmd_lsval (char *key)
>    perror ("hivexsh: lsval");
>    return -1;
>  }
> +
> +static int
> +cmd_setval (char *nrvals_str)
> +{
> +  strtol_error xerr;
> +
> +  /* Parse number of values. */
> +  long nrvals;
> +  xerr = xstrtol (nrvals_str, NULL, 0, &nrvals, "");
> +  if (xerr != LONGINT_OK) {
> +    fprintf (stderr, _("%s: %s: invalid integer parameter (%s returned %d)\n"),
> +             "setval", "nrvals", "xstrtol", xerr);
> +    return -1;
> +  }
> +  if (nrvals < 0 || nrvals > 1000) {

IIRC, this 1000 was an arbitrary constant in the previous patch. Could
you replace both with a #define?

> +    fprintf (stderr, _("%s: %s: integer out of range\n"),
> +             "setval", "nrvals");
> +    return -1;
> +  }
> +
> +  struct hive_set_value *values =
> +    calloc (nrvals, sizeof (struct hive_set_value));
> +  if (values == NULL) {
> +    perror ("calloc");
> +    exit (EXIT_FAILURE);
> +  }
> +
> +  int ret = -1;
> +
> +  /* Read nrvals * 2 lines of input, nrvals * (key, value) pairs, as
> +   * explained in the man page.
> +   */
> +  int prompt = isatty (0) ? 2 : 0;

You've already got this in a global. In fact, I can't see it used
anywhere. Doesn't this generate a warning?

> +  int i, j;
> +  for (i = 0; i < nrvals; ++i) {
> +    /* Read key. */
> +    char *buf = rl_gets ("  key> ");
> +    if (!buf) {
> +      fprintf (stderr, _("hivexsh: setval: unexpected end of input\n"));
> +      quit = 1;
> +      goto error;
> +    }
> +
> +    /* Note that buf will be overwritten by the next call to rl_gets. */
> +    if (STREQ (buf, "@"))
> +      values[i].key = strdup ("");
> +    else
> +      values[i].key = strdup (buf);
> +    if (values[i].key == NULL) {
> +      perror ("strdup");
> +      exit (EXIT_FAILURE);
> +    }
> +
> +    /* Read value. */
> +    buf = rl_gets ("value> ");

Why no indentation on this prompt, as for key?

> +    if (!buf) {
> +      fprintf (stderr, _("hivexsh: setval: unexpected end of input\n"));
> +      quit = 1;
> +      goto error;
> +    }
> +
> +    if (STREQ (buf, "none")) {
> +      values[i].t = hive_t_none;
> +      values[i].len = 0;
> +    }
> +    else if (STRPREFIX (buf, "string:")) {
> +      buf += 7;
> +      values[i].t = hive_t_string;
> +      int nr_chars = strlen (buf);
> +      values[i].len = 2 * (nr_chars + 1);
> +      values[i].value = malloc (values[i].len);
> +      if (!values[i].value) {
> +        perror ("malloc");
> +        exit (EXIT_FAILURE);
> +      }
> +      for (j = 0; j <= /* sic */ nr_chars; ++j) {
> +        if (buf[j] & 0x80) {
> +          fprintf (stderr, _("hivexsh: string(utf16le): only 7 bit ASCII strings are supported for input\n"));
> +          goto error;
> +        }
> +        values[i].value[2*j] = buf[j];
> +        values[i].value[2*j+1] = '\0';

There must be a library function to do the above. Where does the 7 bit
ASCII restriction come from?

> +      }
> +    }

This doesn't look like regedit's expandstring format. What's the purpose
of it?

> +    else if (STRPREFIX (buf, "expandstring:")) {
> +      buf += 13;
> +      values[i].t = hive_t_string;
> +      int nr_chars = strlen (buf);
> +      values[i].len = 2 * (nr_chars + 1);
> +      values[i].value = malloc (values[i].len);
> +      if (!values[i].value) {
> +        perror ("malloc");
> +        exit (EXIT_FAILURE);
> +      }
> +      for (j = 0; j <= /* sic */ nr_chars; ++j) {
> +        if (buf[j] & 0x80) {
> +          fprintf (stderr, _("hivexsh: string(utf16le): only 7 bit ASCII strings are supported for input\n"));
> +          goto error;
> +        }
> +        values[i].value[2*j] = buf[j];
> +        values[i].value[2*j+1] = '\0';
> +      }
> +    }
> +    else if (STRPREFIX (buf, "dword:")) {
> +      buf += 6;
> +      values[i].t = hive_t_dword;
> +      values[i].len = 4;
> +      values[i].value = malloc (4);
> +      if (!values[i].value) {
> +        perror ("malloc");
> +        exit (EXIT_FAILURE);
> +      }
> +      long n;
> +      xerr = xstrtol (buf, NULL, 0, &n, "");

Does xstrtol support 0x notation?

> +      if (xerr != LONGINT_OK) {
> +        fprintf (stderr, _("%s: %s: invalid integer parameter (%s returned %d)\n"),
> +                 "setval", "dword", "xstrtol", xerr);
> +        goto error;
> +      }
> +      if (n < 0 || n > UINT32_MAX) {
> +        fprintf (stderr, _("%s: %s: integer out of range\n"),
> +                 "setval", "dword");
> +        goto error;
> +      }
> +      uint32_t u32 = htole32 (n);
> +      memcpy (values[i].value, &u32, 4);
> +    }
> +    else if (STRPREFIX (buf, "qword:")) {
> +      buf += 6;
> +      values[i].t = hive_t_qword;
> +      values[i].len = 8;
> +      values[i].value = malloc (8);
> +      if (!values[i].value) {
> +        perror ("malloc");
> +        exit (EXIT_FAILURE);
> +      }
> +      long long n;
> +      xerr = xstrtoll (buf, NULL, 0, &n, "");
> +      if (xerr != LONGINT_OK) {
> +        fprintf (stderr, _("%s: %s: invalid integer parameter (%s returned %d)\n"),
> +                 "setval", "dword", "xstrtoll", xerr);
> +        goto error;
> +      }
> +#if 0
> +      if (n < 0 || n > UINT64_MAX) {
> +        fprintf (stderr, _("%s: %s: integer out of range\n"),
> +                 "setval", "dword");
> +        goto error;
> +      }
> +#endif

Why have you commented this out?

> +      uint64_t u64 = htole64 (n);
> +      memcpy (values[i].value, &u64, 4);
> +    }
> +    else if (STRPREFIX (buf, "hex:")) {
> +      /* Read the type. */
> +      buf += 4;
> +      size_t len = strcspn (buf, ":");
> +      char *nextbuf;
> +      if (buf[len] == '\0')     /* "hex:t" */
> +        nextbuf = &buf[len];
> +      else {                    /* "hex:t:..." */
> +        buf[len] = '\0';
> +        nextbuf = &buf[len+1];
> +      }
> +
> +      long t;
> +      xerr = xstrtol (buf, NULL, 0, &t, "");
> +      if (xerr != LONGINT_OK) {
> +        fprintf (stderr, _("%s: %s: invalid integer parameter (%s returned %d)\n"),
> +                 "setval", "hex", "xstrtol", xerr);
> +        goto error;
> +      }
> +      if (t < 0 || t > UINT32_MAX) {
> +        fprintf (stderr, _("%s: %s: integer out of range\n"),
> +                 "setval", "hex");
> +        goto error;
> +      }
> +      values[i].t = t;
> +
> +      /* Read the hex data. */
> +      buf = nextbuf;
> +
> +      /* The allocation length is an overestimate, but it doesn't matter. */
> +      values[i].value = malloc (1 + strlen (buf) / 2);
> +      if (!values[i].value) {
> +        perror ("malloc");
> +        exit (EXIT_FAILURE);
> +      }
> +      values[i].len = 0;
> +
> +      while (*buf) {
> +        int c = 0;
> +
> +        for (j = 0; *buf && j < 2; buf++) {
> +          if (c_isxdigit (*buf)) { /* NB: ignore non-hex digits. */

The documentation defines the limiter to be a comma. I'd stick to this
strictly, making it more likely to catch typos.

> +            c <<= 4;
> +            c |= get_xdigit (*buf);
> +            j++;
> +          }
> +        }
> +
> +        if (j == 2) values[i].value[values[i].len++] = c;
> +        else if (j == 1) {
> +          fprintf (stderr, _("hivexsh: setval: trailing garbage after hex string\n"));
> +          goto error;
> +        }
> +      }
> +    }
> +    else {
> +      fprintf (stderr,
> +               _("hivexsh: setval: cannot parse value string, please refer to the man page hivexsh(1) for help: %s\n"),
> +               buf);
> +      goto error;
> +    }
> +  }
> +
> +  ret = hivex_node_set_values (h, cwd, nrvals, values, 0);
> +
> + error:
> +  /* Free values array. */
> +  for (i = 0; i < nrvals; ++i) {
> +    free (values[i].key);
> +    free (values[i].value);
> +  }
> +  free (values);
> +
> +  return ret;
> +}

-- 
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team

M:       +44 (0)7977 267231
GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490


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