On Thu, Aug 21, 2014 at 01:50:18PM +0100, Richard W.M. Jones wrote:
+ (* extract ova (tar) file *) + let cmd = sprintf ("tar -xf %s -C %s") (ova) (dir) in
Lots of extra parentheses here :-) The same command can be written more naturally without any of them: let cmd = sprintf "tar -xf %s -C %s" ova dir in However I think what you might have meant is to call the `quote' function to safely quote arguments, so that you're not adding security holes through unescaped input, ie: let cmd = sprintf "tar -xf %s -C %s" (quote ova) (quote dir) in Also I don't think uncompressing into the OVA directory is a good idea. However it works for now until we work out how and if we are able to avoid copying those large disk images unnecessarily.
+ if Sys.command (cmd) <> 0 then
You don't need to put parentheses around command line arguments: if Sys.command cmd <> 0 then In functional languages, function application is written: f a b c meaning call function `f' with 3 parameters `a', `b' and `c'. Function application binds tightest, so: f a b c + 3 means (f a b c) + 3
+ error (f_"error running command: %s") cmd + exit 1;
The error function already calls exit (see mllib/common_utils.ml). What you're actually doing here is exploiting a bug in ocaml-gettext where it prevents the compiler from correctly detecting extra parameters passed to a printf-like function. Just delete "exit 1" (but not the semicolon). Same thing several times later on too.
+ let files = Sys.readdir(dir) in
No parens needed around function parameters.
+ let mf = ref "" in + let ovf = ref "" in + (* search for the ovf file *) + Array.iter (fun file -> + let len = String.length file in + if len >= 4 && String.lowercase (String.sub file (len-4) 4) = ".ovf" then + ovf := file + else if len >= 3 && String.lowercase (String.sub file (len-3) 3) = ".mf" then + mf := file + ) files; + + (* verify sha1 from manifest file *) + let mf = dir // !mf in + let rex = Str.regexp "SHA1(\\(.*\\))= *\\(.*?\\).*$" in + let lines = read_file mf in + List.iter ( + fun line -> + if Str.string_match rex line 0 then + let file = Str.matched_group 1 line in + let sha1 = Str.matched_group 2 line in + + let cmd = sprintf "sha1sum %s" (dir // file) in
Quoting needed, so: let cmd = sprintf "sha1sum %s" (quote (dir // file)) in
+ let out = external_command ~prog cmd in + if List.exists (fun line -> string_contains line sha1) out == false then
You wouldn't normally write `== false'. This is more natural and shorter: if not (List.exists (fun line -> string_contains line sha1) out) then
+ error (f_"Checksum of %s does not match manifes sha1 %s") (file) (sha1)
Don't need parens around parameters `file' and `sha1'. (f_ ...) is a special case because it's calling a function called `f_', ie. the gettext function. Therefore the parser could not work out if you are calling: error f_ "Checksum of ..." (error + 2 parameters) unless you put in the parentheses: error (f_"Checksum of ...") (1 parameter which is the result of calling function f_).
type overlay = { - ov_overlay : string; (** Local overlay file (qcow2 format). *) + ov_overlay : string; (** Local overlgy file (qcow2 format). *)
Typo added.
+let string_contains s1 s2 =
There's a function already defined for this in mllib/common_utils.ml. `string_find'. It returns -1 if not found, or >= 0 if found.
+let read_file filename =
See mllib/common_utils.ml `read_whole_file'. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/