Re: [PATCH 1/2] check scripts: handle unintialized driver vars in check-driverimpls.pl

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Apr 19, 2018 at 09:56:07AM +0300, Nikolay Shirokovskiy wrote:
> 
> 
> On 18.04.2018 20:25, Daniel P. Berrangé wrote:
> > On Wed, Apr 18, 2018 at 05:31:12PM +0300, Nikolay Shirokovskiy wrote:
> >> Current script does not recognize uninitialized driver vars and
> >> interprets next lines as if there is initialization section.
> > 
> > IIUC, the problem is the line
> > 
> >    static virHypervisorDriver parallelsHypervisorDriver;
> > 
> > confuses it because it doesn't start a block ?
> 
> Yes.
> 
> > 
> >> Let's assume we have sane initialization syntax for drivers
> >> if it's present and check we if we have open braces.
> >> ---
> >>  src/check-driverimpls.pl | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/src/check-driverimpls.pl b/src/check-driverimpls.pl
> >> index 4c28f20..63da2e1 100755
> >> --- a/src/check-driverimpls.pl
> >> +++ b/src/check-driverimpls.pl
> >> @@ -67,7 +67,7 @@ while (<>) {
> >>                  $status = 1;
> >>              }
> >>          }
> >> -    } elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+/) {
> >> +    } elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+(?:\w+)\s*=\s*{\s*$/) {
> > 
> > I'd rather we did a blacklist of the ";" character, so we
> > still detect it if there's other unusal formatting used
> > eg use a negative assertion lookahead like
> > 
> >     } elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+(?!.*;)/) {
> > 
> 
> Agree. With your patch in case of misformatting ; we can break and fix
> formatting. With my patch in case of misformatting { we can skip the check
> which is worse.
> 
> I'll push series with your patch if you don't mind.

Sure that's fine.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux