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

[Ian Campbell <ian.campbell@citrix.com>]

On Mon, 2016-02-01 at 15:19 +0000, Ian Jackson wrote:
> 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.

OK

> > +    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.

Indeed. Fixed.

> -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.

ITIYM:

   foreach my $row (map { $_->[1] }
                    sort { $a->[0] cmp $b->[0] }
                    map { $xform->($_) }
                    @rows) {
        ...
   }

Since doing the xform-> in the sort defeats the purpose (or I don't
properly understand the Schwartzian transform)

> This makes the synth sorting easy too.

Right.



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