[libvirt] [PATCH 0/6] syntax-check: Introduce a new rule to check misaligned stuff and fix it (batch I)

Shi Lei posted 6 patches 5 years, 7 months ago
Failed in applying to current master (apply log)
build-aux/check-spacing.pl       | 476 ++++++++++++++++++++-----------
src/util/vircgroup.c             |  34 +--
src/util/virconf.c               |   4 +-
src/util/virdbus.c               |   2 +-
src/util/virdnsmasq.c            |   5 +-
src/util/virerror.c              |   2 +-
src/util/virevent.c              |  14 +-
src/util/vireventpoll.c          |   8 +-
src/util/virfile.c               |   6 +-
src/util/virgic.c                |   2 +-
src/util/virhook.c               |   2 +-
src/util/virhostdev.c            |  23 +-
src/util/viridentity.c           |  18 +-
src/util/viriscsi.c              |  22 +-
src/util/virjson.c               |   2 +-
src/util/virkeycode.c            |  22 +-
src/util/virkeyfile.c            |   6 +-
src/util/virlockspace.c          |   2 +-
src/util/virlog.c                |  18 +-
src/util/virnetdev.c             |   4 +-
src/util/virnetdevbridge.c       |   2 +-
src/util/virnetdevip.c           |  11 +-
src/util/virnetdevmacvlan.c      |  30 +-
src/util/virnetdevopenvswitch.c  |  34 +--
src/util/virnetdevvportprofile.c |  24 +-
src/util/virnetlink.c            |  10 +-
src/util/virobject.c             |   2 +-
src/util/virpci.c                |   4 +-
src/util/virperf.c               |   4 +-
src/util/virprocess.c            |   2 +-
src/util/virqemu.c               |   4 +-
src/util/virresctrl.c            |   7 +-
src/util/virsocketaddr.c         |   2 +-
src/util/virstorageencryption.c  |   2 +-
src/util/virstoragefile.c        |  60 ++--
src/util/virsysinfo.c            |   2 +-
src/util/viruri.c                |   2 +-
src/util/virutil.c               |   2 +-
src/util/virxml.c                |   2 +-
39 files changed, 512 insertions(+), 366 deletions(-)
[libvirt] [PATCH 0/6] syntax-check: Introduce a new rule to check misaligned stuff and fix it (batch I)
Posted by Shi Lei 5 years, 7 months ago
This series check misaligned stuff in parenthesis:
1. For misaligned arguments of function
2. For misaligned conditions of [if|while|switch|...]

It adds a temporary filter for this new rule since there're
too much misalignment.
Now it just fix misalignment in src/util and it's the batch I.
Next step, we need modify this path filter to finish all changes.

Also, it simplifies the check-spacing.pl by adding wrapper functions
before introducing the new rule.

Thanks,

Shi Lei

Shi Lei (6):
  util: Fix misaligned arguments and misaligned conditions for
    [if|while|...]
  build-aux:check-spacing: Add wrapper function of CheckFunctionBody
  build-aux:check-spacing: Add wrapper function of KillComments
  build-aux:check-spacing: Add wrapper function of CheckWhiteSpaces
  build-aux:check-spacing: Add wrapper function of CheckCurlyBrackets
  build-aux:check-spacing: Introduce a new rule to check misaligned
    stuff in parenthesises

 build-aux/check-spacing.pl       | 476 ++++++++++++++++++++-----------
 src/util/vircgroup.c             |  34 +--
 src/util/virconf.c               |   4 +-
 src/util/virdbus.c               |   2 +-
 src/util/virdnsmasq.c            |   5 +-
 src/util/virerror.c              |   2 +-
 src/util/virevent.c              |  14 +-
 src/util/vireventpoll.c          |   8 +-
 src/util/virfile.c               |   6 +-
 src/util/virgic.c                |   2 +-
 src/util/virhook.c               |   2 +-
 src/util/virhostdev.c            |  23 +-
 src/util/viridentity.c           |  18 +-
 src/util/viriscsi.c              |  22 +-
 src/util/virjson.c               |   2 +-
 src/util/virkeycode.c            |  22 +-
 src/util/virkeyfile.c            |   6 +-
 src/util/virlockspace.c          |   2 +-
 src/util/virlog.c                |  18 +-
 src/util/virnetdev.c             |   4 +-
 src/util/virnetdevbridge.c       |   2 +-
 src/util/virnetdevip.c           |  11 +-
 src/util/virnetdevmacvlan.c      |  30 +-
 src/util/virnetdevopenvswitch.c  |  34 +--
 src/util/virnetdevvportprofile.c |  24 +-
 src/util/virnetlink.c            |  10 +-
 src/util/virobject.c             |   2 +-
 src/util/virpci.c                |   4 +-
 src/util/virperf.c               |   4 +-
 src/util/virprocess.c            |   2 +-
 src/util/virqemu.c               |   4 +-
 src/util/virresctrl.c            |   7 +-
 src/util/virsocketaddr.c         |   2 +-
 src/util/virstorageencryption.c  |   2 +-
 src/util/virstoragefile.c        |  60 ++--
 src/util/virsysinfo.c            |   2 +-
 src/util/viruri.c                |   2 +-
 src/util/virutil.c               |   2 +-
 src/util/virxml.c                |   2 +-
 39 files changed, 512 insertions(+), 366 deletions(-)

-- 
2.17.1


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6] syntax-check: Introduce a new rule to check misaligned stuff and fix it (batch I)
Posted by Michal Privoznik 5 years, 6 months ago
On 09/19/2018 10:38 AM, Shi Lei wrote:
> This series check misaligned stuff in parenthesis:
> 1. For misaligned arguments of function
> 2. For misaligned conditions of [if|while|switch|...]
> 
> It adds a temporary filter for this new rule since there're
> too much misalignment.
> Now it just fix misalignment in src/util and it's the batch I.
> Next step, we need modify this path filter to finish all changes.
> 
> Also, it simplifies the check-spacing.pl by adding wrapper functions
> before introducing the new rule.
> 
> Thanks,
> 
> Shi Lei
> 
> Shi Lei (6):
>   util: Fix misaligned arguments and misaligned conditions for
>     [if|while|...]
>   build-aux:check-spacing: Add wrapper function of CheckFunctionBody
>   build-aux:check-spacing: Add wrapper function of KillComments
>   build-aux:check-spacing: Add wrapper function of CheckWhiteSpaces
>   build-aux:check-spacing: Add wrapper function of CheckCurlyBrackets
>   build-aux:check-spacing: Introduce a new rule to check misaligned
>     stuff in parenthesises
> 
>  build-aux/check-spacing.pl       | 476 ++++++++++++++++++++-----------
>  src/util/vircgroup.c             |  34 +--
>  src/util/virconf.c               |   4 +-
>  src/util/virdbus.c               |   2 +-
>  src/util/virdnsmasq.c            |   5 +-
>  src/util/virerror.c              |   2 +-
>  src/util/virevent.c              |  14 +-
>  src/util/vireventpoll.c          |   8 +-
>  src/util/virfile.c               |   6 +-
>  src/util/virgic.c                |   2 +-
>  src/util/virhook.c               |   2 +-
>  src/util/virhostdev.c            |  23 +-
>  src/util/viridentity.c           |  18 +-
>  src/util/viriscsi.c              |  22 +-
>  src/util/virjson.c               |   2 +-
>  src/util/virkeycode.c            |  22 +-
>  src/util/virkeyfile.c            |   6 +-
>  src/util/virlockspace.c          |   2 +-
>  src/util/virlog.c                |  18 +-
>  src/util/virnetdev.c             |   4 +-
>  src/util/virnetdevbridge.c       |   2 +-
>  src/util/virnetdevip.c           |  11 +-
>  src/util/virnetdevmacvlan.c      |  30 +-
>  src/util/virnetdevopenvswitch.c  |  34 +--
>  src/util/virnetdevvportprofile.c |  24 +-
>  src/util/virnetlink.c            |  10 +-
>  src/util/virobject.c             |   2 +-
>  src/util/virpci.c                |   4 +-
>  src/util/virperf.c               |   4 +-
>  src/util/virprocess.c            |   2 +-
>  src/util/virqemu.c               |   4 +-
>  src/util/virresctrl.c            |   7 +-
>  src/util/virsocketaddr.c         |   2 +-
>  src/util/virstorageencryption.c  |   2 +-
>  src/util/virstoragefile.c        |  60 ++--
>  src/util/virsysinfo.c            |   2 +-
>  src/util/viruri.c                |   2 +-
>  src/util/virutil.c               |   2 +-
>  src/util/virxml.c                |   2 +-
>  39 files changed, 512 insertions(+), 366 deletions(-)
> 

Yay! ACKed and pushed.

We definitely want more of these.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6] syntax-check: Introduce a new rule to check misaligned stuff and fix it (batch I)
Posted by Ján Tomko 5 years, 6 months ago
On Mon, Sep 24, 2018 at 10:15:22AM +0200, Michal Privoznik wrote:
>On 09/19/2018 10:38 AM, Shi Lei wrote:
>> This series check misaligned stuff in parenthesis:
>> 1. For misaligned arguments of function
>> 2. For misaligned conditions of [if|while|switch|...]
>>
>> It adds a temporary filter for this new rule since there're
>> too much misalignment.
>> Now it just fix misalignment in src/util and it's the batch I.
>> Next step, we need modify this path filter to finish all changes.
>>
>> Also, it simplifies the check-spacing.pl by adding wrapper functions
>> before introducing the new rule.
>>
>> Thanks,
>>
>> Shi Lei
>>
>> Shi Lei (6):
>>   util: Fix misaligned arguments and misaligned conditions for
>>     [if|while|...]
>>   build-aux:check-spacing: Add wrapper function of CheckFunctionBody
>>   build-aux:check-spacing: Add wrapper function of KillComments
>>   build-aux:check-spacing: Add wrapper function of CheckWhiteSpaces
>>   build-aux:check-spacing: Add wrapper function of CheckCurlyBrackets
>>   build-aux:check-spacing: Introduce a new rule to check misaligned
>>     stuff in parenthesises
>>
>
>Yay! ACKed and pushed.
>
>We definitely want more of these.
>

Please no.

Despite my comments on the first spacing rule [0], this series slows
down the spacing check by 100 % (from 1.6s to 3.1s in my case).

For such slowdown I'd expect more than just catching a few corner cases.
Also note that a significant part of the slowdown is introduced by the
wrapper functions.

Jano

>Michal
>

[0] https://www.redhat.com/archives/libvir-list/2018-September/msg00537.html
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6] syntax-check: Introduce a new rule to check misaligned stuff and fix it (batch I)
Posted by Michal Privoznik 5 years, 6 months ago
On 09/24/2018 03:14 PM, Ján Tomko wrote:
> On Mon, Sep 24, 2018 at 10:15:22AM +0200, Michal Privoznik wrote:
>> On 09/19/2018 10:38 AM, Shi Lei wrote:
>>> This series check misaligned stuff in parenthesis:
>>> 1. For misaligned arguments of function
>>> 2. For misaligned conditions of [if|while|switch|...]
>>>
>>> It adds a temporary filter for this new rule since there're
>>> too much misalignment.
>>> Now it just fix misalignment in src/util and it's the batch I.
>>> Next step, we need modify this path filter to finish all changes.
>>>
>>> Also, it simplifies the check-spacing.pl by adding wrapper functions
>>> before introducing the new rule.
>>>
>>> Thanks,
>>>
>>> Shi Lei
>>>
>>> Shi Lei (6):
>>>   util: Fix misaligned arguments and misaligned conditions for
>>>     [if|while|...]
>>>   build-aux:check-spacing: Add wrapper function of CheckFunctionBody
>>>   build-aux:check-spacing: Add wrapper function of KillComments
>>>   build-aux:check-spacing: Add wrapper function of CheckWhiteSpaces
>>>   build-aux:check-spacing: Add wrapper function of CheckCurlyBrackets
>>>   build-aux:check-spacing: Introduce a new rule to check misaligned
>>>     stuff in parenthesises
>>>
>>
>> Yay! ACKed and pushed.
>>
>> We definitely want more of these.
>>
> 
> Please no.
> 
> Despite my comments on the first spacing rule [0], this series slows
> down the spacing check by 100 % (from 1.6s to 3.1s in my case).

I don't think that syntax-check run time is our goal. I think it's the
code cleanliness.

> 
> For such slowdown I'd expect more than just catching a few corner cases.
> Also note that a significant part of the slowdown is introduced by the
> wrapper functions.

Well, apparently we as developers and reviewers are not doing a good job
in formatting the code right. Having a script that checks for that is a
necessity then.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6] syntax-check: Introduce a new rule to check misaligned stuff and fix it (batch I)
Posted by Ján Tomko 5 years, 6 months ago
On Mon, Sep 24, 2018 at 03:30:02PM +0200, Michal Privoznik wrote:
>On 09/24/2018 03:14 PM, Ján Tomko wrote:
>> Please no.
>>
>> Despite my comments on the first spacing rule [0], this series slows
>> down the spacing check by 100 % (from 1.6s to 3.1s in my case).
>
>I don't think that syntax-check run time is our goal.

Well that's just sad.

Jano

>I think it's the
>code cleanliness.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6] syntax-check: Introduce a new rule to check misaligned stuff and fix it (batch I)
Posted by Michal Privoznik 5 years, 6 months ago
On 09/25/2018 02:03 PM, Ján Tomko wrote:
> On Mon, Sep 24, 2018 at 03:30:02PM +0200, Michal Privoznik wrote:
>> On 09/24/2018 03:14 PM, Ján Tomko wrote:
>>> Please no.
>>>
>>> Despite my comments on the first spacing rule [0], this series slows
>>> down the spacing check by 100 % (from 1.6s to 3.1s in my case).
>>
>> I don't think that syntax-check run time is our goal.
> 
> Well that's just sad.

Why is that? I spend more time trying to figure our what the code does
than running syntax-check over it. Therefore in my view code cleanliness
is more important. But if you provide some argument otherwise we can
certainly discuss it.

Michal

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