Re: [Libguestfs] [PATCH v3 1/5] New API: file-architecture
On Tue, Aug 10, 2010 at 01:41:21PM +0100, Richard W.M. Jones wrote:
> >+ r = pcre_exec (re, NULL, str, len, 0, 0, vec, 30);
>
> You can ditch vec here and pass in NULL, 0. None of the above REs
> use back references, so this shouldn't be an issue.
My reading of the pcre_exec docs suggested otherwise. However I will
need to try it when I get back from the conference.
The docs say I can pass ovector as NULL, but in that case pcre_exec
might have to malloc extra space (although only in the case where the
regexp contains back-references -- ours don't *yet* but someone might
add one in future). I think it's safer to just leave this.
However I've changed all calls to use sizeof(vec)/sizeof(vec[0])
instead of the fixed integer.
> >+ char *ret = NULL;
> >+
> >+ const char *method;
> >+ if (strstr (file, "gzip"))
> >+ method = "zcat";
> >+ else if (strstr (file, "bzip2"))
> >+ method = "bzcat";
> >+ else
> >+ method = "cat";
> >+
> >+ char dir[] = "/tmp/initrd.XXXXXX";
> >+#define dir_len 18
>
> This should be strlen(dir). The compiler will recognise this as a constant.
OK.
Actually I used 'sizeof dir' instead here.
> >+
> >+ if (is_regular_file (bin)) {
> >+ snprintf (cmd, dir_len + 256, "file %s", bin);
>
> Reusing a magic number from above in a different context. Magic
> numbers are getting a bit hard to track at this point. Could do with
> cmd_len for the size of cmd.
>
> Stepping back slightly, you could more cleanly use libmagic here
> instead of invoking file. It would be less code, and wouldn't encode
> length limits which aren't defined anywhere.
Yes, that would make a lot of sense.
The code isn't shorter, but I changed it to use libmagic anyway.
I'm going to post the whole series again, since it got longer ...
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://et.redhat.com/~rjones/libguestfs/
See what it can do: http://et.redhat.com/~rjones/libguestfs/recipes.html