[v1] vz: build fix

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

Posted by Nikolay Shirokovskiy 7 years ago

Current script does not recognize uninitialized driver vars and
interprets next lines as if there is initialization section.

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*$/) {
         next if $1 eq "virNWFilterCallbackDriver" ||
                 $1 eq "virNWFilterTechDriver" ||
                 $1 eq "virConnectDriver";
-- 
1.8.3.1

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

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

Posted by Daniel P. Berrangé 7 years ago

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 ?

> 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+(?!.*;)/) {

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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

Posted by Nikolay Shirokovskiy 7 years ago


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.

Nikolay

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

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

Posted by Daniel P. Berrangé 7 years ago

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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

Posted by Nikolay Shirokovskiy 7 years ago


On 19.04.2018 10:49, Daniel P. Berrangé wrote:
> 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.
> 

Thanx! Pushed.

Nikolay

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