Re: [Xen-devel] [PATCH OSSTEST v1 5/5] mg-show-flight-runvars: recurse on buildjobs upon request

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("[PATCH OSSTEST v1 5/5] mg-show-flight-runvars: recurse on 
buildjobs upon request"):
> By looping over @rows looking for buildjobs runvars and adding those
> jobs to the output until nothing changes.
...
> Note that synth runvars (if requests) are now sorted in with the
> regular ones, whereas previously they were listed last. Retaining the
> old behaviour would be too complex I feel.

...
> -sub collect ($) {
> -    my ($flight) = @_;
> +    $jobcond //= "TRUE";
...
>      my $q = $dbh_tests->prepare
>       ("SELECT synth, ".(join ',', @cols)." $qfrom ORDER BY synth, name, 
> job");

You probably want to drop the `synth' from the ORDER here, even if you
take my suggestion below.  The sort is still a good idea here because
it ensures that the output is deterministic.

> +    my ($oflight, $ojob) = flight_otherjob($tflight, $row->[2]);
> +    collect($oflight, "job = '$ojob'");

SQL injection vulnerability, I think.  (There are lots of places
where jobs are treated this way, but they come from the command line
or similar, or have been checked against some regexp.)

I think you should use the $jobcond @jobcondparams pattern.


> -foreach my $row (@rows) {
> +# Sort by runvar name, then (flight+)job.
> +foreach my $row (sort { $a->[1] cmp $b->[1]//$a->[0] cmp $b->[1] } @rows) {

If you retain it this way, put spaces round your //.

But maybe you should use a Schwartzian transform instead.

   my $xform = sub { [ ($->[1] =~ m/\~$/)." $_->[1] $_->[0]", $_ ]; };
   foreach my $row (map { $_->[1] } sort { $xform->($a) cmp
       $xform->($b) etc.

This makes the synth sorting easy too.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel