[v3] use GNU C's cleanup attribute in src/util (batch I)

[libvirt] [PATCH v3 02/35] cfg.mk: variable initialization when declared with cleanup macro

Posted by Sukrit Bhatnagar 6 years, 10 months ago

A variable, which is never assigned a value in the function, might get
passed into the cleanup function which may or may not raise any errors.

To maintain the correct usage, the variable must be initialized, either
with a value or with NULL. This syntax-check rule takes care of that.

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

diff --git a/cfg.mk b/cfg.mk
index 6bebd0a..196d1b2 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1057,6 +1057,17 @@ sc_prohibit_backslash_alignment:
 	halt='Do not attempt to right-align backslashes' \
 	  $(_sc_search_regexp)
 
+# Some syntax rules pertaining to the usage of cleanup macros
+# implementing GNU C's cleanup attribute
+
+# Rule to ensure that varibales declared using a cleanup macro are
+# always initialized.
+sc_require_attribute_cleanup_initialization:
+	@prohibit='VIR_AUTO(FREE|PTR)\(.+\) [^=]+;' \
+	in_vc_files='\.[chx]$$' \
+	halt='variable declared with a cleanup macro must be initialized' \
+	  $(_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 02/35] cfg.mk: variable initialization when declared with cleanup macro

Posted by Erik Skultety 6 years, 10 months ago

On Sat, Jun 30, 2018 at 02:30:06PM +0530, Sukrit Bhatnagar wrote:
> A variable, which is never assigned a value in the function, might get
> passed into the cleanup function which may or may not raise any errors.
>
> To maintain the correct usage, the variable must be initialized, either
> with a value or with NULL. This syntax-check rule takes care of that.
>
> Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
> ---
>  cfg.mk | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/cfg.mk b/cfg.mk
> index 6bebd0a..196d1b2 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -1057,6 +1057,17 @@ sc_prohibit_backslash_alignment:
>  	halt='Do not attempt to right-align backslashes' \
>  	  $(_sc_search_regexp)
>
> +# Some syntax rules pertaining to the usage of cleanup macros
> +# implementing GNU C's cleanup attribute
> +
> +# Rule to ensure that varibales declared using a cleanup macro are
> +# always initialized.
> +sc_require_attribute_cleanup_initialization:
> +	@prohibit='VIR_AUTO(FREE|PTR)\(.+\) [^=]+;' \

the following will be caught by spacing check:
VIR_AUTOFREE (type) var;

this will not be caught by any of the macros:
VIR_AUTOFREE(type)var; OR VIR_AUTOFREE(type)<tab>var;

So with a slight modification you'll get:
VIR_AUTO(FREE|PTR)\(.+\) *[^=]+;

The example with the <tab> above makes me wonder why do we have a syntax check
that forbids tabs for indent, but not for a separator, we don't or at least
should not use tabs in any \.[ch] sources.

With the modification above:
Reviewed-by: Erik Skultety <eskultet@redhat.com>

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