[libvirt] [PATCH v3 04/35] cfg.mk: single variable declaration per line when using cleanup macro

Sukrit Bhatnagar posted 35 patches 6 years, 10 months ago
There is a newer version of this series
[libvirt] [PATCH v3 04/35] cfg.mk: single variable declaration per line when using cleanup macro
Posted by Sukrit Bhatnagar 6 years, 10 months ago
Add rule to ensure that each variable declaration made using
a cleanup macro is in its own separate line.

Sometimes a variable might be initialized from a value returned
by a macro or a function, which may take on more than one
parameter, thereby introducing a comma, which might be mistaken
for multiple declarations in a line. This rule takes care of
that too.

Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
---
 cfg.mk | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/cfg.mk b/cfg.mk
index d9e90d5..7949fc8 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1077,6 +1077,14 @@ sc_require_attribute_cleanup_scalar_type_space:
     halt='there must be exactly one space between the type and asterisk inside VIR_AUTOFREE' \
       $(_sc_search_regexp)
 
+# Rule to ensure that there is only one variable declaration in
+# a line when the variable is declared using a cleanup macro.
+sc_require_attribute_cleanup_one_per_line:
+	@prohibit='VIR_AUTO(FREE|PTR)\(.+\) ([^\(]*(, .*)+|[^\(]*\(.*\)(, .*)+);' \
+    in_vc_files='\.[chx]$$' \
+    halt='there must be only one variable declaration per line when using cleanup macro' \
+      $(_sc_search_regexp)
+
 # We don't use this feature of maint.mk.
 prev_version_file = /dev/null
 
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 04/35] cfg.mk: single variable declaration per line when using cleanup macro
Posted by Erik Skultety 6 years, 10 months ago
On Sat, Jun 30, 2018 at 02:30:08PM +0530, Sukrit Bhatnagar wrote:
> Add rule to ensure that each variable declaration made using
> a cleanup macro is in its own separate line.
>
> Sometimes a variable might be initialized from a value returned
> by a macro or a function, which may take on more than one
> parameter, thereby introducing a comma, which might be mistaken
> for multiple declarations in a line. This rule takes care of
> that too.

I can't think of an example or I'm just not seeing it, can you please give me
an example where you actually need the rule below? Because right now I don't
see a need for it.

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 04/35] cfg.mk: single variable declaration per line when using cleanup macro
Posted by Sukrit Bhatnagar 6 years, 10 months ago
On Tue, 10 Jul 2018 at 16:24, Erik Skultety <eskultet@redhat.com> wrote:
>
> On Sat, Jun 30, 2018 at 02:30:08PM +0530, Sukrit Bhatnagar wrote:
> > Add rule to ensure that each variable declaration made using
> > a cleanup macro is in its own separate line.
> >
> > Sometimes a variable might be initialized from a value returned
> > by a macro or a function, which may take on more than one
> > parameter, thereby introducing a comma, which might be mistaken
> > for multiple declarations in a line. This rule takes care of
> > that too.
>
> I can't think of an example or I'm just not seeing it, can you please give me
> an example where you actually need the rule below? Because right now I don't
> see a need for it.

In src/util/virfile.c in virFileAbsPath function:
...
VIR_AUTOFREE(char *) buf = getcwd(NULL, 0);
...

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 04/35] cfg.mk: single variable declaration per line when using cleanup macro
Posted by Pavel Hrdina 6 years, 10 months ago
On Wed, Jul 11, 2018 at 12:42:43AM +0530, Sukrit Bhatnagar wrote:
> On Tue, 10 Jul 2018 at 16:24, Erik Skultety <eskultet@redhat.com> wrote:
> >
> > On Sat, Jun 30, 2018 at 02:30:08PM +0530, Sukrit Bhatnagar wrote:
> > > Add rule to ensure that each variable declaration made using
> > > a cleanup macro is in its own separate line.
> > >
> > > Sometimes a variable might be initialized from a value returned
> > > by a macro or a function, which may take on more than one
> > > parameter, thereby introducing a comma, which might be mistaken
> > > for multiple declarations in a line. This rule takes care of
> > > that too.
> >
> > I can't think of an example or I'm just not seeing it, can you please give me
> > an example where you actually need the rule below? Because right now I don't
> > see a need for it.
> 
> In src/util/virfile.c in virFileAbsPath function:
> ...
> VIR_AUTOFREE(char *) buf = getcwd(NULL, 0);
> ...

I don't see anything wrong with it, it is properly initialized to some
value, it doesn't have to be only NULL.

Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 04/35] cfg.mk: single variable declaration per line when using cleanup macro
Posted by Erik Skultety 6 years, 10 months ago
On Wed, Jul 11, 2018 at 10:35:25AM +0200, Pavel Hrdina wrote:
> On Wed, Jul 11, 2018 at 12:42:43AM +0530, Sukrit Bhatnagar wrote:
> > On Tue, 10 Jul 2018 at 16:24, Erik Skultety <eskultet@redhat.com> wrote:
> > >
> > > On Sat, Jun 30, 2018 at 02:30:08PM +0530, Sukrit Bhatnagar wrote:
> > > > Add rule to ensure that each variable declaration made using
> > > > a cleanup macro is in its own separate line.
> > > >
> > > > Sometimes a variable might be initialized from a value returned
> > > > by a macro or a function, which may take on more than one
> > > > parameter, thereby introducing a comma, which might be mistaken
> > > > for multiple declarations in a line. This rule takes care of
> > > > that too.
> > >
> > > I can't think of an example or I'm just not seeing it, can you please give me
> > > an example where you actually need the rule below? Because right now I don't
> > > see a need for it.
> >
> > In src/util/virfile.c in virFileAbsPath function:
> > ...
> > VIR_AUTOFREE(char *) buf = getcwd(NULL, 0);
> > ...
>
> I don't see anything wrong with it, it is properly initialized to some
> value, it doesn't have to be only NULL.
>
> Pavel

Agreed,

Erik


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 04/35] cfg.mk: single variable declaration per line when using cleanup macro
Posted by Sukrit Bhatnagar 6 years, 10 months ago
The problem is not that it is initialized to a non-NULL value.
If we were to detect multiple declarations in a line. we would
search for a comma (separator), right? In the case I mentioned,
the comma inside the function has to be avoided by the rule.
On Wed, 11 Jul 2018 at 15:57, Erik Skultety <eskultet@redhat.com> wrote:
>
> On Wed, Jul 11, 2018 at 10:35:25AM +0200, Pavel Hrdina wrote:
> > On Wed, Jul 11, 2018 at 12:42:43AM +0530, Sukrit Bhatnagar wrote:
> > > On Tue, 10 Jul 2018 at 16:24, Erik Skultety <eskultet@redhat.com> wrote:
> > > >
> > > > On Sat, Jun 30, 2018 at 02:30:08PM +0530, Sukrit Bhatnagar wrote:
> > > > > Add rule to ensure that each variable declaration made using
> > > > > a cleanup macro is in its own separate line.
> > > > >
> > > > > Sometimes a variable might be initialized from a value returned
> > > > > by a macro or a function, which may take on more than one
> > > > > parameter, thereby introducing a comma, which might be mistaken
> > > > > for multiple declarations in a line. This rule takes care of
> > > > > that too.
> > > >
> > > > I can't think of an example or I'm just not seeing it, can you please give me
> > > > an example where you actually need the rule below? Because right now I don't
> > > > see a need for it.
> > >
> > > In src/util/virfile.c in virFileAbsPath function:
> > > ...
> > > VIR_AUTOFREE(char *) buf = getcwd(NULL, 0);
> > > ...
> >
> > I don't see anything wrong with it, it is properly initialized to some
> > value, it doesn't have to be only NULL.
> >
> > Pavel
>
> Agreed,
>
> Erik
>
>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 04/35] cfg.mk: single variable declaration per line when using cleanup macro
Posted by Erik Skultety 6 years, 10 months ago
On Wed, Jul 11, 2018 at 08:36:59PM +0530, Sukrit Bhatnagar wrote:
> The problem is not that it is initialized to a non-NULL value.
> If we were to detect multiple declarations in a line. we would
> search for a comma (separator), right? In the case I mentioned,
> the comma inside the function has to be avoided by the rule.

Syntax-wise, our macro signatures follow the ones of a function, i.e. you also
have parentheses. If we're ever going to create a syntax-check rule for that
it won't be as simple as matching commas, you'd use more context for such a
regex, for the reasons you've mentioned. Therefore, the example you provided
would never be affected by such a rule.

Erik

> On Wed, 11 Jul 2018 at 15:57, Erik Skultety <eskultet@redhat.com> wrote:
> >
> > On Wed, Jul 11, 2018 at 10:35:25AM +0200, Pavel Hrdina wrote:
> > > On Wed, Jul 11, 2018 at 12:42:43AM +0530, Sukrit Bhatnagar wrote:
> > > > On Tue, 10 Jul 2018 at 16:24, Erik Skultety <eskultet@redhat.com> wrote:
> > > > >
> > > > > On Sat, Jun 30, 2018 at 02:30:08PM +0530, Sukrit Bhatnagar wrote:
> > > > > > Add rule to ensure that each variable declaration made using
> > > > > > a cleanup macro is in its own separate line.
> > > > > >
> > > > > > Sometimes a variable might be initialized from a value returned
> > > > > > by a macro or a function, which may take on more than one
> > > > > > parameter, thereby introducing a comma, which might be mistaken
> > > > > > for multiple declarations in a line. This rule takes care of
> > > > > > that too.
> > > > >
> > > > > I can't think of an example or I'm just not seeing it, can you please give me
> > > > > an example where you actually need the rule below? Because right now I don't
> > > > > see a need for it.
> > > >
> > > > In src/util/virfile.c in virFileAbsPath function:
> > > > ...
> > > > VIR_AUTOFREE(char *) buf = getcwd(NULL, 0);
> > > > ...
> > >
> > > I don't see anything wrong with it, it is properly initialized to some
> > > value, it doesn't have to be only NULL.
> > >
> > > Pavel
> >
> > Agreed,
> >
> > Erik
> >
> >

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