[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 05/02/10 12:54, Richard W.M. Jones wrote:
>>> +    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?
> 
> There's iconv, but that's even crazier than doing it by hand.  This is
> fine for 7 bit ASCII, but would break if you pass in UTF-8 (hence the
> check that no high bits are set).

I'll take your word for it :)

>> This doesn't look like regedit's expandstring format. What's the purpose
>> of it?
>>
>>> +    else if (STRPREFIX (buf, "expandstring:")) {
> 
> Not sure what you mean -- expandstring is a separate type in the
> hive.  We don't care about what regedit may or may not do.

Ah, I assumed they were just input formatting differences, because both
string: and expandstring: set values[i].t = hive_t_string. Is that a bug?

>>> +        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.
> 
> The documentation just says that non-hex digits are ignored, and I
> can't be bothered to code more complex parsing here.

Fair enough.

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