4 a.m.
This series of patches first introduces a new set of macros which help
in using GNU C's __attribute__((cleanup)) in the code.
Then a few syntax-check rules are added which help in ensuring correct
usage of the newly introduced cleanup macros.
Then the patches modify a few files in src/util to use VIR_AUTOFREE
and VIR_AUTOPTR for automatic freeing of memory and get rid of some
VIR_FREE macro invocations and *Free function calls.
Sukrit Bhatnagar (35):
util: alloc: add macros for implementing automatic cleanup
functionality
cfg.mk: variable initialization when declared with cleanup macro
cfg.mk: correct spacing between type and asterisk in VIR_AUTOFREE
cfg.mk: single variable declaration per line when using cleanup macro
cfg.mk: no trailing semicolon at line invoking VIR_DEFINE_* cleanup
macro
util: string: introduce typedef for string
util: string: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
util: command: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
util: command: remove redundant include directive
util: command: use VIR_AUTOFREE instead of VIR_FREE for scalar types
util: command: use VIR_AUTOPTR for aggregate types
util: file: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
util: file: remove redundant include directive
util: file: use VIR_AUTOFREE instead of VIR_FREE for scalar types
util: file: use VIR_AUTOPTR for aggregate types
util: authconfig: define cleanup function using
VIR_DEFINE_AUTOPTR_FUNC
util: authconfig: remove redundant include directive
util: authconfig: use VIR_AUTOFREE instead of VIR_FREE for scalar
types
util: auth: remove redundant include directive
util: auth: use VIR_AUTOFREE instead of VIR_FREE for scalar types
util: auth: use VIR_AUTOPTR for aggregate types
util: json: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
util: json: remove redundant include directive
util: json: use VIR_AUTOFREE instead of VIR_FREE for scalar types
util: json: use VIR_AUTOPTR for aggregate types
util: bitmap: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
util: bitmap: remove redundant include directive
util: bitmap: use VIR_AUTOPTR for aggregate types
util: iohelper: use VIR_AUTOFREE instead of VIR_FREE for scalar types
util: arptable: use VIR_AUTOFREE instead of VIR_FREE for scalar types
util: audit: use VIR_AUTOFREE instead of VIR_FREE for scalar types
util: fcp: use VIR_AUTOFREE instead of VIR_FREE for scalar types
util: eventpoll: use VIR_AUTOFREE instead of VIR_FREE for scalar types
util: filecache: use VIR_AUTOFREE instead of VIR_FREE for scalar types
util: identity: use VIR_AUTOFREE instead of VIR_FREE for scalar types
cfg.mk | 37 ++++++
src/util/iohelper.c | 4 +-
src/util/viralloc.h | 44 +++++++
src/util/virarptable.c | 14 +-
src/util/viraudit.c | 3 +-
src/util/virauth.c | 61 +++------
src/util/virauthconfig.c | 35 ++---
src/util/virauthconfig.h | 3 +
src/util/virbitmap.c | 8 +-
src/util/virbitmap.h | 3 +
src/util/vircommand.c | 47 ++-----
src/util/vircommand.h | 2 +
src/util/vireventpoll.c | 7 +-
src/util/virfcp.c | 20 +--
src/util/virfile.c | 327 ++++++++++++++++-------------------------------
src/util/virfile.h | 3 +
src/util/virfilecache.c | 35 ++---
src/util/viridentity.c | 52 ++++----
src/util/virjson.c | 68 +++-------
src/util/virjson.h | 3 +
src/util/virstring.h | 5 +
21 files changed, 319 insertions(+), 462 deletions(-)
--
1.8.3.1
4 a.m.
New subject: [libvirt] [PATCH v3 01/35] util: alloc: add macros for implementing automatic cleanup functionality
New macros are introduced which help in adding GNU C's cleanup
attribute to variable declarations. Variables declared with these
macros will have their allocated memory freed automatically when
they go out of scope.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/viralloc.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 69d0f90..5c1d0d5 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -596,4 +596,48 @@ void virAllocTestInit(void);
int virAllocTestCount(void);
void virAllocTestOOM(int n, int m);
void virAllocTestHook(void (*func)(int, void*), void *data);
+
+# define VIR_AUTOPTR_TYPE_NAME(type) type##AutoPtr
+# define VIR_AUTOPTR_FUNC_NAME(type) type##AutoPtrFree
+
+/**
+ * VIR_DEFINE_AUTOPTR_FUNC:
+ * @type: type of the variable to be freed automatically
+ * @func: cleanup function to be automatically called
+ *
+ * This macro defines a function for automatic freeing of
+ * resources allocated to a variable of type @type. This newly
+ * defined function works as a necessary wrapper around @func.
+ */
+# define VIR_DEFINE_AUTOPTR_FUNC(type, func) \
+ typedef type *VIR_AUTOPTR_TYPE_NAME(type); \
+ static inline void VIR_AUTOPTR_FUNC_NAME(type)(type **_ptr) \
+ { \
+ if (*_ptr) \
+ (func)(*_ptr); \
+ *_ptr = NULL; \
+ } \
+
+/**
+ * VIR_AUTOFREE:
+ * @type: type of the variable to be freed automatically
+ *
+ * Macro to automatically free the memory allocated to
+ * the variable declared with it by calling virFree
+ * when the variable goes out of scope.
+ */
+# define VIR_AUTOFREE(type) __attribute__((cleanup(virFree))) type
+
+/**
+ * VIR_AUTOPTR:
+ * @type: type of the variable to be freed automatically
+ *
+ * Macro to automatically free the memory allocated to
+ * the variable declared with it by calling the function
+ * defined by VIR_DEFINE_AUTOPTR_FUNC when the variable
+ * goes out of scope.
+ */
+# define VIR_AUTOPTR(type) \
+ __attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type)))) VIR_AUTOPTR_TYPE_NAME(type)
+
#endif /* __VIR_MEMORY_H_ */
--
1.8.3.1
11:39 a.m.
New subject: [libvirt] [PATCH v3 01/35] util: alloc: add macros for implementing automatic cleanup functionality
On Sat, Jun 30, 2018 at 02:30:05PM +0530, Sukrit Bhatnagar wrote:
New macros are introduced which help in adding GNU C's cleanup
attribute to variable declarations. Variables declared with these
macros will have their allocated memory freed automatically when
they go out of scope.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/viralloc.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 69d0f90..5c1d0d5 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -596,4 +596,48 @@ void virAllocTestInit(void);
int virAllocTestCount(void);
void virAllocTestOOM(int n, int m);
void virAllocTestHook(void (*func)(int, void*), void *data);
+
+# define VIR_AUTOPTR_TYPE_NAME(type) type##AutoPtr
+# define VIR_AUTOPTR_FUNC_NAME(type) type##AutoPtrFree
+
+/**
+ * VIR_DEFINE_AUTOPTR_FUNC:
+ * @type: type of the variable to be freed automatically
+ * @func: cleanup function to be automatically called
+ *
+ * This macro defines a function for automatic freeing of
+ * resources allocated to a variable of type @type. This newly
+ * defined function works as a necessary wrapper around @func.
+ */
+# define VIR_DEFINE_AUTOPTR_FUNC(type, func) \
+ typedef type *VIR_AUTOPTR_TYPE_NAME(type); \
So, it's not visible at first glance how ^this typedef is used...
+ static inline void VIR_AUTOPTR_FUNC_NAME(type)(type **_ptr) \
+ { \
+ if (*_ptr) \
+ (func)(*_ptr); \
+ *_ptr = NULL; \
+ } \
...therefore I'd write it explicitly as
VIR_AUTOPTR_FUNC_NAME(type)(VIR_AUTOPTR_TYPE_NAME(type) *_ptr)
+
+# define VIR_AUTOPTR(type) \
+ __attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type)))) VIR_AUTOPTR_TYPE_NAME(type)
+
Also, since we're going to use it like this:
VIR_AUTOPTR(virDomainDef) foo
instead of this:
VIR_AUTOPTR(virDomainDefPtr) foo
why do we need VIR_AUTOPTR_TYPE_NAME anyway, since you could just use
"type *" in the VIR_AUTOPTR macro definition and that should work for external
types as well.
Erik
1:14 p.m.
New subject: [libvirt] [PATCH v3 01/35] util: alloc: add macros for implementing automatic cleanup functionality
On Thu, 12 Jul 2018 at 22:09, Erik Skultety <eskultet(a)redhat.com> wrote:
On Sat, Jun 30, 2018 at 02:30:05PM +0530, Sukrit Bhatnagar wrote:
> New macros are introduced which help in adding GNU C's cleanup
> attribute to variable declarations. Variables declared with these
> macros will have their allocated memory freed automatically when
> they go out of scope.
>
> Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
> ---
> src/util/viralloc.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/src/util/viralloc.h b/src/util/viralloc.h
> index 69d0f90..5c1d0d5 100644
> --- a/src/util/viralloc.h
> +++ b/src/util/viralloc.h
> @@ -596,4 +596,48 @@ void virAllocTestInit(void);
> int virAllocTestCount(void);
> void virAllocTestOOM(int n, int m);
> void virAllocTestHook(void (*func)(int, void*), void *data);
> +
> +# define VIR_AUTOPTR_TYPE_NAME(type) type##AutoPtr
> +# define VIR_AUTOPTR_FUNC_NAME(type) type##AutoPtrFree
> +
> +/**
> + * VIR_DEFINE_AUTOPTR_FUNC:
> + * @type: type of the variable to be freed automatically
> + * @func: cleanup function to be automatically called
> + *
> + * This macro defines a function for automatic freeing of
> + * resources allocated to a variable of type @type. This newly
> + * defined function works as a necessary wrapper around @func.
> + */
> +# define VIR_DEFINE_AUTOPTR_FUNC(type, func) \
> + typedef type *VIR_AUTOPTR_TYPE_NAME(type); \
So, it's not visible at first glance how ^this typedef is used...
> + static inline void VIR_AUTOPTR_FUNC_NAME(type)(type **_ptr) \
> + { \
> + if (*_ptr) \
> + (func)(*_ptr); \
> + *_ptr = NULL; \
> + } \
...therefore I'd write it explicitly as
VIR_AUTOPTR_FUNC_NAME(type)(VIR_AUTOPTR_TYPE_NAME(type) *_ptr)
> +
> +# define VIR_AUTOPTR(type) \
> + __attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type))))
VIR_AUTOPTR_TYPE_NAME(type)
> +
Also, since we're going to use it like this:
VIR_AUTOPTR(virDomainDef) foo
instead of this:
VIR_AUTOPTR(virDomainDefPtr) foo
why do we need VIR_AUTOPTR_TYPE_NAME anyway, since you could just use
"type *" in the VIR_AUTOPTR macro definition and that should work for external
types as well.
I agree. We don't really need it. Here is how the code will look after
the changes
you suggested:
# define VIR_AUTOPTR_FUNC_NAME(type) type##AutoPtrFree
# define VIR_DEFINE_AUTOPTR_FUNC(type, func) \
static inline void VIR_AUTOPTR_FUNC_NAME(type)(type **_ptr) \
{ \
if (*_ptr) \
(func)(*_ptr); \
*_ptr = NULL; \
} \
# define VIR_AUTOFREE(type) __attribute__((cleanup(virFree))) type
# define VIR_AUTOPTR(type) \
__attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type)))) type *
Shall I proceed to send the first series of patches?
2:14 a.m.
New subject: [libvirt] [PATCH v3 01/35] util: alloc: add macros for implementing automatic cleanup functionality
On Thu, Jul 12, 2018 at 11:44:16PM +0530, Sukrit Bhatnagar wrote:
On Thu, 12 Jul 2018 at 22:09, Erik Skultety
<eskultet(a)redhat.com> wrote:
>
> On Sat, Jun 30, 2018 at 02:30:05PM +0530, Sukrit Bhatnagar wrote:
> > New macros are introduced which help in adding GNU C's cleanup
> > attribute to variable declarations. Variables declared with these
> > macros will have their allocated memory freed automatically when
> > they go out of scope.
> >
> > Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
> > ---
> > src/util/viralloc.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 44 insertions(+)
> >
> > diff --git a/src/util/viralloc.h b/src/util/viralloc.h
> > index 69d0f90..5c1d0d5 100644
> > --- a/src/util/viralloc.h
> > +++ b/src/util/viralloc.h
> > @@ -596,4 +596,48 @@ void virAllocTestInit(void);
> > int virAllocTestCount(void);
> > void virAllocTestOOM(int n, int m);
> > void virAllocTestHook(void (*func)(int, void*), void *data);
> > +
> > +# define VIR_AUTOPTR_TYPE_NAME(type) type##AutoPtr
> > +# define VIR_AUTOPTR_FUNC_NAME(type) type##AutoPtrFree
> > +
> > +/**
> > + * VIR_DEFINE_AUTOPTR_FUNC:
> > + * @type: type of the variable to be freed automatically
> > + * @func: cleanup function to be automatically called
> > + *
> > + * This macro defines a function for automatic freeing of
> > + * resources allocated to a variable of type @type. This newly
> > + * defined function works as a necessary wrapper around @func.
> > + */
> > +# define VIR_DEFINE_AUTOPTR_FUNC(type, func) \
> > + typedef type *VIR_AUTOPTR_TYPE_NAME(type); \
>
> So, it's not visible at first glance how ^this typedef is used...
>
> > + static inline void VIR_AUTOPTR_FUNC_NAME(type)(type **_ptr) \
> > + { \
> > + if (*_ptr) \
> > + (func)(*_ptr); \
> > + *_ptr = NULL; \
> > + } \
>
> ...therefore I'd write it explicitly as
>
> VIR_AUTOPTR_FUNC_NAME(type)(VIR_AUTOPTR_TYPE_NAME(type) *_ptr)
>
> > +
> > +# define VIR_AUTOPTR(type) \
> > + __attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type))))
VIR_AUTOPTR_TYPE_NAME(type)
> > +
>
> Also, since we're going to use it like this:
> VIR_AUTOPTR(virDomainDef) foo
>
> instead of this:
> VIR_AUTOPTR(virDomainDefPtr) foo
>
> why do we need VIR_AUTOPTR_TYPE_NAME anyway, since you could just use
> "type *" in the VIR_AUTOPTR macro definition and that should work for
external
> types as well.
I agree. We don't really need it. Here is how the code will look after
the changes
you suggested:
# define VIR_AUTOPTR_FUNC_NAME(type) type##AutoPtrFree
# define VIR_DEFINE_AUTOPTR_FUNC(type, func) \
static inline void VIR_AUTOPTR_FUNC_NAME(type)(type **_ptr) \
{ \
if (*_ptr) \
(func)(*_ptr); \
*_ptr = NULL; \
} \
# define VIR_AUTOFREE(type) __attribute__((cleanup(virFree))) type
# define VIR_AUTOPTR(type) \
__attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type)))) type *
Shall I proceed to send the first series of patches?
Yep, please do, since there weren't any major flaws code-wise, I think it
should be very straightforward.
Erik
4 a.m.
New subject: [libvirt] [PATCH v3 02/35] cfg.mk: variable initialization when declared with cleanup macro
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(a)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
5:22 a.m.
New subject: [libvirt] [PATCH v3 02/35] cfg.mk: variable initialization when declared with cleanup macro
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(a)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(a)redhat.com>
4 a.m.
New subject: [libvirt] [PATCH v3 03/35] cfg.mk: correct spacing between type and asterisk in VIR_AUTOFREE
Add rule to ensure that there is exactly one blank space between
the type name and the asterisk when passed as the argument to the
VIR_AUTOFREE macro.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
cfg.mk | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/cfg.mk b/cfg.mk
index 196d1b2..d9e90d5 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1068,6 +1068,15 @@ sc_require_attribute_cleanup_initialization:
halt='variable declared with a cleanup macro must be initialized' \
$(_sc_search_regexp)
+# Rule to ensure that there is exactly one blank space between the
+# type name and the asterisk charcater when passed as the argument
+# to VIR_AUTOFREE.
+sc_require_attribute_cleanup_scalar_type_space:
+ @prohibit='VIR_AUTOFREE\([^ ]+(| {2,})\*\) .*;' \
+ in_vc_files='\.[chx]$$' \
+ halt='there must be exactly one space between the type and asterisk inside
VIR_AUTOFREE' \
+ $(_sc_search_regexp)
+
# We don't use this feature of maint.mk.
prev_version_file = /dev/null
--
1.8.3.1
5:30 a.m.
New subject: [libvirt] [PATCH v3 03/35] cfg.mk: correct spacing between type and asterisk in VIR_AUTOFREE
On Sat, Jun 30, 2018 at 02:30:07PM +0530, Sukrit Bhatnagar wrote:
Add rule to ensure that there is exactly one blank space between
the type name and the asterisk when passed as the argument to the
VIR_AUTOFREE macro.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
We would have to enforce this everywhere and I don't really see a point in this
kind of check, so NACK to this.
Erik
4 a.m.
New subject: [libvirt] [PATCH v3 04/35] cfg.mk: single variable declaration per line when using cleanup macro
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(a)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
5:54 a.m.
New subject: [libvirt] [PATCH v3 04/35] cfg.mk: single variable declaration per line when using cleanup macro
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
2:12 p.m.
New subject: [libvirt] [PATCH v3 04/35] cfg.mk: single variable declaration per line when using cleanup macro
On Tue, 10 Jul 2018 at 16:24, Erik Skultety <eskultet(a)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);
...
3:35 a.m.
New subject: [libvirt] [PATCH v3 04/35] cfg.mk: single variable declaration per line when using cleanup macro
On Wed, Jul 11, 2018 at 12:42:43AM +0530, Sukrit Bhatnagar wrote:
On Tue, 10 Jul 2018 at 16:24, Erik Skultety
<eskultet(a)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
5:27 a.m.
New subject: [libvirt] [PATCH v3 04/35] cfg.mk: single variable declaration per line when using cleanup macro
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(a)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
10:06 a.m.
New subject: [libvirt] [PATCH v3 04/35] cfg.mk: single variable declaration per line when using cleanup macro
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(a)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(a)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
3:07 a.m.
New subject: [libvirt] [PATCH v3 04/35] cfg.mk: single variable declaration per line when using cleanup macro
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(a)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(a)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
>
>
4 a.m.
New subject: [libvirt] [PATCH v3 05/35] cfg.mk: no trailing semicolon at line invoking VIR_DEFINE_* cleanup macro
Add rule to ensure that there is no semicolon at the end of
the line where a VIR_DEFINE_* cleanup macro is invoked.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
cfg.mk | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/cfg.mk b/cfg.mk
index 7949fc8..d292005 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1085,6 +1085,15 @@ sc_require_attribute_cleanup_one_per_line:
halt='there must be only one variable declaration per line when using cleanup
macro' \
$(_sc_search_regexp)
+# Rule to ensure that there is no trailing semicolon at the line on
+# which a cleanup funciton is defined using a VIR_DEFINE_* macro.
+sc_require_attribute_cleanup_no_semicolon:
+ @prohibit='VIR_DEFINE_AUTOPTR_FUNC\(.+\)\s*;' \
+ in_vc_files='\.[chx]$$' \
+ halt='cleanup macro defining a function must not end with a semicolon' \
+ $(_sc_search_regexp)
+
+
# We don't use this feature of maint.mk.
prev_version_file = /dev/null
--
1.8.3.1
6 a.m.
New subject: [libvirt] [PATCH v3 05/35] cfg.mk: no trailing semicolon at line invoking VIR_DEFINE_* cleanup macro
On Sat, Jun 30, 2018 at 02:30:09PM +0530, Sukrit Bhatnagar wrote:
Add rule to ensure that there is no semicolon at the end of
the line where a VIR_DEFINE_* cleanup macro is invoked.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
NACK, the compiler will fail in such case, so the syntax-rule doesn't bring any
benefit to us.
Erik
2:28 p.m.
New subject: [libvirt] [PATCH v3 05/35] cfg.mk: no trailing semicolon at line invoking VIR_DEFINE_* cleanup macro
On Tue, 10 Jul 2018 at 16:30, Erik Skultety <eskultet(a)redhat.com> wrote:
On Sat, Jun 30, 2018 at 02:30:09PM +0530, Sukrit Bhatnagar wrote:
> Add rule to ensure that there is no semicolon at the end of
> the line where a VIR_DEFINE_* cleanup macro is invoked.
>
> Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
> ---
NACK, the compiler will fail in such case, so the syntax-rule doesn't bring any
benefit to us.
The make check does not raise an error when a semicolon is appended.
I guess compiler just ignores extra semicolons. I added this rule because if
a semicolon is not needed after a VIR_DEFINE, it should not be there.
But I can remove it if it is not that useful.
4 a.m.
New subject: [libvirt] [PATCH v3 06/35] util: string: introduce typedef for string
Alias virString to (char *) so that the new cleanup macros
can be used for a list of strings (char **).
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virstring.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/util/virstring.h b/src/util/virstring.h
index 607ae66..ac930fd 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -26,6 +26,8 @@
# include "internal.h"
+typedef char *virString;
+
char **virStringSplitCount(const char *string,
const char *delim,
size_t max_tokens,
--
1.8.3.1
7:17 a.m.
New subject: [libvirt] [PATCH v3 06/35] util: string: introduce typedef for string
On Sat, Jun 30, 2018 at 02:30:10PM +0530, Sukrit Bhatnagar wrote:
Alias virString to (char *) so that the new cleanup macros
can be used for a list of strings (char **).
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virstring.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/util/virstring.h b/src/util/virstring.h
index 607ae66..ac930fd 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -26,6 +26,8 @@
# include "internal.h"
+typedef char *virString;
+
char **virStringSplitCount(const char *string,
const char *delim,
size_t max_tokens,
Not a deal breaker, but I'd probably squash this into the next patch.
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
4 a.m.
New subject: [libvirt] [PATCH v3 07/35] util: string: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in
src/util/viralloc.h, define a new wrapper around an existing
cleanup function which will be called when a variable declared
with VIR_AUTOPTR macro goes out of scope.
When a list of strings (virString *) is declared using VIR_AUTOPTR,
the function virStringListFree will be run automatically on it when
it goes out of scope.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virstring.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/util/virstring.h b/src/util/virstring.h
index ac930fd..726e02b 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -25,6 +25,7 @@
# include <stdarg.h>
# include "internal.h"
+# include "viralloc.h"
typedef char *virString;
@@ -311,4 +312,6 @@ int virStringParsePort(const char *str,
unsigned int *port)
ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+VIR_DEFINE_AUTOPTR_FUNC(virString, virStringListFree)
+
#endif /* __VIR_STRING_H__ */
--
1.8.3.1
7:18 a.m.
New subject: [libvirt] [PATCH v3 07/35] util: string: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
On Sat, Jun 30, 2018 at 02:30:11PM +0530, Sukrit Bhatnagar wrote:
Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in
src/util/viralloc.h, define a new wrapper around an existing
cleanup function which will be called when a variable declared
with VIR_AUTOPTR macro goes out of scope.
When a list of strings (virString *) is declared using VIR_AUTOPTR,
the function virStringListFree will be run automatically on it when
it goes out of scope.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
If you decide to go with the squash of the previous patch into this one, a
small note should be added into the commit message.
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
4 a.m.
New subject: [libvirt] [PATCH v3 08/35] util: command: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in
src/util/viralloc.h, define a new wrapper around an existing
cleanup function which will be called when a variable declared
with VIR_AUTOPTR macro goes out of scope.
When a variable of type virCommandPtr is declared using VIR_AUTOPTR,
the function virCommandFree will be run automatically on it when it
goes out of scope.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/vircommand.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/util/vircommand.h b/src/util/vircommand.h
index 883e212..90bcc6c 100644
--- a/src/util/vircommand.h
+++ b/src/util/vircommand.h
@@ -24,6 +24,7 @@
# include "internal.h"
# include "virbuffer.h"
+# include "viralloc.h"
typedef struct _virCommand virCommand;
typedef virCommand *virCommandPtr;
@@ -218,5 +219,6 @@ int virCommandRunNul(virCommandPtr cmd,
virCommandRunNulFunc func,
void *data);
+VIR_DEFINE_AUTOPTR_FUNC(virCommand, virCommandFree)
#endif /* __VIR_COMMAND_H__ */
--
1.8.3.1
7:19 a.m.
New subject: [libvirt] [PATCH v3 08/35] util: command: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
On Sat, Jun 30, 2018 at 02:30:12PM +0530, Sukrit Bhatnagar wrote:
Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in
src/util/viralloc.h, define a new wrapper around an existing
cleanup function which will be called when a variable declared
with VIR_AUTOPTR macro goes out of scope.
When a variable of type virCommandPtr is declared using VIR_AUTOPTR,
the function virCommandFree will be run automatically on it when it
goes out of scope.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
4 a.m.
New subject: [libvirt] [PATCH v3 09/35] util: command: remove redundant include directive
The include directive for viralloc.h is added in vircommand.h
in a previous patch.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/vircommand.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 6dab105..8681e7b 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -44,7 +44,6 @@
#define __VIR_COMMAND_PRIV_H_ALLOW__
#include "vircommandpriv.h"
-#include "viralloc.h"
#include "virerror.h"
#include "virutil.h"
#include "virlog.h"
--
1.8.3.1
7:20 a.m.
New subject: [libvirt] [PATCH v3 09/35] util: command: remove redundant include directive
On Sat, Jun 30, 2018 at 02:30:13PM +0530, Sukrit Bhatnagar wrote:
The include directive for viralloc.h is added in vircommand.h
in a previous patch.
How about:
"viralloc.h is pulled in by vircommand.h"
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
4 a.m.
New subject: [libvirt] [PATCH v3 10/35] util: command: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/vircommand.c | 40 ++++++++++++----------------------------
1 file changed, 12 insertions(+), 28 deletions(-)
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 8681e7b..d328431 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -507,11 +507,11 @@ virExec(virCommandPtr cmd)
int childout = -1;
int childerr = -1;
int tmpfd;
- char *binarystr = NULL;
+ VIR_AUTOFREE(char *) binarystr = NULL;
const char *binary = NULL;
int ret;
struct sigaction waxon, waxoff;
- gid_t *groups = NULL;
+ VIR_AUTOFREE(gid_t *) groups = NULL;
int ngroups;
if (cmd->args[0][0] != '/') {
@@ -604,9 +604,6 @@ virExec(virCommandPtr cmd)
cmd->pid = pid;
- VIR_FREE(groups);
- VIR_FREE(binarystr);
-
return 0;
}
@@ -796,9 +793,6 @@ virExec(virCommandPtr cmd)
/* This is cleanup of parent process only - child
should never jump here on error */
- VIR_FREE(binarystr);
- VIR_FREE(groups);
-
/* NB we don't virReportError() on any failures here
because the code which jumped here already raised
an error condition which we must not overwrite */
@@ -2386,7 +2380,7 @@ int
virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
{
int ret = -1;
- char *str = NULL;
+ VIR_AUTOFREE(char *) str = NULL;
size_t i;
bool synchronous = false;
int infd[2] = {-1, -1};
@@ -2511,7 +2505,6 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
VIR_FORCE_CLOSE(cmd->infd);
VIR_FORCE_CLOSE(cmd->inpipe);
}
- VIR_FREE(str);
return ret;
}
@@ -2588,8 +2581,8 @@ virCommandWait(virCommandPtr cmd, int *exitstatus)
if (exitstatus && (cmd->rawStatus || WIFEXITED(status))) {
*exitstatus = cmd->rawStatus ? status : WEXITSTATUS(status);
} else if (status) {
- char *str = virCommandToString(cmd);
- char *st = virProcessTranslateStatus(status);
+ VIR_AUTOFREE(char *) str = virCommandToString(cmd);
+ VIR_AUTOFREE(char *) st = virProcessTranslateStatus(status);
bool haveErrMsg = cmd->errbuf && *cmd->errbuf &&
(*cmd->errbuf)[0];
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -2597,8 +2590,6 @@ virCommandWait(virCommandPtr cmd, int *exitstatus)
str ? str : cmd->args[0], NULLSTR(st),
haveErrMsg ? ": " : "",
haveErrMsg ? *cmd->errbuf : "");
- VIR_FREE(str);
- VIR_FREE(st);
return -1;
}
}
@@ -2718,7 +2709,7 @@ int virCommandHandshakeWait(virCommandPtr cmd)
return -1;
}
if (c != '1') {
- char *msg;
+ VIR_AUTOFREE(char *) msg = NULL;
ssize_t len;
if (VIR_ALLOC_N(msg, 1024) < 0) {
VIR_FORCE_CLOSE(cmd->handshakeWait[0]);
@@ -2731,7 +2722,6 @@ int virCommandHandshakeWait(virCommandPtr cmd)
if ((len = saferead(cmd->handshakeWait[0], msg, 1024)) < 0) {
VIR_FORCE_CLOSE(cmd->handshakeWait[0]);
- VIR_FREE(msg);
virReportSystemError(errno, "%s",
_("No error message from child failure"));
return -1;
@@ -2739,7 +2729,6 @@ int virCommandHandshakeWait(virCommandPtr cmd)
VIR_FORCE_CLOSE(cmd->handshakeWait[0]);
msg[len-1] = '\0';
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", msg);
- VIR_FREE(msg);
return -1;
}
VIR_FORCE_CLOSE(cmd->handshakeWait[0]);
@@ -2853,8 +2842,8 @@ virCommandFree(virCommandPtr cmd)
* This requests asynchronous string IO on @cmd. It is useful in
* combination with virCommandRunAsync():
*
- * virCommandPtr cmd = virCommandNew*(...);
- * char *buf = NULL;
+ * VIR_AUTOPTR(virCommand) cmd = virCommandNew*(...);
+ * VIR_AUTOFREE(char *) buf = NULL;
*
* ...
*
@@ -2862,21 +2851,18 @@ virCommandFree(virCommandPtr cmd)
* virCommandDoAsyncIO(cmd);
*
* if (virCommandRunAsync(cmd, NULL) < 0)
- * goto cleanup;
+ * return;
*
* ...
*
* if (virCommandWait(cmd, NULL) < 0)
- * goto cleanup;
+ * return;
*
* // @buf now contains @cmd's stdout
* VIR_DEBUG("STDOUT: %s", NULLSTR(buf));
*
* ...
*
- * cleanup:
- * VIR_FREE(buf);
- * virCommandFree(cmd);
*
* The libvirt's event loop is used for handling stdios of @cmd.
* Since current implementation uses strlen to determine length
@@ -2969,11 +2955,11 @@ virCommandRunRegex(virCommandPtr cmd,
{
int err;
regex_t *reg;
- regmatch_t *vars = NULL;
+ VIR_AUTOFREE(regmatch_t *) vars = NULL;
size_t i, j, k;
int totgroups = 0, ngroup = 0, maxvars = 0;
char **groups;
- char *outbuf = NULL;
+ VIR_AUTOFREE(char *) outbuf = NULL;
char **lines = NULL;
int ret = -1;
@@ -3054,13 +3040,11 @@ virCommandRunRegex(virCommandPtr cmd,
ret = 0;
cleanup:
virStringListFree(lines);
- VIR_FREE(outbuf);
if (groups) {
for (j = 0; j < totgroups; j++)
VIR_FREE(groups[j]);
VIR_FREE(groups);
}
- VIR_FREE(vars);
for (i = 0; i < nregex; i++)
regfree(®[i]);
--
1.8.3.1
7:21 a.m.
New subject: [libvirt] [PATCH v3 10/35] util: command: use VIR_AUTOFREE instead of VIR_FREE for scalar types
On Sat, Jun 30, 2018 at 02:30:14PM +0530, Sukrit Bhatnagar wrote:
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
4 a.m.
New subject: [libvirt] [PATCH v3 11/35] util: command: use VIR_AUTOPTR for aggregate types
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOPTR macro for declaring aggregate pointer variables,
majority of the calls to *Free functions can be dropped, which
in turn leads to getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/vircommand.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index d328431..6edb4d5 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -823,10 +823,9 @@ int
virRun(const char *const*argv, int *status)
{
int ret;
- virCommandPtr cmd = virCommandNewArgs(argv);
+ VIR_AUTOPTR(virCommand) cmd = virCommandNewArgs(argv);
ret = virCommandRun(cmd, status);
- virCommandFree(cmd);
return ret;
}
@@ -2960,7 +2959,7 @@ virCommandRunRegex(virCommandPtr cmd,
int totgroups = 0, ngroup = 0, maxvars = 0;
char **groups;
VIR_AUTOFREE(char *) outbuf = NULL;
- char **lines = NULL;
+ VIR_AUTOPTR(virString) lines = NULL;
int ret = -1;
/* Compile all regular expressions */
@@ -3039,7 +3038,6 @@ virCommandRunRegex(virCommandPtr cmd,
ret = 0;
cleanup:
- virStringListFree(lines);
if (groups) {
for (j = 0; j < totgroups; j++)
VIR_FREE(groups[j]);
--
1.8.3.1
7:24 a.m.
New subject: [libvirt] [PATCH v3 11/35] util: command: use VIR_AUTOPTR for aggregate types
On Sat, Jun 30, 2018 at 02:30:15PM +0530, Sukrit Bhatnagar wrote:
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOPTR macro for declaring aggregate pointer variables,
majority of the calls to *Free functions can be dropped, which
in turn leads to getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/vircommand.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index d328431..6edb4d5 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -823,10 +823,9 @@ int
virRun(const char *const*argv, int *status)
{
int ret;
- virCommandPtr cmd = virCommandNewArgs(argv);
+ VIR_AUTOPTR(virCommand) cmd = virCommandNewArgs(argv);
ret = virCommandRun(cmd, status);
- virCommandFree(cmd);
You can return virCommandRun directly.
return ret;
With that:
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
4 a.m.
New subject: [libvirt] [PATCH v3 12/35] util: file: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in
src/util/viralloc.h, define a new wrapper around an existing
cleanup function which will be called when a variable declared
with VIR_AUTOPTR macro goes out of scope.
When a variable of type virFileWrapperFdPtr is declared using
VIR_AUTOPTR, the function virFileWrapperFdFree will be run
automatically on it when it goes out of scope.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virfile.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 6f1e802..b30a1d3 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -32,6 +32,7 @@
# include "internal.h"
# include "virbitmap.h"
# include "virstoragefile.h"
+# include "viralloc.h"
typedef enum {
VIR_FILE_CLOSE_PRESERVE_ERRNO = 1 << 0,
@@ -367,4 +368,6 @@ int virFileInData(int fd,
int *inData,
long long *length);
+VIR_DEFINE_AUTOPTR_FUNC(virFileWrapperFd, virFileWrapperFdFree)
+
#endif /* __VIR_FILE_H */
--
1.8.3.1
4 a.m.
New subject: [libvirt] [PATCH v3 13/35] util: file: remove redundant include directive
The include directive for viralloc.h is added in virfile.h
in a previous patch.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virfile.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 378d03e..2690e2d 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -67,7 +67,6 @@
#include "configmake.h"
#include "intprops.h"
-#include "viralloc.h"
#include "vircommand.h"
#include "virerror.h"
#include "virfile.h"
--
1.8.3.1
4 a.m.
New subject: [libvirt] [PATCH v3 14/35] util: file: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virfile.c | 310 ++++++++++++++++++-----------------------------------
1 file changed, 102 insertions(+), 208 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 2690e2d..3a7445f 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -211,7 +211,7 @@ virFileWrapperFdNew(int *fd, const char *name, unsigned int flags)
bool output = false;
int pipefd[2] = { -1, -1 };
int mode = -1;
- char *iohelper_path = NULL;
+ VIR_AUTOFREE(char *) iohelper_path = NULL;
if (!flags) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -262,8 +262,6 @@ virFileWrapperFdNew(int *fd, const char *name, unsigned int flags)
ret->cmd = virCommandNewArgList(iohelper_path, name, NULL);
- VIR_FREE(iohelper_path);
-
if (output) {
virCommandSetInputFD(ret->cmd, pipefd[0]);
virCommandSetOutputFD(ret->cmd, fd);
@@ -294,7 +292,6 @@ virFileWrapperFdNew(int *fd, const char *name, unsigned int flags)
return ret;
error:
- VIR_FREE(iohelper_path);
VIR_FORCE_CLOSE(pipefd[0]);
VIR_FORCE_CLOSE(pipefd[1]);
virFileWrapperFdFree(ret);
@@ -492,7 +489,7 @@ virFileRewrite(const char *path,
virFileRewriteFunc rewrite,
const void *opaque)
{
- char *newfile = NULL;
+ VIR_AUTOFREE(char *) newfile = NULL;
int fd = -1;
int ret = -1;
@@ -533,10 +530,8 @@ virFileRewrite(const char *path,
cleanup:
VIR_FORCE_CLOSE(fd);
- if (newfile) {
+ if (newfile)
unlink(newfile);
- VIR_FREE(newfile);
- }
return ret;
}
@@ -763,7 +758,7 @@ int virFileLoopDeviceAssociate(const char *file,
int lofd = -1;
int fsfd = -1;
struct loop_info64 lo;
- char *loname = NULL;
+ VIR_AUTOFREE(char *) loname = NULL;
int ret = -1;
if ((lofd = virFileLoopDeviceOpen(&loname)) < 0)
@@ -801,7 +796,6 @@ int virFileLoopDeviceAssociate(const char *file,
ret = 0;
cleanup:
- VIR_FREE(loname);
VIR_FORCE_CLOSE(fsfd);
if (ret == -1)
VIR_FORCE_CLOSE(lofd);
@@ -816,8 +810,7 @@ int virFileLoopDeviceAssociate(const char *file,
static int
virFileNBDDeviceIsBusy(const char *dev_name)
{
- char *path;
- int ret = -1;
+ VIR_AUTOFREE(char *) path = NULL;
if (virAsprintf(&path, SYSFS_BLOCK_DIR "/%s/pid",
dev_name) < 0)
@@ -825,18 +818,14 @@ virFileNBDDeviceIsBusy(const char *dev_name)
if (!virFileExists(path)) {
if (errno == ENOENT)
- ret = 0;
+ return 0;
else
virReportSystemError(errno,
_("Cannot check NBD device %s pid"),
dev_name);
- goto cleanup;
+ return -1;
}
- ret = 1;
-
- cleanup:
- VIR_FREE(path);
- return ret;
+ return 1;
}
@@ -881,15 +870,13 @@ virFileNBDLoadDriver(void)
"administratively prohibited"));
return false;
} else {
- char *errbuf = NULL;
+ VIR_AUTOFREE(char *) errbuf = NULL;
if ((errbuf = virKModLoad(NBD_DRIVER, true))) {
- VIR_FREE(errbuf);
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Failed to load nbd module"));
return false;
}
- VIR_FREE(errbuf);
}
return true;
}
@@ -899,8 +886,8 @@ int virFileNBDDeviceAssociate(const char *file,
bool readonly,
char **dev)
{
- char *nbddev = NULL;
- char *qemunbd = NULL;
+ VIR_AUTOFREE(char *) nbddev = NULL;
+ VIR_AUTOFREE(char *) qemunbd = NULL;
virCommandPtr cmd = NULL;
int ret = -1;
const char *fmtstr = NULL;
@@ -948,8 +935,6 @@ int virFileNBDDeviceAssociate(const char *file,
ret = 0;
cleanup:
- VIR_FREE(nbddev);
- VIR_FREE(qemunbd);
virCommandFree(cmd);
return ret;
}
@@ -996,7 +981,6 @@ int virFileDeleteTree(const char *dir)
{
DIR *dh;
struct dirent *de;
- char *filepath = NULL;
int ret = -1;
int direrr;
@@ -1008,6 +992,7 @@ int virFileDeleteTree(const char *dir)
return -1;
while ((direrr = virDirRead(dh, &de, dir)) > 0) {
+ VIR_AUTOFREE(char *) filepath = NULL;
struct stat sb;
if (virAsprintf(&filepath, "%s/%s",
@@ -1031,8 +1016,6 @@ int virFileDeleteTree(const char *dir)
goto cleanup;
}
}
-
- VIR_FREE(filepath);
}
if (direrr < 0)
goto cleanup;
@@ -1047,7 +1030,6 @@ int virFileDeleteTree(const char *dir)
ret = 0;
cleanup:
- VIR_FREE(filepath);
VIR_DIR_CLOSE(dh);
return ret;
}
@@ -1205,7 +1187,7 @@ static int
safezero_slow(int fd, off_t offset, off_t len)
{
int r;
- char *buf;
+ VIR_AUTOFREE(char *) buf = NULL;
unsigned long long remain, bytes;
if (lseek(fd, offset, SEEK_SET) < 0)
@@ -1226,15 +1208,12 @@ safezero_slow(int fd, off_t offset, off_t len)
bytes = remain;
r = safewrite(fd, buf, bytes);
- if (r < 0) {
- VIR_FREE(buf);
+ if (r < 0)
return -1;
- }
/* safewrite() guarantees all data will be written */
remain -= bytes;
}
- VIR_FREE(buf);
return 0;
}
@@ -1597,8 +1576,7 @@ virFileRelLinkPointsTo(const char *directory,
const char *checkLink,
const char *checkDest)
{
- char *candidate;
- int ret;
+ VIR_AUTOFREE(char *) candidate = NULL;
if (*checkLink == '/')
return virFileLinkPointsTo(checkLink, checkDest);
@@ -1610,9 +1588,7 @@ virFileRelLinkPointsTo(const char *directory,
}
if (virAsprintf(&candidate, "%s/%s", directory, checkLink) < 0)
return -1;
- ret = virFileLinkPointsTo(candidate, checkDest);
- VIR_FREE(candidate);
- return ret;
+ return virFileLinkPointsTo(candidate, checkDest);
}
@@ -1945,44 +1921,40 @@ virFileIsExecutable(const char *file)
*/
int virFileIsMountPoint(const char *file)
{
- char *parent = NULL;
- int ret = -1;
+ VIR_AUTOFREE(char *) parent = NULL;
+ int ret;
struct stat sb1, sb2;
if (!(parent = mdir_name(file))) {
virReportOOMError();
- goto cleanup;
+ return -1;
}
VIR_DEBUG("Comparing '%s' to '%s'", file, parent);
if (stat(file, &sb1) < 0) {
if (errno == ENOENT)
- ret = 0;
+ return 0;
else
virReportSystemError(errno,
_("Cannot stat '%s'"),
file);
- goto cleanup;
+ return -1;
}
if (stat(parent, &sb2) < 0) {
virReportSystemError(errno,
_("Cannot stat '%s'"),
parent);
- goto cleanup;
+ return -1;
}
- if (!S_ISDIR(sb1.st_mode)) {
- ret = 0;
- goto cleanup;
- }
+ if (!S_ISDIR(sb1.st_mode))
+ return 0;
ret = sb1.st_dev != sb2.st_dev;
VIR_DEBUG("Is mount %d", ret);
- cleanup:
- VIR_FREE(parent);
return ret;
}
@@ -2165,7 +2137,7 @@ virFileAccessibleAs(const char *path, int mode,
{
pid_t pid = 0;
int status, ret = 0;
- gid_t *groups;
+ VIR_AUTOFREE(gid_t *) groups = NULL;
int ngroups;
if (uid == geteuid() &&
@@ -2178,13 +2150,10 @@ virFileAccessibleAs(const char *path, int mode,
pid = virFork();
- if (pid < 0) {
- VIR_FREE(groups);
+ if (pid < 0)
return -1;
- }
if (pid) { /* parent */
- VIR_FREE(groups);
if (virProcessWait(pid, &status, false) < 0) {
/* virProcessWait() already reported error */
errno = EINTR;
@@ -2280,7 +2249,7 @@ virFileOpenForked(const char *path, int openflags, mode_t mode,
int recvfd_errno = 0;
int fd = -1;
int pair[2] = { -1, -1 };
- gid_t *groups;
+ VIR_AUTOFREE(gid_t *) groups = NULL;
int ngroups;
bool created = false;
@@ -2298,16 +2267,12 @@ virFileOpenForked(const char *path, int openflags, mode_t mode,
virReportSystemError(errno,
_("failed to create socket needed for
'%s'"),
path);
- VIR_FREE(groups);
return ret;
}
pid = virFork();
- if (pid < 0) {
- ret = -errno;
- VIR_FREE(groups);
- return ret;
- }
+ if (pid < 0)
+ return -errno;
if (pid == 0) {
@@ -2372,7 +2337,6 @@ virFileOpenForked(const char *path, int openflags, mode_t mode,
/* parent */
- VIR_FREE(groups);
VIR_FORCE_CLOSE(pair[1]);
do {
@@ -2573,7 +2537,7 @@ virFileRemove(const char *path,
{
pid_t pid;
int status = 0, ret = 0;
- gid_t *groups;
+ VIR_AUTOFREE(gid_t *) groups = NULL;
int ngroups;
if (!virFileRemoveNeedsSetuid(path, uid, gid)) {
@@ -2598,15 +2562,11 @@ virFileRemove(const char *path,
pid = virFork();
- if (pid < 0) {
- ret = -errno;
- VIR_FREE(groups);
- return ret;
- }
+ if (pid < 0)
+ return -errno;
if (pid) { /* parent */
/* wait for child to complete, and retrieve its exit code */
- VIR_FREE(groups);
if (virProcessWait(pid, &status, 0) < 0) {
/* virProcessWait() reports errno on waitpid failure, so we'll just
@@ -2738,7 +2698,7 @@ virDirCreate(const char *path,
struct stat st;
pid_t pid;
int status = 0, ret = 0;
- gid_t *groups;
+ VIR_AUTOFREE(gid_t *) groups = NULL;
int ngroups;
bool created = false;
@@ -2774,15 +2734,11 @@ virDirCreate(const char *path,
pid = virFork();
- if (pid < 0) {
- ret = -errno;
- VIR_FREE(groups);
- return ret;
- }
+ if (pid < 0)
+ return -errno;
if (pid) { /* parent */
/* wait for child to complete, and retrieve its exit code */
- VIR_FREE(groups);
if (virProcessWait(pid, &status, 0) < 0) {
/* virProcessWait() reports errno on waitpid failure, so we'll just
@@ -3047,13 +3003,13 @@ int virFileChownFiles(const char *name,
int ret = -1;
int direrr;
DIR *dir;
- char *path = NULL;
if (virDirOpen(&dir, name) < 0)
return -1;
while ((direrr = virDirRead(dir, &ent, name)) > 0) {
- VIR_FREE(path);
+ VIR_AUTOFREE(char *) path = NULL;
+
if (virAsprintf(&path, "%s/%s", name, ent->d_name) < 0)
goto cleanup;
@@ -3075,8 +3031,6 @@ int virFileChownFiles(const char *name,
ret = 0;
cleanup:
- VIR_FREE(path);
-
virDirClose(&dir);
return ret;
@@ -3138,19 +3092,14 @@ int
virFileMakePathWithMode(const char *path,
mode_t mode)
{
- int ret = -1;
- char *tmp;
+ VIR_AUTOFREE(char *) tmp = NULL;
if (VIR_STRDUP(tmp, path) < 0) {
errno = ENOMEM;
- goto cleanup;
+ return -1;
}
- ret = virFileMakePathHelper(tmp, mode);
-
- cleanup:
- VIR_FREE(tmp);
- return ret;
+ return virFileMakePathHelper(tmp, mode);
}
@@ -3158,8 +3107,7 @@ int
virFileMakeParentPath(const char *path)
{
char *p;
- char *tmp;
- int ret = -1;
+ VIR_AUTOFREE(char *) tmp = NULL;
VIR_DEBUG("path=%s", path);
@@ -3170,15 +3118,11 @@ virFileMakeParentPath(const char *path)
if ((p = strrchr(tmp, '/')) == NULL) {
errno = EINVAL;
- goto cleanup;
+ return -1;
}
*p = '\0';
- ret = virFileMakePathHelper(tmp, 0777);
-
- cleanup:
- VIR_FREE(tmp);
- return ret;
+ return virFileMakePathHelper(tmp, 0777);
}
@@ -3212,7 +3156,7 @@ virFileOpenTty(int *ttymaster, char **ttyName, int rawmode)
* doesn't have to worry about that mess? */
int ret = -1;
int slave = -1;
- char *name = NULL;
+ VIR_AUTOFREE(char *) name = NULL;
/* Unfortunately, we can't use the name argument of openpty, since
* there is no guarantee on how large the buffer has to be.
@@ -3273,7 +3217,6 @@ virFileOpenTty(int *ttymaster, char **ttyName, int rawmode)
if (ret != 0)
VIR_FORCE_CLOSE(*ttymaster);
VIR_FORCE_CLOSE(slave);
- VIR_FREE(name);
return ret;
}
@@ -3373,21 +3316,17 @@ virFileSkipRoot(const char *path)
int
virFileAbsPath(const char *path, char **abspath)
{
- char *buf;
-
if (path[0] == '/') {
if (VIR_STRDUP(*abspath, path) < 0)
return -1;
} else {
- buf = getcwd(NULL, 0);
+ VIR_AUTOFREE(char *) buf = getcwd(NULL, 0);
+
if (buf == NULL)
return -1;
- if (virAsprintf(abspath, "%s/%s", buf, path) < 0) {
- VIR_FREE(buf);
+ if (virAsprintf(abspath, "%s/%s", buf, path) < 0)
return -1;
- }
- VIR_FREE(buf);
}
return 0;
@@ -3487,7 +3426,7 @@ virFileRemoveLastComponent(char *path)
int virFilePrintf(FILE *fp, const char *msg, ...)
{
va_list vargs;
- char *str;
+ VIR_AUTOFREE(char *) str = NULL;
int ret;
va_start(vargs, msg);
@@ -3501,8 +3440,6 @@ int virFilePrintf(FILE *fp, const char *msg, ...)
ret = -1;
}
- VIR_FREE(str);
-
cleanup:
va_end(vargs);
@@ -3538,7 +3475,8 @@ int
virFileIsSharedFSType(const char *path,
int fstypes)
{
- char *dirpath, *p;
+ VIR_AUTOFREE(char *) dirpath = NULL;
+ char *p;
struct statfs sb;
int statfs_ret;
@@ -3558,7 +3496,6 @@ virFileIsSharedFSType(const char *path,
if ((p = strrchr(dirpath, '/')) == NULL) {
virReportSystemError(EINVAL,
_("Invalid relative path '%s'"), path);
- VIR_FREE(dirpath);
return -1;
}
@@ -3571,8 +3508,6 @@ virFileIsSharedFSType(const char *path,
} while ((statfs_ret < 0) && (p != dirpath));
- VIR_FREE(dirpath);
-
if (statfs_ret < 0) {
virReportSystemError(errno,
_("cannot determine filesystem for
'%s'"),
@@ -3639,18 +3574,20 @@ virFileGetHugepageSize(const char *path,
static int
virFileGetDefaultHugepageSize(unsigned long long *size)
{
- int ret = -1;
- char *meminfo, *c, *n, *unit;
+ VIR_AUTOFREE(char *) meminfo = NULL;
+ char *c;
+ char *n;
+ char *unit;
if (virFileReadAll(PROC_MEMINFO, 4096, &meminfo) < 0)
- goto cleanup;
+ return -1;
if (!(c = strstr(meminfo, HUGEPAGESIZE_STR))) {
virReportError(VIR_ERR_NO_SUPPORT,
_("%s not found in %s"),
HUGEPAGESIZE_STR,
PROC_MEMINFO);
- goto cleanup;
+ return -1;
}
c += strlen(HUGEPAGESIZE_STR);
@@ -3663,13 +3600,10 @@ virFileGetDefaultHugepageSize(unsigned long long *size)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to parse %s %s"),
HUGEPAGESIZE_STR, c);
- goto cleanup;
+ return -1;
}
- ret = 0;
- cleanup:
- VIR_FREE(meminfo);
- return ret;
+ return 0;
}
# define PROC_MOUNTS "/proc/mounts"
@@ -3963,10 +3897,8 @@ virFileCopyACLs(const char *src,
int
virFileComparePaths(const char *p1, const char *p2)
{
- int ret = -1;
- char *res1, *res2;
-
- res1 = res2 = NULL;
+ VIR_AUTOFREE(char *) res1 = NULL;
+ VIR_AUTOFREE(char *) res2 = NULL;
/* Assume p1 and p2 are symlinks, so try to resolve and canonicalize them.
* Canonicalization fails for example on file systems names like 'proc' or
@@ -3975,19 +3907,13 @@ virFileComparePaths(const char *p1, const char *p2)
*/
ignore_value(virFileResolveLink(p1, &res1));
if (!res1 && VIR_STRDUP(res1, p1) < 0)
- goto cleanup;
+ return -1;
ignore_value(virFileResolveLink(p2, &res2));
if (!res2 && VIR_STRDUP(res2, p2) < 0)
- goto cleanup;
-
- ret = STREQ_NULLABLE(res1, res2);
-
- cleanup:
- VIR_FREE(res1);
- VIR_FREE(res2);
+ return -1;
- return ret;
+ return STREQ_NULLABLE(res1, res2);
}
@@ -4131,25 +4057,22 @@ virFileInData(int fd ATTRIBUTE_UNUSED,
int
virFileReadValueInt(int *value, const char *format, ...)
{
- int ret = -1;
- char *str = NULL;
- char *path = NULL;
+ VIR_AUTOFREE(char *) str = NULL;
+ VIR_AUTOFREE(char *) path = NULL;
va_list ap;
va_start(ap, format);
if (virVasprintf(&path, format, ap) < 0) {
va_end(ap);
- goto cleanup;
+ return -1;
}
va_end(ap);
- if (!virFileExists(path)) {
- ret = -2;
- goto cleanup;
- }
+ if (!virFileExists(path))
+ return -2;
if (virFileReadAll(path, INT_BUFSIZE_BOUND(*value), &str) < 0)
- goto cleanup;
+ return -1;
virStringTrimOptionalNewline(str);
@@ -4157,14 +4080,10 @@ virFileReadValueInt(int *value, const char *format, ...)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid integer value '%s' in file
'%s'"),
str, path);
- goto cleanup;
+ return -1;
}
- ret = 0;
- cleanup:
- VIR_FREE(path);
- VIR_FREE(str);
- return ret;
+ return 0;
}
@@ -4181,25 +4100,22 @@ virFileReadValueInt(int *value, const char *format, ...)
int
virFileReadValueUint(unsigned int *value, const char *format, ...)
{
- int ret = -1;
- char *str = NULL;
- char *path = NULL;
+ VIR_AUTOFREE(char *) str = NULL;
+ VIR_AUTOFREE(char *) path = NULL;
va_list ap;
va_start(ap, format);
if (virVasprintf(&path, format, ap) < 0) {
va_end(ap);
- goto cleanup;
+ return -1;
}
va_end(ap);
- if (!virFileExists(path)) {
- ret = -2;
- goto cleanup;
- }
+ if (!virFileExists(path))
+ return -2;
if (virFileReadAll(path, INT_BUFSIZE_BOUND(*value), &str) < 0)
- goto cleanup;
+ return -1;
virStringTrimOptionalNewline(str);
@@ -4207,14 +4123,10 @@ virFileReadValueUint(unsigned int *value, const char *format,
...)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid unsigned integer value '%s' in file
'%s'"),
str, path);
- goto cleanup;
+ return -1;
}
- ret = 0;
- cleanup:
- VIR_FREE(path);
- VIR_FREE(str);
- return ret;
+ return 0;
}
@@ -4231,26 +4143,23 @@ virFileReadValueUint(unsigned int *value, const char *format,
...)
int
virFileReadValueScaledInt(unsigned long long *value, const char *format, ...)
{
- int ret = -1;
- char *str = NULL;
+ VIR_AUTOFREE(char *) str = NULL;
+ VIR_AUTOFREE(char *) path = NULL;
char *endp = NULL;
- char *path = NULL;
va_list ap;
va_start(ap, format);
if (virVasprintf(&path, format, ap) < 0) {
va_end(ap);
- goto cleanup;
+ return -1;
}
va_end(ap);
- if (!virFileExists(path)) {
- ret = -2;
- goto cleanup;
- }
+ if (!virFileExists(path))
+ return -2;
if (virFileReadAll(path, INT_BUFSIZE_BOUND(*value), &str) < 0)
- goto cleanup;
+ return -1;
virStringTrimOptionalNewline(str);
@@ -4258,14 +4167,10 @@ virFileReadValueScaledInt(unsigned long long *value, const char
*format, ...)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid unsigned scaled integer value '%s' in file
'%s'"),
str, path);
- goto cleanup;
+ return -1;
}
- ret = virScaleInteger(value, endp, 1024, ULLONG_MAX);
- cleanup:
- VIR_FREE(path);
- VIR_FREE(str);
- return ret;
+ return virScaleInteger(value, endp, 1024, ULLONG_MAX);
}
/* Arbitrarily sized number, feel free to change, but the function should be
@@ -4285,37 +4190,30 @@ virFileReadValueScaledInt(unsigned long long *value, const char
*format, ...)
int
virFileReadValueBitmap(virBitmapPtr *value, const char *format, ...)
{
- int ret = -1;
- char *str = NULL;
- char *path = NULL;
+ VIR_AUTOFREE(char *) str = NULL;
+ VIR_AUTOFREE(char *) path = NULL;
va_list ap;
va_start(ap, format);
if (virVasprintf(&path, format, ap) < 0) {
va_end(ap);
- goto cleanup;
+ return -1;
}
va_end(ap);
- if (!virFileExists(path)) {
- ret = -2;
- goto cleanup;
- }
+ if (!virFileExists(path))
+ return -2;
if (virFileReadAll(path, VIR_FILE_READ_VALUE_STRING_MAX, &str) < 0)
- goto cleanup;
+ return -1;
virStringTrimOptionalNewline(str);
*value = virBitmapParseUnlimited(str);
if (!*value)
- goto cleanup;
+ return -1;
- ret = 0;
- cleanup:
- VIR_FREE(path);
- VIR_FREE(str);
- return ret;
+ return 0;
}
/**
@@ -4333,30 +4231,26 @@ virFileReadValueBitmap(virBitmapPtr *value, const char *format,
...)
int
virFileReadValueString(char **value, const char *format, ...)
{
- int ret = -1;
- char *str = NULL;
- char *path = NULL;
+ int ret;
+ VIR_AUTOFREE(char *) str = NULL;
+ VIR_AUTOFREE(char *) path = NULL;
va_list ap;
va_start(ap, format);
if (virVasprintf(&path, format, ap) < 0) {
va_end(ap);
- goto cleanup;
+ return -1;
}
va_end(ap);
- if (!virFileExists(path)) {
- ret = -2;
- goto cleanup;
- }
+ if (!virFileExists(path))
+ return -2;
ret = virFileReadAll(path, VIR_FILE_READ_VALUE_STRING_MAX, value);
if (*value)
virStringTrimOptionalNewline(*value);
- cleanup:
- VIR_FREE(path);
- VIR_FREE(str);
+
return ret;
}
--
1.8.3.1
7:29 a.m.
New subject: [libvirt] [PATCH v3 14/35] util: file: use VIR_AUTOFREE instead of VIR_FREE for scalar types
On Sat, Jun 30, 2018 at 02:30:18PM +0530, Sukrit Bhatnagar wrote:
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virfile.c | 310 ++++++++++++++++++-----------------------------------
1 file changed, 102 insertions(+), 208 deletions(-)
Nice stat :).
Reviewed-by: Erik Skultety <eskultet(a)redhat.com> (patches 12-14)
4 a.m.
New subject: [libvirt] [PATCH v3 15/35] util: file: use VIR_AUTOPTR for aggregate types
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOPTR macro for declaring aggregate pointer variables,
majority of the calls to *Free functions can be dropped, which
in turn leads to getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virfile.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 3a7445f..6b94885 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -888,20 +888,19 @@ int virFileNBDDeviceAssociate(const char *file,
{
VIR_AUTOFREE(char *) nbddev = NULL;
VIR_AUTOFREE(char *) qemunbd = NULL;
- virCommandPtr cmd = NULL;
- int ret = -1;
+ VIR_AUTOPTR(virCommand) cmd = NULL;
const char *fmtstr = NULL;
if (!virFileNBDLoadDriver())
- goto cleanup;
+ return -1;
if (!(nbddev = virFileNBDDeviceFindUnused()))
- goto cleanup;
+ return -1;
if (!(qemunbd = virFindFileInPath("qemu-nbd"))) {
virReportSystemError(ENOENT, "%s",
_("Unable to find 'qemu-nbd' binary in
$PATH"));
- goto cleanup;
+ return -1;
}
if (fmt > 0)
@@ -926,17 +925,14 @@ int virFileNBDDeviceAssociate(const char *file,
/* qemu-nbd will daemonize itself */
if (virCommandRun(cmd, NULL) < 0)
- goto cleanup;
+ return -1;
VIR_DEBUG("Associated NBD device %s with file %s and format %s",
nbddev, file, fmtstr);
*dev = nbddev;
nbddev = NULL;
- ret = 0;
- cleanup:
- virCommandFree(cmd);
- return ret;
+ return 0;
}
#else /* __linux__ */
--
1.8.3.1
7:30 a.m.
New subject: [libvirt] [PATCH v3 15/35] util: file: use VIR_AUTOPTR for aggregate types
On Sat, Jun 30, 2018 at 02:30:19PM +0530, Sukrit Bhatnagar wrote:
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOPTR macro for declaring aggregate pointer variables,
majority of the calls to *Free functions can be dropped, which
in turn leads to getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
4 a.m.
New subject: [libvirt] [PATCH v3 16/35] util: authconfig: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in
src/util/viralloc.h, define a new wrapper around an existing
cleanup function which will be called when a variable declared
with VIR_AUTOPTR macro goes out of scope.
When a variable of type virAuthConfigPtr is declared using
VIR_AUTOPTR, the function virAuthConfigFree will be run
automatically on it when it goes out of scope.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virauthconfig.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/util/virauthconfig.h b/src/util/virauthconfig.h
index ac0ceeb..d8a3849 100644
--- a/src/util/virauthconfig.h
+++ b/src/util/virauthconfig.h
@@ -24,6 +24,7 @@
# define __VIR_AUTHCONFIG_H__
# include "internal.h"
+# include "viralloc.h"
typedef struct _virAuthConfig virAuthConfig;
typedef virAuthConfig *virAuthConfigPtr;
@@ -42,4 +43,6 @@ int virAuthConfigLookup(virAuthConfigPtr auth,
const char *credname,
const char **value);
+VIR_DEFINE_AUTOPTR_FUNC(virAuthConfig, virAuthConfigFree)
+
#endif /* __VIR_AUTHCONFIG_H__ */
--
1.8.3.1
7:31 a.m.
New subject: [libvirt] [PATCH v3 16/35] util: authconfig: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
On Sat, Jun 30, 2018 at 02:30:20PM +0530, Sukrit Bhatnagar wrote:
Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in
src/util/viralloc.h, define a new wrapper around an existing
cleanup function which will be called when a variable declared
with VIR_AUTOPTR macro goes out of scope.
When a variable of type virAuthConfigPtr is declared using
VIR_AUTOPTR, the function virAuthConfigFree will be run
automatically on it when it goes out of scope.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
4 a.m.
New subject: [libvirt] [PATCH v3 17/35] util: authconfig: remove redundant include directive
The include directive for viralloc.h is added in virauthconfig.h
in a previous patch.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virauthconfig.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/util/virauthconfig.c b/src/util/virauthconfig.c
index 91c9c0c..3487cc2 100644
--- a/src/util/virauthconfig.c
+++ b/src/util/virauthconfig.c
@@ -25,7 +25,6 @@
#include "virauthconfig.h"
#include "virkeyfile.h"
-#include "viralloc.h"
#include "virlog.h"
#include "virerror.h"
#include "virstring.h"
--
1.8.3.1
7:31 a.m.
New subject: [libvirt] [PATCH v3 17/35] util: authconfig: remove redundant include directive
On Sat, Jun 30, 2018 at 02:30:21PM +0530, Sukrit Bhatnagar wrote:
The include directive for viralloc.h is added in virauthconfig.h
in a previous patch.
Same suggestion as in patch 9.
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
4 a.m.
New subject: [libvirt] [PATCH v3 18/35] util: authconfig: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virauthconfig.c | 34 ++++++++++++----------------------
1 file changed, 12 insertions(+), 22 deletions(-)
diff --git a/src/util/virauthconfig.c b/src/util/virauthconfig.c
index 3487cc2..4acdf1d 100644
--- a/src/util/virauthconfig.c
+++ b/src/util/virauthconfig.c
@@ -105,10 +105,9 @@ int virAuthConfigLookup(virAuthConfigPtr auth,
const char *credname,
const char **value)
{
- char *authgroup = NULL;
- char *credgroup = NULL;
+ VIR_AUTOFREE(char *) authgroup = NULL;
+ VIR_AUTOFREE(char *) credgroup = NULL;
const char *authcred;
- int ret = -1;
*value = NULL;
@@ -118,47 +117,38 @@ int virAuthConfigLookup(virAuthConfigPtr auth,
hostname = "localhost";
if (virAsprintf(&authgroup, "auth-%s-%s", service, hostname) < 0)
- goto cleanup;
+ return -1;
if (!virKeyFileHasGroup(auth->keyfile, authgroup)) {
VIR_FREE(authgroup);
if (virAsprintf(&authgroup, "auth-%s-%s", service,
"default") < 0)
- goto cleanup;
+ return -1;
}
- if (!virKeyFileHasGroup(auth->keyfile, authgroup)) {
- ret = 0;
- goto cleanup;
- }
+ if (!virKeyFileHasGroup(auth->keyfile, authgroup))
+ return 0;
if (!(authcred = virKeyFileGetValueString(auth->keyfile, authgroup,
"credentials"))) {
virReportError(VIR_ERR_CONF_SYNTAX,
_("Missing item 'credentials' in group '%s'
in '%s'"),
authgroup, auth->path);
- goto cleanup;
+ return -1;
}
if (virAsprintf(&credgroup, "credentials-%s", authcred) < 0)
- goto cleanup;
+ return -1;
if (!virKeyFileHasGroup(auth->keyfile, credgroup)) {
virReportError(VIR_ERR_CONF_SYNTAX,
_("Missing group 'credentials-%s' referenced from
group '%s' in '%s'"),
authcred, authgroup, auth->path);
- goto cleanup;
+ return -1;
}
- if (!virKeyFileHasValue(auth->keyfile, credgroup, credname)) {
- ret = 0;
- goto cleanup;
- }
+ if (!virKeyFileHasValue(auth->keyfile, credgroup, credname))
+ return 0;
*value = virKeyFileGetValueString(auth->keyfile, credgroup, credname);
- ret = 0;
-
- cleanup:
- VIR_FREE(authgroup);
- VIR_FREE(credgroup);
- return ret;
+ return 0;
}
--
1.8.3.1
7:32 a.m.
New subject: [libvirt] [PATCH v3 18/35] util: authconfig: use VIR_AUTOFREE instead of VIR_FREE for scalar types
On Sat, Jun 30, 2018 at 02:30:22PM +0530, Sukrit Bhatnagar wrote:
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
4 a.m.
New subject: [libvirt] [PATCH v3 19/35] util: auth: remove redundant include directive
The include directive for viralloc.h is added in virauthconfig.h
in a previous patch.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virauth.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/util/virauth.c b/src/util/virauth.c
index adb093e..c6a2ce7 100644
--- a/src/util/virauth.c
+++ b/src/util/virauth.c
@@ -26,7 +26,6 @@
#include "virauth.h"
#include "virutil.h"
-#include "viralloc.h"
#include "virlog.h"
#include "datatypes.h"
#include "virerror.h"
--
1.8.3.1
7:33 a.m.
New subject: [libvirt] [PATCH v3 19/35] util: auth: remove redundant include directive
On Sat, Jun 30, 2018 at 02:30:23PM +0530, Sukrit Bhatnagar wrote:
The include directive for viralloc.h is added in virauthconfig.h
in a previous patch.
Same as patch 9.
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
4 a.m.
New subject: [libvirt] [PATCH v3 20/35] util: auth: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virauth.c | 45 +++++++++++++--------------------------------
1 file changed, 13 insertions(+), 32 deletions(-)
diff --git a/src/util/virauth.c b/src/util/virauth.c
index c6a2ce7..d3a5cc7 100644
--- a/src/util/virauth.c
+++ b/src/util/virauth.c
@@ -41,10 +41,9 @@ int
virAuthGetConfigFilePathURI(virURIPtr uri,
char **path)
{
- int ret = -1;
size_t i;
const char *authenv = virGetEnvBlockSUID("LIBVIRT_AUTH_FILE");
- char *userdir = NULL;
+ VIR_AUTOFREE(char *) userdir = NULL;
*path = NULL;
@@ -53,7 +52,7 @@ virAuthGetConfigFilePathURI(virURIPtr uri,
if (authenv) {
VIR_DEBUG("Using path from env '%s'", authenv);
if (VIR_STRDUP(*path, authenv) < 0)
- goto cleanup;
+ return -1;
return 0;
}
@@ -63,17 +62,17 @@ virAuthGetConfigFilePathURI(virURIPtr uri,
uri->params[i].value) {
VIR_DEBUG("Using path from URI '%s'",
uri->params[i].value);
if (VIR_STRDUP(*path, uri->params[i].value) < 0)
- goto cleanup;
+ return -1;
return 0;
}
}
}
if (!(userdir = virGetUserConfigDirectory()))
- goto cleanup;
+ return -1;
if (virAsprintf(path, "%s/auth.conf", userdir) < 0)
- goto cleanup;
+ return -1;
VIR_DEBUG("Checking for readability of '%s'", *path);
if (access(*path, R_OK) == 0)
@@ -82,7 +81,7 @@ virAuthGetConfigFilePathURI(virURIPtr uri,
VIR_FREE(*path);
if (VIR_STRDUP(*path, SYSCONFDIR "/libvirt/auth.conf") < 0)
- goto cleanup;
+ return -1;
VIR_DEBUG("Checking for readability of '%s'", *path);
if (access(*path, R_OK) == 0)
@@ -91,13 +90,9 @@ virAuthGetConfigFilePathURI(virURIPtr uri,
VIR_FREE(*path);
done:
- ret = 0;
-
VIR_DEBUG("Using auth file '%s'", NULLSTR(*path));
- cleanup:
- VIR_FREE(userdir);
- return ret;
+ return 0;
}
@@ -155,7 +150,7 @@ virAuthGetUsernamePath(const char *path,
{
unsigned int ncred;
virConnectCredential cred;
- char *prompt;
+ VIR_AUTOFREE(char *) prompt = NULL;
char *ret = NULL;
if (virAuthGetCredential(servicename, hostname, "username", path, &ret)
< 0)
@@ -192,8 +187,6 @@ virAuthGetUsernamePath(const char *path,
break;
}
- VIR_FREE(prompt);
-
return cred.result;
}
@@ -205,18 +198,13 @@ virAuthGetUsername(virConnectPtr conn,
const char *defaultUsername,
const char *hostname)
{
- char *ret;
- char *path;
+ VIR_AUTOFREE(char *) path = NULL;
if (virAuthGetConfigFilePath(conn, &path) < 0)
return NULL;
- ret = virAuthGetUsernamePath(path, auth, servicename,
+ return virAuthGetUsernamePath(path, auth, servicename,
defaultUsername, hostname);
-
- VIR_FREE(path);
-
- return ret;
}
@@ -229,7 +217,7 @@ virAuthGetPasswordPath(const char *path,
{
unsigned int ncred;
virConnectCredential cred;
- char *prompt;
+ VIR_AUTOFREE(char *) prompt = NULL;
char *ret = NULL;
if (virAuthGetCredential(servicename, hostname, "password", path, &ret)
< 0)
@@ -263,8 +251,6 @@ virAuthGetPasswordPath(const char *path,
break;
}
- VIR_FREE(prompt);
-
return cred.result;
}
@@ -276,15 +262,10 @@ virAuthGetPassword(virConnectPtr conn,
const char *username,
const char *hostname)
{
- char *ret;
- char *path;
+ VIR_AUTOFREE(char *) path = NULL;
if (virAuthGetConfigFilePath(conn, &path) < 0)
return NULL;
- ret = virAuthGetPasswordPath(path, auth, servicename, username, hostname);
-
- VIR_FREE(path);
-
- return ret;
+ return virAuthGetPasswordPath(path, auth, servicename, username, hostname);
}
--
1.8.3.1
7:34 a.m.
New subject: [libvirt] [PATCH v3 20/35] util: auth: use VIR_AUTOFREE instead of VIR_FREE for scalar types
On Sat, Jun 30, 2018 at 02:30:24PM +0530, Sukrit Bhatnagar wrote:
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
4 a.m.
New subject: [libvirt] [PATCH v3 21/35] util: auth: use VIR_AUTOPTR for aggregate types
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOPTR macro for declaring aggregate pointer variables,
majority of the calls to *Free functions can be dropped, which
in turn leads to getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virauth.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/src/util/virauth.c b/src/util/virauth.c
index d3a5cc7..8c450b6 100644
--- a/src/util/virauth.c
+++ b/src/util/virauth.c
@@ -111,8 +111,7 @@ virAuthGetCredential(const char *servicename,
const char *path,
char **value)
{
- int ret = -1;
- virAuthConfigPtr config = NULL;
+ VIR_AUTOPTR(virAuthConfig) config = NULL;
const char *tmp;
*value = NULL;
@@ -121,23 +120,19 @@ virAuthGetCredential(const char *servicename,
return 0;
if (!(config = virAuthConfigNew(path)))
- goto cleanup;
+ return -1;
if (virAuthConfigLookup(config,
servicename,
hostname,
credname,
&tmp) < 0)
- goto cleanup;
+ return -1;
if (VIR_STRDUP(*value, tmp) < 0)
- goto cleanup;
-
- ret = 0;
+ return -1;
- cleanup:
- virAuthConfigFree(config);
- return ret;
+ return 0;
}
--
1.8.3.1
7:35 a.m.
New subject: [libvirt] [PATCH v3 21/35] util: auth: use VIR_AUTOPTR for aggregate types
On Sat, Jun 30, 2018 at 02:30:25PM +0530, Sukrit Bhatnagar wrote:
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOPTR macro for declaring aggregate pointer variables,
majority of the calls to *Free functions can be dropped, which
in turn leads to getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
4 a.m.
New subject: [libvirt] [PATCH v3 22/35] util: json: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in
src/util/viralloc.h, define a new wrapper around an existing
cleanup function which will be called when a variable declared
with VIR_AUTOPTR macro goes out of scope.
When a variable of type virJSONValuePtr is declared using
VIR_AUTOPTR, the function virJSONValueFree will be run
automatically on it when it goes out of scope.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virjson.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/util/virjson.h b/src/util/virjson.h
index e4a82bd..75f7f17 100644
--- a/src/util/virjson.h
+++ b/src/util/virjson.h
@@ -26,6 +26,7 @@
# include "internal.h"
# include "virbitmap.h"
+# include "viralloc.h"
# include <stdarg.h>
@@ -156,4 +157,6 @@ char *virJSONStringReformat(const char *jsonstr, bool pretty);
virJSONValuePtr virJSONValueObjectDeflatten(virJSONValuePtr json);
+VIR_DEFINE_AUTOPTR_FUNC(virJSONValue, virJSONValueFree)
+
#endif /* __VIR_JSON_H_ */
--
1.8.3.1
4 a.m.
New subject: [libvirt] [PATCH v3 23/35] util: json: remove redundant include directive
The include directive for viralloc.h is added in virjson.h
in a previous patch.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virjson.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 0559d40..92f3994 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -24,7 +24,6 @@
#include <config.h>
#include "virjson.h"
-#include "viralloc.h"
#include "virerror.h"
#include "virlog.h"
#include "virstring.h"
--
1.8.3.1
7:36 a.m.
New subject: [libvirt] [PATCH v3 23/35] util: json: remove redundant include directive
On Sat, Jun 30, 2018 at 02:30:27PM +0530, Sukrit Bhatnagar wrote:
The include directive for viralloc.h is added in virjson.h
in a previous patch.
Same as patch 9.
Reviewed-by: Erik Skultety <eskultet(a)redhat.com> (patches 22-23)
4 a.m.
New subject: [libvirt] [PATCH v3 24/35] util: json: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virjson.c | 49 ++++++++++++++-----------------------------------
1 file changed, 14 insertions(+), 35 deletions(-)
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 92f3994..82f539f 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -496,65 +496,50 @@ virJSONValueNewNumber(const char *data)
virJSONValuePtr
virJSONValueNewNumberInt(int data)
{
- virJSONValuePtr val = NULL;
- char *str;
+ VIR_AUTOFREE(char *) str = NULL;
if (virAsprintf(&str, "%i", data) < 0)
return NULL;
- val = virJSONValueNewNumber(str);
- VIR_FREE(str);
- return val;
+ return virJSONValueNewNumber(str);
}
virJSONValuePtr
virJSONValueNewNumberUint(unsigned int data)
{
- virJSONValuePtr val = NULL;
- char *str;
+ VIR_AUTOFREE(char *) str = NULL;
if (virAsprintf(&str, "%u", data) < 0)
return NULL;
- val = virJSONValueNewNumber(str);
- VIR_FREE(str);
- return val;
+ return virJSONValueNewNumber(str);
}
virJSONValuePtr
virJSONValueNewNumberLong(long long data)
{
- virJSONValuePtr val = NULL;
- char *str;
+ VIR_AUTOFREE(char *) str = NULL;
if (virAsprintf(&str, "%lld", data) < 0)
return NULL;
- val = virJSONValueNewNumber(str);
- VIR_FREE(str);
- return val;
+ return virJSONValueNewNumber(str);
}
virJSONValuePtr
virJSONValueNewNumberUlong(unsigned long long data)
{
- virJSONValuePtr val = NULL;
- char *str;
+ VIR_AUTOFREE(char *) str = NULL;
if (virAsprintf(&str, "%llu", data) < 0)
return NULL;
- val = virJSONValueNewNumber(str);
- VIR_FREE(str);
- return val;
+ return virJSONValueNewNumber(str);
}
virJSONValuePtr
virJSONValueNewNumberDouble(double data)
{
- virJSONValuePtr val = NULL;
- char *str;
+ VIR_AUTOFREE(char *) str = NULL;
if (virDoubleToStr(&str, data) < 0)
return NULL;
- val = virJSONValueNewNumber(str);
- VIR_FREE(str);
- return val;
+ return virJSONValueNewNumber(str);
}
@@ -1171,10 +1156,9 @@ int
virJSONValueGetArrayAsBitmap(const virJSONValue *val,
virBitmapPtr *bitmap)
{
- int ret = -1;
virJSONValuePtr elem;
size_t i;
- unsigned long long *elems = NULL;
+ VIR_AUTOFREE(unsigned long long *) elems = NULL;
unsigned long long maxelem = 0;
*bitmap = NULL;
@@ -1191,25 +1175,20 @@ virJSONValueGetArrayAsBitmap(const virJSONValue *val,
if (elem->type != VIR_JSON_TYPE_NUMBER ||
virStrToLong_ullp(elem->data.number, NULL, 10, &elems[i]) < 0)
- goto cleanup;
+ return -1;
if (elems[i] > maxelem)
maxelem = elems[i];
}
if (!(*bitmap = virBitmapNewQuiet(maxelem + 1)))
- goto cleanup;
+ return -1;
/* second pass sets the correct bits in the map */
for (i = 0; i < val->data.array.nvalues; i++)
ignore_value(virBitmapSetBit(*bitmap, elems[i]));
- ret = 0;
-
- cleanup:
- VIR_FREE(elems);
-
- return ret;
+ return 0;
}
--
1.8.3.1
4 a.m.
New subject: [libvirt] [PATCH v3 25/35] util: json: use VIR_AUTOPTR for aggregate types
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOPTR macro for declaring aggregate pointer variables,
majority of the calls to *Free functions can be dropped, which
in turn leads to getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virjson.c | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 82f539f..29530dc 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -1786,7 +1786,7 @@ virJSONValueFromString(const char *jsonstring)
size_t len = strlen(jsonstring);
# ifndef WITH_YAJL2
yajl_parser_config cfg = { 0, 1 }; /* Match yajl 2 default behavior */
- virJSONValuePtr tmp;
+ VIR_AUTOPTR(virJSONValue) tmp = NULL;
# endif
VIR_DEBUG("string=%s", jsonstring);
@@ -1850,7 +1850,6 @@ virJSONValueFromString(const char *jsonstring)
jsonstring);
else
ret = virJSONValueArraySteal(tmp, 0);
- virJSONValueFree(tmp);
# endif
}
@@ -2023,16 +2022,12 @@ char *
virJSONStringReformat(const char *jsonstr,
bool pretty)
{
- virJSONValuePtr json;
- char *ret;
+ VIR_AUTOPTR(virJSONValue) json = NULL;
if (!(json = virJSONValueFromString(jsonstr)))
return NULL;
- ret = virJSONValueToString(json, pretty);
-
- virJSONValueFree(json);
- return ret;
+ return virJSONValueToString(json, pretty);
}
@@ -2121,7 +2116,7 @@ virJSONValueObjectDeflattenWorker(const char *key,
virJSONValuePtr
virJSONValueObjectDeflatten(virJSONValuePtr json)
{
- virJSONValuePtr deflattened;
+ VIR_AUTOPTR(virJSONValue) deflattened = NULL;
virJSONValuePtr ret = NULL;
if (!(deflattened = virJSONValueNewObject()))
@@ -2130,12 +2125,9 @@ virJSONValueObjectDeflatten(virJSONValuePtr json)
if (virJSONValueObjectForeachKeyValue(json,
virJSONValueObjectDeflattenWorker,
deflattened) < 0)
- goto cleanup;
+ return NULL;
VIR_STEAL_PTR(ret, deflattened);
- cleanup:
- virJSONValueFree(deflattened);
-
return ret;
}
--
1.8.3.1
4 a.m.
New subject: [libvirt] [PATCH v3 26/35] util: bitmap: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in
src/util/viralloc.h, define a new wrapper around an existing
cleanup function which will be called when a variable declared
with VIR_AUTOPTR macro goes out of scope.
When a variable of type virBitmapPtr is declared using
VIR_AUTOPTR, the function virBitmapFree will be run
automatically on it when it goes out of scope.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virbitmap.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h
index 2464814..312e7e2 100644
--- a/src/util/virbitmap.h
+++ b/src/util/virbitmap.h
@@ -25,6 +25,7 @@
# define __BITMAP_H__
# include "internal.h"
+# include "viralloc.h"
# include <sys/types.h>
@@ -155,4 +156,6 @@ void virBitmapSubtract(virBitmapPtr a, virBitmapPtr b)
void virBitmapShrink(virBitmapPtr map, size_t b);
+VIR_DEFINE_AUTOPTR_FUNC(virBitmap, virBitmapFree)
+
#endif
--
1.8.3.1
7:39 a.m.
New subject: [libvirt] [PATCH v3 26/35] util: bitmap: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
On Sat, Jun 30, 2018 at 02:30:30PM +0530, Sukrit Bhatnagar wrote:
Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in
src/util/viralloc.h, define a new wrapper around an existing
cleanup function which will be called when a variable declared
with VIR_AUTOPTR macro goes out of scope.
When a variable of type virBitmapPtr is declared using
VIR_AUTOPTR, the function virBitmapFree will be run
automatically on it when it goes out of scope.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
patches 24-26:
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
4 a.m.
New subject: [libvirt] [PATCH v3 27/35] util: bitmap: remove redundant include directive
The include directive for viralloc.h is added in virbitmap.h
in a previous patch.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virbitmap.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index 0cc5292..ef18dad 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -31,7 +31,6 @@
#include <sys/types.h>
#include "virbitmap.h"
-#include "viralloc.h"
#include "virbuffer.h"
#include "c-ctype.h"
#include "count-one-bits.h"
--
1.8.3.1
7:39 a.m.
New subject: [libvirt] [PATCH v3 27/35] util: bitmap: remove redundant include directive
On Sat, Jun 30, 2018 at 02:30:31PM +0530, Sukrit Bhatnagar wrote:
The include directive for viralloc.h is added in virbitmap.h
in a previous patch.
Same as patch 9.
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
4 a.m.
New subject: [libvirt] [PATCH v3 28/35] util: bitmap: use VIR_AUTOPTR for aggregate types
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOPTR macro for declaring aggregate pointer variables,
majority of the calls to *Free functions can be dropped, which
in turn leads to getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virbitmap.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index ef18dad..5b6e55f 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -1202,15 +1202,12 @@ char *
virBitmapDataFormat(const void *data,
int len)
{
- virBitmapPtr map = NULL;
- char *ret = NULL;
+ VIR_AUTOPTR(virBitmap) map = NULL;
if (!(map = virBitmapNewData(data, len)))
return NULL;
- ret = virBitmapFormat(map);
- virBitmapFree(map);
- return ret;
+ return virBitmapFormat(map);
}
--
1.8.3.1
8:45 a.m.
New subject: [libvirt] [PATCH v3 28/35] util: bitmap: use VIR_AUTOPTR for aggregate types
On Sat, Jun 30, 2018 at 02:30:32PM +0530, Sukrit Bhatnagar wrote:
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOPTR macro for declaring aggregate pointer variables,
majority of the calls to *Free functions can be dropped, which
in turn leads to getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
4 a.m.
New subject: [libvirt] [PATCH v3 29/35] util: iohelper: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/iohelper.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/util/iohelper.c b/src/util/iohelper.c
index bb8a8dd..f7794dc 100644
--- a/src/util/iohelper.c
+++ b/src/util/iohelper.c
@@ -46,7 +46,7 @@
static int
runIO(const char *path, int fd, int oflags)
{
- void *base = NULL; /* Location to be freed */
+ VIR_AUTOFREE(void *) base = NULL; /* Location to be freed */
char *buf = NULL; /* Aligned location within base */
size_t buflen = 1024*1024;
intptr_t alignMask = 64*1024 - 1;
@@ -174,8 +174,6 @@ runIO(const char *path, int fd, int oflags)
virReportSystemError(errno, _("Unable to close %s"), path);
ret = -1;
}
-
- VIR_FREE(base);
return ret;
}
--
1.8.3.1
8:45 a.m.
New subject: [libvirt] [PATCH v3 29/35] util: iohelper: use VIR_AUTOFREE instead of VIR_FREE for scalar types
On Sat, Jun 30, 2018 at 02:30:33PM +0530, Sukrit Bhatnagar wrote:
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
4 a.m.
New subject: [libvirt] [PATCH v3 30/35] util: arptable: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virarptable.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/src/util/virarptable.c b/src/util/virarptable.c
index c0e90dc..bf40267 100644
--- a/src/util/virarptable.c
+++ b/src/util/virarptable.c
@@ -71,9 +71,8 @@ virArpTableGet(void)
{
int num = 0;
int msglen;
- void *nlData = NULL;
+ VIR_AUTOFREE(void *) nlData = NULL;
virArpTablePtr table = NULL;
- char *ipstr = NULL;
struct nlmsghdr* nh;
struct rtattr * tb[NDA_MAX+1];
@@ -89,6 +88,7 @@ virArpTableGet(void)
VIR_WARNINGS_NO_CAST_ALIGN
for (; NLMSG_OK(nh, msglen); nh = NLMSG_NEXT(nh, msglen)) {
VIR_WARNINGS_RESET
+ VIR_AUTOFREE(char *) ipstr = NULL;
struct ndmsg *r = NLMSG_DATA(nh);
int len = nh->nlmsg_len;
void *addr;
@@ -108,7 +108,7 @@ virArpTableGet(void)
continue;
if (nh->nlmsg_type == NLMSG_DONE)
- goto end_of_netlink_messages;
+ return table;
VIR_WARNINGS_NO_CAST_ALIGN
parse_rtattr(tb, NDA_MAX, NDA_RTA(r),
@@ -134,8 +134,6 @@ virArpTableGet(void)
if (VIR_STRDUP(table->t[num].ipaddr, ipstr) < 0)
goto cleanup;
-
- VIR_FREE(ipstr);
}
if (tb[NDA_LLADDR]) {
@@ -154,14 +152,8 @@ virArpTableGet(void)
}
}
- end_of_netlink_messages:
- VIR_FREE(nlData);
- return table;
-
cleanup:
virArpTableFree(table);
- VIR_FREE(ipstr);
- VIR_FREE(nlData);
return NULL;
}
--
1.8.3.1
8:45 a.m.
New subject: [libvirt] [PATCH v3 30/35] util: arptable: use VIR_AUTOFREE instead of VIR_FREE for scalar types
On Sat, Jun 30, 2018 at 02:30:34PM +0530, Sukrit Bhatnagar wrote:
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virarptable.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/src/util/virarptable.c b/src/util/virarptable.c
index c0e90dc..bf40267 100644
--- a/src/util/virarptable.c
+++ b/src/util/virarptable.c
@@ -71,9 +71,8 @@ virArpTableGet(void)
{
int num = 0;
int msglen;
- void *nlData = NULL;
+ VIR_AUTOFREE(void *) nlData = NULL;
virArpTablePtr table = NULL;
- char *ipstr = NULL;
struct nlmsghdr* nh;
struct rtattr * tb[NDA_MAX+1];
@@ -89,6 +88,7 @@ virArpTableGet(void)
VIR_WARNINGS_NO_CAST_ALIGN
for (; NLMSG_OK(nh, msglen); nh = NLMSG_NEXT(nh, msglen)) {
VIR_WARNINGS_RESET
+ VIR_AUTOFREE(char *) ipstr = NULL;
okay, but see below, ^this declaration can be moved further down
struct ndmsg *r = NLMSG_DATA(nh);
int len = nh->nlmsg_len;
void *addr;
@@ -108,7 +108,7 @@ virArpTableGet(void)
continue;
if (nh->nlmsg_type == NLMSG_DONE)
- goto end_of_netlink_messages;
+ return table;
VIR_WARNINGS_NO_CAST_ALIGN
parse_rtattr(tb, NDA_MAX, NDA_RTA(r),
@@ -134,8 +134,6 @@ virArpTableGet(void)
if (VIR_STRDUP(table->t[num].ipaddr, ipstr) < 0)
goto cleanup;
-
- VIR_FREE(ipstr);
ipstr is only used in ^this corresponding block, so you can define it in ^this
corresponding rather than the 'for' block.
With that:
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
4 a.m.
New subject: [libvirt] [PATCH v3 31/35] util: audit: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/viraudit.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/util/viraudit.c b/src/util/viraudit.c
index 0085dc3..a49d458 100644
--- a/src/util/viraudit.c
+++ b/src/util/viraudit.c
@@ -97,7 +97,7 @@ void virAuditSend(virLogSourcePtr source,
virAuditRecordType type ATTRIBUTE_UNUSED, bool success,
const char *fmt, ...)
{
- char *str = NULL;
+ VIR_AUTOFREE(char *) str = NULL;
va_list args;
/* Duplicate later checks, to short circuit & avoid printf overhead
@@ -144,7 +144,6 @@ void virAuditSend(virLogSourcePtr source,
}
}
#endif
- VIR_FREE(str);
}
void virAuditClose(void)
--
1.8.3.1
4 a.m.
New subject: [libvirt] [PATCH v3 32/35] util: fcp: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virfcp.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/src/util/virfcp.c b/src/util/virfcp.c
index 7660ba7..b703744 100644
--- a/src/util/virfcp.c
+++ b/src/util/virfcp.c
@@ -40,16 +40,12 @@
bool
virFCIsCapableRport(const char *rport)
{
- bool ret = false;
- char *path = NULL;
+ VIR_AUTOFREE(char *) path = NULL;
if (virBuildPath(&path, SYSFS_FC_RPORT_PATH, rport) < 0)
return false;
- ret = virFileExists(path);
- VIR_FREE(path);
-
- return ret;
+ return virFileExists(path);
}
int
@@ -57,8 +53,8 @@ virFCReadRportValue(const char *rport,
const char *entry,
char **result)
{
- int ret = -1;
- char *buf = NULL, *p = NULL;
+ VIR_AUTOFREE(char *) buf = NULL;
+ char *p = NULL;
if (virFileReadValueString(&buf, "%s/%s/%s",
SYSFS_FC_RPORT_PATH, rport, entry) < 0) {
@@ -69,13 +65,9 @@ virFCReadRportValue(const char *rport,
*p = '\0';
if (VIR_STRDUP(*result, buf) < 0)
- goto cleanup;
-
- ret = 0;
+ return -1;
- cleanup:
- VIR_FREE(buf);
- return ret;
+ return 0;
}
#else
--
1.8.3.1
4 a.m.
New subject: [libvirt] [PATCH v3 33/35] util: eventpoll: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/vireventpoll.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c
index 81ecab4..13d278d 100644
--- a/src/util/vireventpoll.c
+++ b/src/util/vireventpoll.c
@@ -618,7 +618,7 @@ static void virEventPollCleanupHandles(void)
*/
int virEventPollRunOnce(void)
{
- struct pollfd *fds = NULL;
+ VIR_AUTOFREE(struct pollfd *) fds = NULL;
int ret, timeout, nfds;
virMutexLock(&eventLoop.lock);
@@ -645,7 +645,7 @@ int virEventPollRunOnce(void)
goto retry;
virReportSystemError(errno, "%s",
_("Unable to poll on file handles"));
- goto error_unlocked;
+ return -1;
}
EVENT_DEBUG("Poll got %d event(s)", ret);
@@ -662,13 +662,10 @@ int virEventPollRunOnce(void)
eventLoop.running = 0;
virMutexUnlock(&eventLoop.lock);
- VIR_FREE(fds);
return 0;
error:
virMutexUnlock(&eventLoop.lock);
- error_unlocked:
- VIR_FREE(fds);
return -1;
}
--
1.8.3.1
4 a.m.
New subject: [libvirt] [PATCH v3 34/35] util: filecache: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virfilecache.c | 35 +++++++++++------------------------
1 file changed, 11 insertions(+), 24 deletions(-)
diff --git a/src/util/virfilecache.c b/src/util/virfilecache.c
index 96ae96d..2927c68 100644
--- a/src/util/virfilecache.c
+++ b/src/util/virfilecache.c
@@ -100,18 +100,17 @@ static char *
virFileCacheGetFileName(virFileCachePtr cache,
const char *name)
{
- char *file = NULL;
- char *namehash = NULL;
+ VIR_AUTOFREE(char *) namehash = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
if (virCryptoHashString(VIR_CRYPTO_HASH_SHA256, name, &namehash) < 0)
- goto cleanup;
+ return NULL;
if (virFileMakePath(cache->dir) < 0) {
virReportSystemError(errno,
_("Unable to create directory '%s'"),
cache->dir);
- goto cleanup;
+ return NULL;
}
virBufferAsprintf(&buf, "%s/%s", cache->dir, namehash);
@@ -120,13 +119,9 @@ virFileCacheGetFileName(virFileCachePtr cache,
virBufferAsprintf(&buf, ".%s", cache->suffix);
if (virBufferCheckError(&buf) < 0)
- goto cleanup;
-
- file = virBufferContentAndReset(&buf);
+ return NULL;
- cleanup:
- VIR_FREE(namehash);
- return file;
+ return virBufferContentAndReset(&buf);
}
@@ -135,7 +130,7 @@ virFileCacheLoad(virFileCachePtr cache,
const char *name,
void **data)
{
- char *file = NULL;
+ VIR_AUTOFREE(char *) file = NULL;
int ret = -1;
void *loadData = NULL;
@@ -178,7 +173,6 @@ virFileCacheLoad(virFileCachePtr cache,
cleanup:
virObjectUnref(loadData);
- VIR_FREE(file);
return ret;
}
@@ -188,20 +182,15 @@ virFileCacheSave(virFileCachePtr cache,
const char *name,
void *data)
{
- char *file = NULL;
- int ret = -1;
+ VIR_AUTOFREE(char *) file = NULL;
if (!(file = virFileCacheGetFileName(cache, name)))
- return ret;
+ return -1;
if (cache->handlers.saveFile(data, file, cache->priv) < 0)
- goto cleanup;
-
- ret = 0;
+ return -1;
- cleanup:
- VIR_FREE(file);
- return ret;
+ return 0;
}
@@ -346,7 +335,7 @@ virFileCacheLookupByFunc(virFileCachePtr cache,
const void *iterData)
{
void *data = NULL;
- char *name = NULL;
+ VIR_AUTOFREE(char *) name = NULL;
virObjectLock(cache);
@@ -356,8 +345,6 @@ virFileCacheLookupByFunc(virFileCachePtr cache,
virObjectRef(data);
virObjectUnlock(cache);
- VIR_FREE(name);
-
return data;
}
--
1.8.3.1
4 a.m.
New subject: [libvirt] [PATCH v3 35/35] util: identity: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/viridentity.c | 52 +++++++++++++++++++++-----------------------------
1 file changed, 22 insertions(+), 30 deletions(-)
diff --git a/src/util/viridentity.c b/src/util/viridentity.c
index 2f4307b..c621444 100644
--- a/src/util/viridentity.c
+++ b/src/util/viridentity.c
@@ -133,8 +133,8 @@ int virIdentitySetCurrent(virIdentityPtr ident)
*/
virIdentityPtr virIdentityGetSystem(void)
{
- char *username = NULL;
- char *groupname = NULL;
+ VIR_AUTOFREE(char *) username = NULL;
+ VIR_AUTOFREE(char *) groupname = NULL;
unsigned long long startTime;
virIdentityPtr ret = NULL;
#if WITH_SELINUX
@@ -154,14 +154,14 @@ virIdentityPtr virIdentityGetSystem(void)
goto error;
if (!(username = virGetUserName(geteuid())))
- goto cleanup;
+ return ret;
if (virIdentitySetUNIXUserName(ret, username) < 0)
goto error;
if (virIdentitySetUNIXUserID(ret, getuid()) < 0)
goto error;
if (!(groupname = virGetGroupName(getegid())))
- goto cleanup;
+ return ret;
if (virIdentitySetUNIXGroupName(ret, groupname) < 0)
goto error;
if (virIdentitySetUNIXGroupID(ret, getgid()) < 0)
@@ -172,7 +172,7 @@ virIdentityPtr virIdentityGetSystem(void)
if (getcon(&con) < 0) {
virReportSystemError(errno, "%s",
_("Unable to lookup SELinux process
context"));
- goto cleanup;
+ return ret;
}
if (virIdentitySetSELinuxContext(ret, con) < 0) {
freecon(con);
@@ -182,15 +182,11 @@ virIdentityPtr virIdentityGetSystem(void)
}
#endif
- cleanup:
- VIR_FREE(username);
- VIR_FREE(groupname);
return ret;
error:
virObjectUnref(ret);
- ret = NULL;
- goto cleanup;
+ return NULL;
}
@@ -461,15 +457,14 @@ int virIdentitySetUNIXUserName(virIdentityPtr ident,
int virIdentitySetUNIXUserID(virIdentityPtr ident,
uid_t uid)
{
- char *val;
- int ret;
+ VIR_AUTOFREE(char *) val = NULL;
+
if (virAsprintf(&val, "%d", (int)uid) < 0)
return -1;
- ret = virIdentitySetAttr(ident,
+
+ return virIdentitySetAttr(ident,
VIR_IDENTITY_ATTR_UNIX_USER_ID,
val);
- VIR_FREE(val);
- return ret;
}
@@ -485,45 +480,42 @@ int virIdentitySetUNIXGroupName(virIdentityPtr ident,
int virIdentitySetUNIXGroupID(virIdentityPtr ident,
gid_t gid)
{
- char *val;
- int ret;
+ VIR_AUTOFREE(char *) val = NULL;
+
if (virAsprintf(&val, "%d", (int)gid) < 0)
return -1;
- ret = virIdentitySetAttr(ident,
+
+ return virIdentitySetAttr(ident,
VIR_IDENTITY_ATTR_UNIX_GROUP_ID,
val);
- VIR_FREE(val);
- return ret;
}
int virIdentitySetUNIXProcessID(virIdentityPtr ident,
pid_t pid)
{
- char *val;
- int ret;
+ VIR_AUTOFREE(char *) val = NULL;
+
if (virAsprintf(&val, "%lld", (long long) pid) < 0)
return -1;
- ret = virIdentitySetAttr(ident,
+
+ return virIdentitySetAttr(ident,
VIR_IDENTITY_ATTR_UNIX_PROCESS_ID,
val);
- VIR_FREE(val);
- return ret;
}
int virIdentitySetUNIXProcessTime(virIdentityPtr ident,
unsigned long long timestamp)
{
- char *val;
- int ret;
+ VIR_AUTOFREE(char *) val = NULL;
+
if (virAsprintf(&val, "%llu", timestamp) < 0)
return -1;
- ret = virIdentitySetAttr(ident,
+
+ return virIdentitySetAttr(ident,
VIR_IDENTITY_ATTR_UNIX_PROCESS_TIME,
val);
- VIR_FREE(val);
- return ret;
}
--
1.8.3.1
8:48 a.m.
New subject: [libvirt] [PATCH v3 35/35] util: identity: use VIR_AUTOFREE instead of VIR_FREE for scalar types
On Sat, Jun 30, 2018 at 02:30:39PM +0530, Sukrit Bhatnagar wrote:
By making use of GNU C's cleanup attribute handled by the
VIR_AUTOFREE macro for declaring scalar variables, majority
of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
patches 31-35:
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
2324
days inactive
2337
days old
69 comments
3 participants
participants (3)
-
Erik Skultety -
Pavel Hrdina -
Sukrit Bhatnagar