Re: [libvirt] [GSoC] Design ideas for implementing cleanup attribute
On 23 May 2018 at 21:35, Pavel Hrdina wrote:
> On Sun, Mar 25, 2018 at 01:55:07AM +0530, Sukrit Bhatnagar wrote:
>> Hi,
>>
>> I am interested in implementing the GCC cleanup attribute for automatic
>> resource freeing as part of GSoC'18. I have shared a proposal for the same.
>>
>> This mail is to discuss the code design for implementing it.
>>
>>
>> Here are some of my ideas:
>>
>> This attribute requires a cleanup function that is called automatically
>> when the corresponding variable goes out of scope. There are some functions
>> whose logic can be reused:
>>
>> - Functions such as virCommandFree, virConfFreeList and virCgroupFree can
>> be directly used as cleanup functions. They have parameter and return type
>> valid for a cleanup function.
>>
>> - Functions such as virFileClose and virFileFclose need some additional
>> consideration as they return a value. I think we can set some global
>> variable in a separate source file (just like errno variable from errno.h).
>> Then the value to be returned can be accessed globally.
>>
>> - Functions such as virDomainEventGraphicsDispose need an entirely new
>> design. They are used as callbacks in object classes and passed as an
>> argument in virClassNew. This would require making changes to
>> virObjectUnref's code too. *This is the part I am not sure how to implement
>> cleanup logic for.*
>>
>>
>> Also, since the __attribute__((__cleanup__(anyfunc))) looks ugly, a macro
>> like autoclean (ideas for macro name welcome!) can be used instead. As
>> Martin pointed out in my proposal, for some types, this can be done right
>> after typedef declarations, so that the type itself contains this attribute.
>>
>> Basically, at most places where VIR_FREE is used to release memory
>> explicitly, the corresponding variable can use the attribute. The existing
>> virFree function also can be reused as it takes void pointer as an argument
>> and returns nothing.
>> One of the exceptions to this will be those variables which are struct
>> members. The cleanup of member has to be done when the enclosing struct
>> variable is cleaned.
>>
>> I can create new files vircleanup.{c,h} for defining cleanup functions for
>> types which do not have an existing cleanup/free function. This can be done
>> separately for each driver supported.
>> For example, cleanups pertaining to lxc driver will be in
>> src/lxc/lxc_cleanup.c.
>
> Hi,
>
> I would like to apologize that it took me that long to reply.
>
> We've already discussed this a little bit off-list so I would like to
> summarize my idea about the design of this effort.
>
> I liked the way how GLib is solving the issue so we can simply use the
> same approach since it looks reasonable.
>
> There would be three different macros that would be used to annotate
> variable with attribute cleanup:
>
> VIR_AUTOFREE char *str = NULL;
>
> - this would call virFree on that variable
This seems fine for basic native datatypes.
But for the complex types, why can't we do something like the following, as was
discussed previously?
* Define simple macros for the attribute per struct type:
#define VIRCOMMAND_AUTOFREE __attribute__((__cleanup__(freefunction))
#define VIRCOMMAND_AUTOCLEAN __attribute__((__cleanup__(cleanfunction))
* Create new datatypes which include this attribute:
#define virCommandAutoFreePtr VIRCOMMAND_AUTOFREE virCommandPtr
#define virCommandAutoCleanPtr VIRCOMMAND_AUTOCLEAN virCommandPtr
* Then simply declare variables as:
virCommandAutoFreePtr cmd = NULL;
We just have to make sure that all Ptr variables are initialized,
atleast to NULL.
Also, we can create and use macros to initialize the variables:
#define CREATE_CMD_PTR(var, val) \
VIRCOMMAND_AUTOFREE virCommandPtr var = (val);
CREATE_CMD_PTR(cmd, virCommandNewArgs(args))
This is equivalent to:
VIRCOMMAND_AUTOFREE virCommandPtr cmd = virCommandNewArgs(args);
Many existing Free functions such as virDomainFree, return a value
which is not allowed for a function that is to be used with cleanup attribute.
Do we ignore all such return values?
> VIR_AUTOPTR(virDomain) domain = NULL;
>
> - this would call registered free function on that variable
> - to register the free function you would use:
>
> VIR_DEFINE_AUTOPTR_FUNC(virDomain, virDomainFree);
>
> VIR_AUTOCLEAR(virDomain) domain = { 0 };
>
> - this would call registered clear function to free the content of
> that structure
> - to register that clear function you would use:
>
> VIR_DEFINE_AUTOCLEAR_FUNC(virDomain, virDomainClear);
>
> In GLib there are three
Re: [libvirt] [GSoC] Design ideas for implementing cleanup attribute
On 25 May 2018 at 16:20, Pavel Hrdina wrote:
> On Fri, May 25, 2018 at 12:06:50PM +0200, Andrea Bolognani wrote:
>> On Fri, 2018-05-25 at 10:04 +0200, Pavel Hrdina wrote:
>> > On Fri, May 25, 2018 at 09:13:51AM +0200, Andrea Bolognani wrote:
>> > > However, I realize it might not be possible to register free
>> > > functions for a native type without having to introduce something
>> > > like
>> > >
>> > > typedef char * virString;
>> > >
>> > > thus causing massive churn. How does GLib deal with that?
>> >
>> > If you would look into GLib documentation you would see that this
>> > design basically copies the one in GLib:
>>
>> Sorry, I should have looked up the documentation and implementation
>> before asking silly questions. Guess the morning coffee hadn't quite
>> kicked in yet :/
>>
>> > GLiblibvirt
>> >
>> > g_autofree VIR_AUTOFREE
>> > g_autoptr VIR_AUTOPTR
>> > g_auto VIR_AUTOCLEAR
>>
>> For what it's worth, I think VIR_AUTOCLEAR is a much better name
>> than g_auto :)
>>
>> > In GLib you are using them like this:
>> >
>> > g_autofree char *string = NULL;
>> > g_autoptr(virDomain) dom = NULL;
>> > g_auto(virDomain) dom = { 0 };
>> >
>> > So yes it would require to introduce a lot of typedefs for basic types
>> > and that is not worth it.
>>
>> I'm not sure we would need so many typedefs, but there would
>> certainly be a lot of churn involved.
>>
>> Personally, I'm not so sure it wouldn't be worth the effort,
>> but it's definitely something that we can experiment with it at
>> a later time instead of holding up what's already a pretty
>> significant improvement.
>>
>> > In libvirt we would have:
>> >
>> > VIR_AUTOFREE char *string = NULL;
>> > VIR_AUTOPTR(virDomainPtr) dom = NULL;
>> > VIR_AUTOCLEAR(virDomain) dom = { 0 };
>> >
>> > If you notice the difference, in libvirt we can use virDomainPtr
>> > directly because we have these typedefs, in GLib macro
>> > G_DEFINE_AUTOPTR_CLEANUP_FUNC creates similar typedef.
>>
>> While I'm not a fan of our *Ptr typedefs in general, I guess this
>> time I'm glad we have them because VIR_AUTOPTR() doesn't hide the
>> fact that what you're declaring is a pointer; that is, the macro
>> argument is also exactly the type of the variable.
>
> So let's make a summary of how it could look like:
>
> VIR_AUTOFREE(char *) string = NULL;
> VIR_AUTOPTR(virDomainPtr) vm = NULL;
> VIR_AUTOCLEAR(virDomain) dom = { 0 };
>
> VIR_DEFINE_AUTOFREE_FUNC(virDomainPtr, virDomainFree);
> VIR_DEFINE_AUTOCLEAR_FUNC(virDomain, virDomainClear);
Do we define new functions for freeing/clearing, because that is what
VIR_DEFINE_AUTOFREE_FUNC seems to do.
This is what new macros will look like:
# define _VIR_TYPE_PTR(type) type##Ptr
# define _VIR_ATTR_AUTOFREE_PTR(type) __attribute__((cleanup(type##Free)))
# define _VIR_ATTR_AUTOCLOSE_PTR(type) __attribute__((cleanup(type##Close)))
# define _VIR_ATTR_AUTOCLEAN_PTR(type) __attribute__((cleanup(type##Clean)))
# define VIR_AUTOFREE_PTR(type) _VIR_ATTR_AUTOFREE_PTR(type) _VIR_TYPE_PTR(type)
The problem is that our vir*Free functions take on vir*Ptr as the
parameter and not
vir*Ptr * (pointer to it).
For example, instead of:
void virArpTableFree(virArpTablePtr table);
we would need:
void virArpTableFree(virArpTablePtr *table);
if we declare something like:
VIR_AUTOFREE_PTR(virArpTable) table = NULL;
Also, I tried to add a new function:
void virArpTablePtrFree(virArpTablePtr *table)
{
size_t i;
if (!*table)
return;
for (i = 0; i < (*table)->n; i++) {
VIR_FREE((*table)->t[i].ipaddr);
VIR_FREE((*table)->t[i].mac);
}
VIR_FREE((*table)->t);
VIR_FREE((*table));
VIR_FREE(table);
}
but I am getting the errors:
*** Error in `/home/skrtbhtngr/libvirt/tests/.libs/lt-virbuftest':
free(): invalid pointer: 0x7ffc7be60d48 ***
...
*** Error in `/home/skrtbhtngr/libvirt/tests/.libs/lt-commandtest':
free(): invalid pointer: 0x7fff727583fc ***
...
I am not quite sure how to debug this. Am I missing something basic?
>
> Note: don't take the types and function names as something that actually
> exists and be used like that, it's just an example to show how it would
> work :).
>
> Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [GSoC] Design ideas for implementing cleanup attribute
On 28 May 2018 at 13:54, Pavel Hrdina wrote:
> On Mon, May 28, 2018 at 01:04:28PM +0530, Sukrit Bhatnagar wrote:
>> On 25 May 2018 at 16:20, Pavel Hrdina wrote:
>> > On Fri, May 25, 2018 at 12:06:50PM +0200, Andrea Bolognani wrote:
>> >> On Fri, 2018-05-25 at 10:04 +0200, Pavel Hrdina wrote:
>> >> > On Fri, May 25, 2018 at 09:13:51AM +0200, Andrea Bolognani wrote:
>> >> > > However, I realize it might not be possible to register free
>> >> > > functions for a native type without having to introduce something
>> >> > > like
>> >> > >
>> >> > > typedef char * virString;
>> >> > >
>> >> > > thus causing massive churn. How does GLib deal with that?
>> >> >
>> >> > If you would look into GLib documentation you would see that this
>> >> > design basically copies the one in GLib:
>> >>
>> >> Sorry, I should have looked up the documentation and implementation
>> >> before asking silly questions. Guess the morning coffee hadn't quite
>> >> kicked in yet :/
>> >>
>> >> > GLiblibvirt
>> >> >
>> >> > g_autofree VIR_AUTOFREE
>> >> > g_autoptr VIR_AUTOPTR
>> >> > g_auto VIR_AUTOCLEAR
>> >>
>> >> For what it's worth, I think VIR_AUTOCLEAR is a much better name
>> >> than g_auto :)
>> >>
>> >> > In GLib you are using them like this:
>> >> >
>> >> > g_autofree char *string = NULL;
>> >> > g_autoptr(virDomain) dom = NULL;
>> >> > g_auto(virDomain) dom = { 0 };
>> >> >
>> >> > So yes it would require to introduce a lot of typedefs for basic types
>> >> > and that is not worth it.
>> >>
>> >> I'm not sure we would need so many typedefs, but there would
>> >> certainly be a lot of churn involved.
>> >>
>> >> Personally, I'm not so sure it wouldn't be worth the effort,
>> >> but it's definitely something that we can experiment with it at
>> >> a later time instead of holding up what's already a pretty
>> >> significant improvement.
>> >>
>> >> > In libvirt we would have:
>> >> >
>> >> > VIR_AUTOFREE char *string = NULL;
>> >> > VIR_AUTOPTR(virDomainPtr) dom = NULL;
>> >> > VIR_AUTOCLEAR(virDomain) dom = { 0 };
>> >> >
>> >> > If you notice the difference, in libvirt we can use virDomainPtr
>> >> > directly because we have these typedefs, in GLib macro
>> >> > G_DEFINE_AUTOPTR_CLEANUP_FUNC creates similar typedef.
>> >>
>> >> While I'm not a fan of our *Ptr typedefs in general, I guess this
>> >> time I'm glad we have them because VIR_AUTOPTR() doesn't hide the
>> >> fact that what you're declaring is a pointer; that is, the macro
>> >> argument is also exactly the type of the variable.
>> >
>> > So let's make a summary of how it could look like:
>> >
>> > VIR_AUTOFREE(char *) string = NULL;
>> > VIR_AUTOPTR(virDomainPtr) vm = NULL;
>> > VIR_AUTOCLEAR(virDomain) dom = { 0 };
>> >
>> > VIR_DEFINE_AUTOFREE_FUNC(virDomainPtr, virDomainFree);
>> > VIR_DEFINE_AUTOCLEAR_FUNC(virDomain, virDomainClear);
>>
>> Do we define new functions for freeing/clearing, because that is what
>> VIR_DEFINE_AUTOFREE_FUNC seems to do.
>>
>>
>> This is what new macros will look like:
>>
>> # define _VIR_TYPE_PTR(type) type##Ptr
>>
>> # define _VIR_ATTR_AUTOFREE_PTR(type) __attribute__((cleanup(type##Free)))
>> # define _VIR_ATTR_AUTOCLOSE_PTR(type) __attribute__((cleanup(type##Close)))
>> # define _VIR_ATTR_AUTOCLEAN_PTR(type) __attribute__((cleanup(type##Clean)))
>>
>> # define VIR_AUTOFREE_PTR(type) _VIR_ATTR_AUTOFREE_PTR(type)
>> _VIR_TYPE_PTR(type)
>
> NACK to this, please look few lines above how the macros should be named
> and which macros we would like to have implemented.
>
> There is no need to have the _VIR* helper macros and you need to
> implement the VIR_DEFINE_* macros.
>
>> The problem is that our vir*Free functions take on vir*Ptr as the
>> parameter and not
>> vir*Ptr * (pointer to it).
>
> The problem is only with your current implementation, the original
> design should not have this
Re: [libvirt] [GSoC] Design ideas for implementing cleanup attribute
On 29 May 2018 at 10:36, Michal Privoznik wrote:
> On 05/28/2018 09:34 AM, Sukrit Bhatnagar wrote:
>>
>> This is what new macros will look like:
>>
>> # define _VIR_TYPE_PTR(type) type##Ptr
>>
>> # define _VIR_ATTR_AUTOFREE_PTR(type) __attribute__((cleanup(type##Free)))
>> # define _VIR_ATTR_AUTOCLOSE_PTR(type) __attribute__((cleanup(type##Close)))
>> # define _VIR_ATTR_AUTOCLEAN_PTR(type) __attribute__((cleanup(type##Clean)))
>>
>> # define VIR_AUTOFREE_PTR(type) _VIR_ATTR_AUTOFREE_PTR(type)
>> _VIR_TYPE_PTR(type)
>>
>>
>> The problem is that our vir*Free functions take on vir*Ptr as the
>> parameter and not
>> vir*Ptr * (pointer to it).
>>
>> For example, instead of:
>> void virArpTableFree(virArpTablePtr table);
>>
>> we would need:
>> void virArpTableFree(virArpTablePtr *table);
>>
>> if we declare something like:
>> VIR_AUTOFREE_PTR(virArpTable) table = NULL;
>>
>>
>> Also, I tried to add a new function:
>> void virArpTablePtrFree(virArpTablePtr *table)
>> {
>> size_t i;
>>
>> if (!*table)
>> return;
>>
>> for (i = 0; i < (*table)->n; i++) {
>> VIR_FREE((*table)->t[i].ipaddr);
>> VIR_FREE((*table)->t[i].mac);
>> }
>> VIR_FREE((*table)->t);
>> VIR_FREE((*table));
>> VIR_FREE(table);
>
> As Erik pointed out, this last VIR_FREE(table) looks fishy. However, do
> you have patch that I can apply and reproduce?
When I was using the above function, even without the last VIR_FREE(table),
I got the same error.
I am now using the original virArpTableFree function, not this.
The error is resolved for now. It seems like some other files I modified were
creating trouble. I reset them.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC 1/4] add macros for implementing automatic cleanup functionality
New macros are added to src/util/viralloc.h which help in
adding cleanup attribute to variable declarations.
Signed-off-by: Sukrit Bhatnagar
---
src/util/viralloc.h | 69 +
1 file changed, 69 insertions(+)
diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 69d0f90..e1c31c3 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -596,4 +596,73 @@ void virAllocTestInit(void);
int virAllocTestCount(void);
void virAllocTestOOM(int n, int m);
void virAllocTestHook(void (*func)(int, void*), void *data);
+
+# define VIR_AUTOPTR_FUNC_NAME(type) virAutoPtr##type
+# define VIR_AUTOCLEAR_FUNC_NAME(type) virAutoClear##type
+
+/**
+ * VIR_DEFINE_AUTOPTR_FUNC:
+ * @type: type of the variables(s) to free automatically
+ * @func: cleanup function to be automatically called
+ *
+ * Thos macro defines a function for automatically freeing
+ * resources allocated to a variable of type @type. The newly
+ * defined function calls the corresponding pre-defined
+ * function @func.
+ */
+# define VIR_DEFINE_AUTOPTR_FUNC(type, func) \
+static inline void VIR_AUTOPTR_FUNC_NAME(type)(type *_ptr) \
+{ \
+(func)(*_ptr); \
+} \
+
+/**
+ * VIR_DEFINE_AUTOCLEAR_FUNC:
+ * @type: type of the variables(s) to free automatically
+ * @func: cleanup function to be automatically called
+ *
+ * This macro defines a function for automatically clearing
+ * a variable of type @type. The newly deifined function calls
+ * the corresponding pre-defined function @func.
+ */
+# define VIR_DEFINE_AUTOCLEAR_FUNC(type, func) \
+static inline void VIR_AUTOCLEAR_FUNC_NAME(type)(type *_ptr) \
+{ \
+(func)(_ptr); \
+} \
+
+/**
+ * VIR_AUTOFREE:
+ * @type: type of the variables(s) to free automatically
+ *
+ * Macro to automatically free the memory allocated to
+ * the variable(s) 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 variables(s) to free automatically
+ *
+ * Macro to automatically free the memory allocated to
+ * the variable(s) 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 type
+
+/**
+ * VIR_AUTOCLEAR:
+ * @type: type of the variables(s) to free automatically
+ *
+ * Macro to automatically clear the variable(s) declared
+ * with it by calling the function defined by
+ * VIR_DEFINE_AUTOCLEAR_FUNC when the variabl* goes out
+ * of scope.
+ */
+# define VIR_AUTOCLEAR(type) \
+__attribute__((cleanup(VIR_AUTOCLEAR_FUNC_NAME(type type
+
#endif /* __VIR_MEMORY_H_ */
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC 0/4] add automatic cleanup functionality in some files
This series of patches aim at augmenting our discussion about the design for implementing the cleanup attribute. A set of macros have been added at the end of viralloc.h A few files have been modified to use the newly created macros. Sukrit Bhatnagar (4): add macros for implementing automatic cleanup functionality add automatic cleanup support in src/util/virarptable.c add automatic cleanup support in src/util/virauthconfig.c add automatic cleanup support in src/util/virauth.c src/util/viralloc.h | 69 src/util/virarptable.c | 9 ++- src/util/virauth.c | 66 + src/util/virauthconfig.c | 34 +--- src/util/virauthconfig.h | 3 +++ 5 files changed, 110 insertions(+), 71 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC 3/4] add automatic cleanup support in src/util/virauthconfig.c
Modifiy code to use cleanup macros where required.
Signed-off-by: Sukrit Bhatnagar
---
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 91c9c0c..66f7f7e 100644
--- a/src/util/virauthconfig.c
+++ b/src/util/virauthconfig.c
@@ -106,10 +106,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;
@@ -119,47 +118,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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC 4/4] add automatic cleanup support in src/util/virauth.c
Define a new cleanup function for virAuthConfigPtr in
src/util/virauthconfig.h.
Modifiy code to use cleanup macros where required.
Signed-off-by: Sukrit Bhatnagar
---
src/util/virauth.c | 66 ++--
src/util/virauthconfig.h | 3 +++
2 files changed, 27 insertions(+), 42 deletions(-)
diff --git a/src/util/virauth.c b/src/util/virauth.c
index adb093e..aa7593c 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"
@@ -42,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;
@@ -54,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;
}
@@ -64,41 +62,38 @@ 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)
-goto done;
+if (access(*path, R_OK) == 0) {
+VIR_DEBUG("Using auth file '%s'", NULLSTR(*path));
+return 0;
+}
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)
-goto done;
+if (access(*path, R_OK) == 0) {
+VIR_DEBUG("Using auth file '%s'", NULLSTR(*path));
+return 0;
+}
VIR_FREE(*path);
- done:
-ret = 0;
-
-VIR_DEBUG("Using auth file '%s'", NULLSTR(*path));
- cleanup:
-VIR_FREE(userdir);
-
-return ret;
+return 0;
}
@@ -117,8 +112,7 @@ virAuthGetCredential(const char *servicename,
const char *path,
char **value)
{
-int ret = -1;
-virAuthConfigPtr config = NULL;
+VIR_AUTOPTR(virAuthConfigPtr) config = NULL;
const char *tmp;
*value = NULL;
@@ -127,23 +121,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;
+return -1;
-ret = 0;
-
- cleanup:
-virAuthConfigFree(config);
-return ret;
+return 0;
}
@@ -156,7 +146,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)
@@ -193,8 +183,6 @@ virAuthGetUsernamePath(const char *path,
break;
}
-VIR_FREE(prompt);
-
return cred.result;
}
@@ -207,7 +195,7 @@ virAuthGetUsername(virConnectPtr conn,
const char *hostname)
{
char *ret;
-char *path;
+VIR_AUTOFREE(char *) path = NULL;
if (virAuthGetConfigFilePath(conn, &path) < 0)
return NULL;
@@ -215,8 +203,6 @@ virAuthGetUsername(virConnectPtr conn,
ret = virAuthGetUsernamePath(path, auth, servicename,
defaultUsername, hostname);
-VIR_FREE(path);
-
return ret;
}
@@ -230,7 +216,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)
@@ -264,8 +250,6 @@ virAut
[libvirt] [RFC 2/4] add automatic cleanup support in src/util/virarptable.c
Modifiy code to use cleanup macros where required.
Signed-off-by: Sukrit Bhatnagar
---
src/util/virarptable.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/src/util/virarptable.c b/src/util/virarptable.c
index c0e90dc..f53a479 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;
@@ -134,8 +134,6 @@ virArpTableGet(void)
if (VIR_STRDUP(table->t[num].ipaddr, ipstr) < 0)
goto cleanup;
-
-VIR_FREE(ipstr);
}
if (tb[NDA_LLADDR]) {
@@ -155,13 +153,10 @@ 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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 00/18] use VIR_AUTOFREE in src/util
This series of patches modifies some files in src/util to use VIR_AUTOFREE for automatic freeing of memory and get rid of some VIR_FREE macro invocations. Sukrit Bhatnagar (18): add macros for implementing automatic cleanup functionality use VIR_AUTOFREE in src/util/virarptable.c use VIR_AUTOFREE in src/util/virauth.c use VIR_AUTOFREE in src/util/virauthconfig.c use VIR_AUTOFREE in src/util/iohelper.c use VIR_AUTOFREE in src/util/viraudit.c use VIR_AUTOFREE in src/util/virbuffer.c use VIR_AUTOFREE in src/util/vircgroup.c use VIR_AUTOFREE in src/util/virfcp.c use VIR_AUTOFREE in src/util/virdnsmasq.c use VIR_AUTOFREE in src/util/vireventpoll.c use VIR_AUTOFREE in src/util/virdbus.c use VIR_AUTOFREE in src/util/virfdstream.c use VIR_AUTOFREE in src/util/virfile.c use VIR_AUTOFREE in src/util/virconf.c use VIR_AUTOFREE in src/util/virfilecache.c use VIR_AUTOFREE in src/util/virfirewall.c use VIR_AUTOFREE in src/util/virhook.c src/util/iohelper.c | 4 +- src/util/viralloc.h | 69 +++ src/util/virarptable.c | 9 +- src/util/viraudit.c | 3 +- src/util/virauth.c | 60 ++ src/util/virauthconfig.c | 34 ++- src/util/virbuffer.c | 33 ++- src/util/vircgroup.c | 526 --- src/util/virconf.c | 42 ++-- src/util/virdbus.c | 28 +-- src/util/virdnsmasq.c| 116 --- src/util/vireventpoll.c | 7 +- src/util/virfcp.c| 20 +- src/util/virfdstream.c | 3 +- src/util/virfile.c | 303 +-- src/util/virfilecache.c | 35 +--- src/util/virfirewall.c | 13 +- src/util/virhook.c | 16 +- 18 files changed, 487 insertions(+), 834 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 02/18] use VIR_AUTOFREE in src/util/virarptable.c
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar
---
src/util/virarptable.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/src/util/virarptable.c b/src/util/virarptable.c
index c0e90dc..f53a479 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;
@@ -134,8 +134,6 @@ virArpTableGet(void)
if (VIR_STRDUP(table->t[num].ipaddr, ipstr) < 0)
goto cleanup;
-
-VIR_FREE(ipstr);
}
if (tb[NDA_LLADDR]) {
@@ -155,13 +153,10 @@ 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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 01/18] add macros for implementing automatic cleanup functionality
New macros are added to src/util/viralloc.h which help in
adding cleanup attribute to variable declarations.
Signed-off-by: Sukrit Bhatnagar
---
src/util/viralloc.h | 69 +
1 file changed, 69 insertions(+)
diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 69d0f90..bb37c48 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -596,4 +596,73 @@ void virAllocTestInit(void);
int virAllocTestCount(void);
void virAllocTestOOM(int n, int m);
void virAllocTestHook(void (*func)(int, void*), void *data);
+
+# define VIR_AUTOPTR_FUNC_NAME(type) virAutoPtr##type
+# define VIR_AUTOCLEAR_FUNC_NAME(type) virAutoClear##type
+
+/**
+ * VIR_DEFINE_AUTOPTR_FUNC:
+ * @type: type of the variables(s) to free 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. The newly
+ * defined function calls the corresponding pre-defined
+ * function @func.
+ */
+# define VIR_DEFINE_AUTOPTR_FUNC(type, func) \
+static inline void VIR_AUTOPTR_FUNC_NAME(type)(type *_ptr) \
+{ \
+(func)(*_ptr); \
+} \
+
+/**
+ * VIR_DEFINE_AUTOCLEAR_FUNC:
+ * @type: type of the variables(s) to free automatically
+ * @func: cleanup function to be automatically called
+ *
+ * This macro defines a function for automatic clearing of
+ * a variable of type @type. The newly deifined function calls
+ * the corresponding pre-defined function @func.
+ */
+# define VIR_DEFINE_AUTOCLEAR_FUNC(type, func) \
+static inline void VIR_AUTOCLEAR_FUNC_NAME(type)(type *_ptr) \
+{ \
+(func)(_ptr); \
+} \
+
+/**
+ * VIR_AUTOFREE:
+ * @type: type of the variables(s) to free automatically
+ *
+ * Macro to automatically free the memory allocated to
+ * the variable(s) 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 variables(s) to free automatically
+ *
+ * Macro to automatically free the memory allocated to
+ * the variable(s) 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 type
+
+/**
+ * VIR_AUTOCLEAR:
+ * @type: type of the variables(s) to free automatically
+ *
+ * Macro to automatically clear the variable(s) declared
+ * with it by calling the function defined by
+ * VIR_DEFINE_AUTOCLEAR_FUNC when the variable goes out
+ * of scope.
+ */
+# define VIR_AUTOCLEAR(type) \
+__attribute__((cleanup(VIR_AUTOCLEAR_FUNC_NAME(type type
+
#endif /* __VIR_MEMORY_H_ */
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 06/18] use VIR_AUTOFREE in src/util/viraudit.c
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 03/18] use VIR_AUTOFREE in src/util/virauth.c
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar
---
src/util/virauth.c | 60 +++---
1 file changed, 21 insertions(+), 39 deletions(-)
diff --git a/src/util/virauth.c b/src/util/virauth.c
index adb093e..089a820 100644
--- a/src/util/virauth.c
+++ b/src/util/virauth.c
@@ -42,10 +42,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;
@@ -54,7 +53,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;
}
@@ -64,41 +63,38 @@ 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)
-goto done;
+if (access(*path, R_OK) == 0) {
+VIR_DEBUG("Using auth file '%s'", NULLSTR(*path));
+return 0;
+}
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)
-goto done;
+if (access(*path, R_OK) == 0) {
+VIR_DEBUG("Using auth file '%s'", NULLSTR(*path));
+return 0;
+}
VIR_FREE(*path);
- done:
-ret = 0;
-
-VIR_DEBUG("Using auth file '%s'", NULLSTR(*path));
- cleanup:
-VIR_FREE(userdir);
-
-return ret;
+return 0;
}
@@ -156,7 +152,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)
@@ -193,8 +189,6 @@ virAuthGetUsernamePath(const char *path,
break;
}
-VIR_FREE(prompt);
-
return cred.result;
}
@@ -206,18 +200,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;
}
@@ -230,7 +219,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)
@@ -264,8 +253,6 @@ virAuthGetPasswordPath(const char *path,
break;
}
-VIR_FREE(prompt);
-
return cred.result;
}
@@ -277,15 +264,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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 08/18] use VIR_AUTOFREE in src/util/vircgroup.c
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar
---
src/util/vircgroup.c | 526 ++-
1 file changed, 179 insertions(+), 347 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 0a31947..ed86d31 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -163,7 +163,7 @@ virCgroupPartitionNeedsEscaping(const char *path)
{
FILE *fp = NULL;
int ret = 0;
-char *line = NULL;
+VIR_AUTOFREE(char *) line = NULL;
size_t buflen;
/* If it starts with 'cgroup.' or a '_' of any
@@ -223,7 +223,6 @@ virCgroupPartitionNeedsEscaping(const char *path)
}
cleanup:
-VIR_FREE(line);
VIR_FORCE_FCLOSE(fp);
return ret;
}
@@ -256,41 +255,40 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
const char *machinename)
{
size_t i;
-bool valid = false;
-char *partname = NULL;
-char *scopename_old = NULL;
-char *scopename_new = NULL;
-char *partmachinename = NULL;
+VIR_AUTOFREE(char *) partname = NULL;
+VIR_AUTOFREE(char *) scopename_old = NULL;
+VIR_AUTOFREE(char *) scopename_new = NULL;
+VIR_AUTOFREE(char *) partmachinename = NULL;
if (virAsprintf(&partname, "%s.libvirt-%s",
name, drivername) < 0)
-goto cleanup;
+return false;
if (virCgroupPartitionEscape(&partname) < 0)
-goto cleanup;
+return false;
if (machinename &&
(virAsprintf(&partmachinename, "%s.libvirt-%s",
machinename, drivername) < 0 ||
virCgroupPartitionEscape(&partmachinename) < 0))
-goto cleanup;
+return false;
if (!(scopename_old = virSystemdMakeScopeName(name, drivername, true)))
-goto cleanup;
+return false;
/* We should keep trying even if this failed */
if (!machinename)
virResetLastError();
else if (!(scopename_new = virSystemdMakeScopeName(machinename,
drivername, false)))
-goto cleanup;
+return false;
if (virCgroupPartitionEscape(&scopename_old) < 0)
-goto cleanup;
+return false;
if (scopename_new &&
virCgroupPartitionEscape(&scopename_new) < 0)
-goto cleanup;
+return false;
for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
char *tmp;
@@ -303,7 +301,7 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
tmp = strrchr(group->controllers[i].placement, '/');
if (!tmp)
-goto cleanup;
+return false;
if (stripEmulatorSuffix &&
(i == VIR_CGROUP_CONTROLLER_CPU ||
@@ -313,7 +311,7 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
*tmp = '\0';
tmp = strrchr(group->controllers[i].placement, '/');
if (!tmp)
-goto cleanup;
+return false;
}
tmp++;
@@ -329,18 +327,11 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
tmp, virCgroupControllerTypeToString(i),
name, NULLSTR(machinename), partname,
scopename_old, NULLSTR(scopename_new));
-goto cleanup;
+return false;
}
}
-valid = true;
-
- cleanup:
-VIR_FREE(partmachinename);
-VIR_FREE(partname);
-VIR_FREE(scopename_old);
-VIR_FREE(scopename_new);
-return valid;
+return true;
}
@@ -378,7 +369,7 @@ virCgroupDetectMountsFromFile(virCgroupPtr group,
FILE *mounts = NULL;
struct mntent entry;
char buf[CGROUP_MAX_VAL];
-char *linksrc = NULL;
+VIR_AUTOFREE(char *) linksrc = NULL;
int ret = -1;
mounts = fopen(path, "r");
@@ -468,7 +459,6 @@ virCgroupDetectMountsFromFile(virCgroupPtr group,
ret = 0;
cleanup:
-VIR_FREE(linksrc);
VIR_FORCE_FCLOSE(mounts);
return ret;
}
@@ -547,7 +537,7 @@ virCgroupDetectPlacement(virCgroupPtr group,
FILE *mapping = NULL;
char line[1024];
int ret = -1;
-char *procfile;
+VIR_AUTOFREE(char *) procfile = NULL;
VIR_DEBUG("Detecting placement for pid %lld path %s",
(long long) pid, path);
@@ -628,9 +618,7 @@ virCgroupDetectPlacement(virCgroupPtr group,
ret = 0;
cleanup:
-VIR_FREE(procfile);
VIR_FORCE_FCLOSE(mapping);
-
return ret;
}
@@ -786,8 +774,7 @@ virCgroupSetValueStr(virCgroupPtr group,
const char *key,
const char *value)
{
-int ret = -1;
-char *keypath = NULL;
+VIR_AUTOFREE(char *) keypath = NULL;
char *tmp = NULL;
if (virCgroupPathOfController(group, controller, key, &keypath)
[libvirt] [PATCH v1 04/18] use VIR_AUTOFREE in src/util/virauthconfig.c
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar
---
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 91c9c0c..66f7f7e 100644
--- a/src/util/virauthconfig.c
+++ b/src/util/virauthconfig.c
@@ -106,10 +106,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;
@@ -119,47 +118,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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 17/18] use VIR_AUTOFREE in src/util/virfirewall.c
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar
---
src/util/virfirewall.c | 13 -
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 10c370a..568612c 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -512,7 +512,7 @@ void virFirewallRuleAddArgFormat(virFirewallPtr firewall,
virFirewallRulePtr rule,
const char *fmt, ...)
{
-char *arg;
+VIR_AUTOFREE(char *) arg = NULL;
va_list list;
VIR_FIREWALL_RULE_RETURN_IF_ERROR(firewall, rule);
@@ -526,13 +526,11 @@ void virFirewallRuleAddArgFormat(virFirewallPtr firewall,
va_end(list);
-VIR_FREE(arg);
return;
no_memory:
firewall->err = ENOMEM;
va_end(list);
-VIR_FREE(arg);
}
@@ -679,7 +677,7 @@ virFirewallApplyRuleDirect(virFirewallRulePtr rule,
virCommandPtr cmd = NULL;
int status;
int ret = -1;
-char *error = NULL;
+VIR_AUTOFREE(char *) error = NULL;
if (!bin) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -715,7 +713,6 @@ virFirewallApplyRuleDirect(virFirewallRulePtr rule,
ret = 0;
cleanup:
-VIR_FREE(error);
virCommandFree(cmd);
return ret;
}
@@ -808,12 +805,11 @@ virFirewallApplyRule(virFirewallPtr firewall,
virFirewallRulePtr rule,
bool ignoreErrors)
{
-char *output = NULL;
+VIR_AUTOFREE(char *) output = NULL;
char **lines = NULL;
int ret = -1;
-char *str = virFirewallRuleToString(rule);
+VIR_AUTOFREE(char *) str = virFirewallRuleToString(rule);
VIR_INFO("Applying rule '%s'", NULLSTR(str));
-VIR_FREE(str);
if (rule->ignoreErrors)
ignoreErrors = rule->ignoreErrors;
@@ -858,7 +854,6 @@ virFirewallApplyRule(virFirewallPtr firewall,
ret = 0;
cleanup:
virStringListFree(lines);
-VIR_FREE(output);
return ret;
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 09/18] use VIR_AUTOFREE in src/util/virfcp.c
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 05/18] use VIR_AUTOFREE in src/util/iohelper.c
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 16/18] use VIR_AUTOFREE in src/util/virfilecache.c
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar
---
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 dab7216..49049b7 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;
@@ -179,7 +174,6 @@ virFileCacheLoad(virFileCachePtr cache,
cleanup:
virObjectUnref(loadData);
-VIR_FREE(file);
return ret;
}
@@ -189,20 +183,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;
}
@@ -347,7 +336,7 @@ virFileCacheLookupByFunc(virFileCachePtr cache,
const void *iterData)
{
void *data = NULL;
-char *name = NULL;
+VIR_AUTOFREE(char *) name = NULL;
virObjectLock(cache);
@@ -357,8 +346,6 @@ virFileCacheLookupByFunc(virFileCachePtr cache,
virObjectRef(data);
virObjectUnlock(cache);
-VIR_FREE(name);
-
return data;
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 07/18] use VIR_AUTOFREE in src/util/virbuffer.c
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar
---
src/util/virbuffer.c | 33 -
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/src/util/virbuffer.c b/src/util/virbuffer.c
index 3d6defb..5152f73 100644
--- a/src/util/virbuffer.c
+++ b/src/util/virbuffer.c
@@ -456,7 +456,8 @@ void
virBufferEscapeString(virBufferPtr buf, const char *format, const char *str)
{
int len;
-char *escaped, *out;
+VIR_AUTOFREE(char *) escaped = NULL;
+char *out;
const char *cur;
const char forbidden_characters[] = {
0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
@@ -533,7 +534,6 @@ virBufferEscapeString(virBufferPtr buf, const char *format,
const char *str)
*out = 0;
virBufferAsprintf(buf, format, escaped);
-VIR_FREE(escaped);
}
/**
@@ -612,7 +612,8 @@ virBufferEscape(virBufferPtr buf, char escape, const char
*toescape,
const char *format, const char *str)
{
int len;
-char *escaped, *out;
+VIR_AUTOFREE(char *) escaped = NULL;
+char *out;
const char *cur;
if ((format == NULL) || (buf == NULL) || (str == NULL))
@@ -644,7 +645,6 @@ virBufferEscape(virBufferPtr buf, char escape, const char
*toescape,
*out = 0;
virBufferAsprintf(buf, format, escaped);
-VIR_FREE(escaped);
}
@@ -675,11 +675,11 @@ virBufferEscapeN(virBufferPtr buf,
{
int len;
size_t i;
-char *escaped = NULL;
+VIR_AUTOFREE(char *) escaped = NULL;
char *out;
const char *cur;
struct _virBufferEscapePair escapeItem;
-struct _virBufferEscapePair *escapeList = NULL;
+VIR_AUTOFREE(struct _virBufferEscapePair *) escapeList = NULL;
size_t nescapeList = 0;
va_list ap;
@@ -696,7 +696,8 @@ virBufferEscapeN(virBufferPtr buf,
while ((escapeItem.escape = va_arg(ap, int))) {
if (!(escapeItem.toescape = va_arg(ap, char *))) {
virBufferSetError(buf, errno);
-goto cleanup;
+va_end(ap);
+return;
}
if (strcspn(str, escapeItem.toescape) == len)
@@ -704,19 +705,22 @@ virBufferEscapeN(virBufferPtr buf,
if (VIR_APPEND_ELEMENT_QUIET(escapeList, nescapeList, escapeItem) < 0)
{
virBufferSetError(buf, errno);
-goto cleanup;
+va_end(ap);
+return;
}
}
if (nescapeList == 0) {
virBufferAsprintf(buf, format, str);
-goto cleanup;
+va_end(ap);
+return;
}
if (xalloc_oversized(2, len) ||
VIR_ALLOC_N_QUIET(escaped, 2 * len + 1) < 0) {
virBufferSetError(buf, errno);
-goto cleanup;
+va_end(ap);
+return;
}
cur = str;
@@ -734,11 +738,6 @@ virBufferEscapeN(virBufferPtr buf,
*out = 0;
virBufferAsprintf(buf, format, escaped);
-
- cleanup:
-va_end(ap);
-VIR_FREE(escapeList);
-VIR_FREE(escaped);
}
@@ -803,7 +802,8 @@ void
virBufferEscapeShell(virBufferPtr buf, const char *str)
{
int len;
-char *escaped, *out;
+VIR_AUTOFREE(char *) escaped = NULL;
+char *out;
const char *cur;
if ((buf == NULL) || (str == NULL))
@@ -847,7 +847,6 @@ virBufferEscapeShell(virBufferPtr buf, const char *str)
*out = 0;
virBufferAdd(buf, escaped, -1);
-VIR_FREE(escaped);
}
/**
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 18/18] use VIR_AUTOFREE in src/util/virhook.c
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar
---
src/util/virhook.c | 16 +---
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/src/util/virhook.c b/src/util/virhook.c
index facd74a..51f0eb5 100644
--- a/src/util/virhook.c
+++ b/src/util/virhook.c
@@ -122,8 +122,7 @@ static int virHooksFound = -1;
static int
virHookCheck(int no, const char *driver)
{
-char *path;
-int ret;
+VIR_AUTOFREE(char *) path = NULL;
if (driver == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -139,18 +138,15 @@ virHookCheck(int no, const char *driver)
}
if (!virFileExists(path)) {
-ret = 0;
VIR_DEBUG("No hook script %s", path);
+return 0;
} else if (!virFileIsExecutable(path)) {
-ret = 0;
VIR_WARN("Non-executable hook script %s", path);
+return 0;
} else {
-ret = 1;
VIR_DEBUG("Found hook script %s", path);
+return 1;
}
-
-VIR_FREE(path);
-return ret;
}
/*
@@ -233,7 +229,7 @@ virHookCall(int driver,
char **output)
{
int ret;
-char *path;
+VIR_AUTOFREE(char *) path = NULL;
virCommandPtr cmd;
const char *drvstr;
const char *opstr;
@@ -318,7 +314,5 @@ virHookCall(int driver,
virCommandFree(cmd);
-VIR_FREE(path);
-
return ret;
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 10/18] use VIR_AUTOFREE in src/util/virdnsmasq.c
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar
---
src/util/virdnsmasq.c | 116 +-
1 file changed, 39 insertions(+), 77 deletions(-)
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
index 492dcad..c806cd2 100644
--- a/src/util/virdnsmasq.c
+++ b/src/util/virdnsmasq.c
@@ -95,7 +95,7 @@ addnhostsAdd(dnsmasqAddnHostsfile *addnhostsfile,
virSocketAddr *ip,
const char *name)
{
-char *ipstr = NULL;
+VIR_AUTOFREE(char *) ipstr = NULL;
int idx = -1;
size_t i;
@@ -111,35 +111,29 @@ addnhostsAdd(dnsmasqAddnHostsfile *addnhostsfile,
if (idx < 0) {
if (VIR_REALLOC_N(addnhostsfile->hosts, addnhostsfile->nhosts + 1) < 0)
-goto error;
+return -1;
idx = addnhostsfile->nhosts;
if (VIR_ALLOC(addnhostsfile->hosts[idx].hostnames) < 0)
-goto error;
+return -1;
if (VIR_STRDUP(addnhostsfile->hosts[idx].ip, ipstr) < 0)
-goto error;
+return -1;
addnhostsfile->hosts[idx].nhostnames = 0;
addnhostsfile->nhosts++;
}
if (VIR_REALLOC_N(addnhostsfile->hosts[idx].hostnames,
addnhostsfile->hosts[idx].nhostnames + 1) < 0)
-goto error;
+return -1;
if
(VIR_STRDUP(addnhostsfile->hosts[idx].hostnames[addnhostsfile->hosts[idx].nhostnames],
name) < 0)
-goto error;
-
-VIR_FREE(ipstr);
+return -1;
addnhostsfile->hosts[idx].nhostnames++;
return 0;
-
- error:
-VIR_FREE(ipstr);
-return -1;
}
static dnsmasqAddnHostsfile *
@@ -178,11 +172,10 @@ addnhostsWrite(const char *path,
dnsmasqAddnHost *hosts,
unsigned int nhosts)
{
-char *tmp;
+VIR_AUTOFREE(char *) tmp = NULL;
FILE *f;
bool istmp = true;
size_t i, j;
-int rc = 0;
/* even if there are 0 hosts, create a 0 length file, to allow
* for runtime addition.
@@ -193,61 +186,50 @@ addnhostsWrite(const char *path,
if (!(f = fopen(tmp, "w"))) {
istmp = false;
-if (!(f = fopen(path, "w"))) {
-rc = -errno;
-goto cleanup;
-}
+if (!(f = fopen(path, "w")))
+return -errno;
}
for (i = 0; i < nhosts; i++) {
if (fputs(hosts[i].ip, f) == EOF || fputc('\t', f) == EOF) {
-rc = -errno;
VIR_FORCE_FCLOSE(f);
if (istmp)
unlink(tmp);
-goto cleanup;
+return -errno;
}
for (j = 0; j < hosts[i].nhostnames; j++) {
if (fputs(hosts[i].hostnames[j], f) == EOF || fputc('\t', f) ==
EOF) {
-rc = -errno;
VIR_FORCE_FCLOSE(f);
if (istmp)
unlink(tmp);
-goto cleanup;
+return -errno;
}
}
if (fputc('\n', f) == EOF) {
-rc = -errno;
VIR_FORCE_FCLOSE(f);
if (istmp)
unlink(tmp);
-goto cleanup;
+return -errno;
}
}
-if (VIR_FCLOSE(f) == EOF) {
-rc = -errno;
-goto cleanup;
-}
+if (VIR_FCLOSE(f) == EOF)
+return -errno;
if (istmp && rename(tmp, path) < 0) {
-rc = -errno;
unlink(tmp);
-goto cleanup;
+return -errno;
}
- cleanup:
-VIR_FREE(tmp);
-
-return rc;
+return 0;
}
static int
@@ -310,9 +292,10 @@ hostsfileAdd(dnsmasqHostsfile *hostsfile,
const char *id,
bool ipv6)
{
-char *ipstr = NULL;
+VIR_AUTOFREE(char *) ipstr = NULL;
+
if (VIR_REALLOC_N(hostsfile->hosts, hostsfile->nhosts + 1) < 0)
-goto error;
+return -1;
if (!(ipstr = virSocketAddrFormat(ip)))
return -1;
@@ -322,38 +305,33 @@ hostsfileAdd(dnsmasqHostsfile *hostsfile,
if (name && id) {
if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host,
"id:%s,%s,[%s]", id, name, ipstr) < 0)
-goto error;
+return -1;
} else if (name && !id) {
if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host,
"%s,[%s]", name, ipstr) < 0)
-goto error;
+return -1;
} else if (!name && id) {
if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host,
"id:%s,[%s]", id, ipstr) < 0)
-goto error;
+return -1;
}
} else if (name && mac) {
i
[libvirt] [PATCH v1 11/18] use VIR_AUTOFREE in src/util/vireventpoll.c
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 12/18] use VIR_AUTOFREE in src/util/virdbus.c
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar
---
src/util/virdbus.c | 28 +++-
1 file changed, 7 insertions(+), 21 deletions(-)
diff --git a/src/util/virdbus.c b/src/util/virdbus.c
index ba8b684..66dbe41 100644
--- a/src/util/virdbus.c
+++ b/src/util/virdbus.c
@@ -614,9 +614,8 @@ virDBusMessageIterEncode(DBusMessageIter *rootiter,
size_t nstack = 0;
size_t siglen;
size_t skiplen;
-char *contsig = NULL;
const char *vsig;
-DBusMessageIter *newiter = NULL;
+VIR_AUTOFREE(DBusMessageIter *) newiter = NULL;
DBusMessageIter *iter = rootiter;
VIR_DEBUG("rootiter=%p types=%s", rootiter, types);
@@ -628,6 +627,7 @@ virDBusMessageIterEncode(DBusMessageIter *rootiter,
nstruct = strlen(types);
for (;;) {
+VIR_AUTOFREE(char *) contsig = NULL;
const char *t;
VIR_DEBUG("Loop nstack=%zu narray=%zd nstruct=%zu types='%s'",
@@ -752,11 +752,8 @@ virDBusMessageIterEncode(DBusMessageIter *rootiter,
goto cleanup;
if (virDBusTypeStackPush(&stack, &nstack,
iter, types,
- nstruct, narray) < 0) {
-VIR_FREE(newiter);
+ nstruct, narray) < 0)
goto cleanup;
-}
-VIR_FREE(contsig);
iter = newiter;
newiter = NULL;
types = t + 1;
@@ -780,10 +777,8 @@ virDBusMessageIterEncode(DBusMessageIter *rootiter,
goto cleanup;
if (virDBusTypeStackPush(&stack, &nstack,
iter, types,
- nstruct, narray) < 0) {
-VIR_FREE(newiter);
+ nstruct, narray) < 0)
goto cleanup;
-}
iter = newiter;
newiter = NULL;
types = vsig;
@@ -811,11 +806,8 @@ virDBusMessageIterEncode(DBusMessageIter *rootiter,
if (virDBusTypeStackPush(&stack, &nstack,
iter, types,
- nstruct, narray) < 0) {
-VIR_FREE(newiter);
+ nstruct, narray) < 0)
goto cleanup;
-}
-VIR_FREE(contsig);
iter = newiter;
newiter = NULL;
types = t + 1;
@@ -847,8 +839,6 @@ virDBusMessageIterEncode(DBusMessageIter *rootiter,
}
virDBusTypeStackFree(&stack, &nstack);
-VIR_FREE(contsig);
-VIR_FREE(newiter);
return ret;
}
# undef SET_NEXT_VAL
@@ -891,9 +881,8 @@ virDBusMessageIterDecode(DBusMessageIter *rootiter,
size_t nstack = 0;
size_t skiplen;
size_t siglen;
-char *contsig = NULL;
const char *vsig;
-DBusMessageIter *newiter = NULL;
+VIR_AUTOFREE(DBusMessageIter *) newiter = NULL;
DBusMessageIter *iter = rootiter;
VIR_DEBUG("rootiter=%p types=%s", rootiter, types);
@@ -905,6 +894,7 @@ virDBusMessageIterDecode(DBusMessageIter *rootiter,
nstruct = strlen(types);
for (;;) {
+VIR_AUTOFREE(char *) contsig = NULL;
const char *t;
bool advanceiter = true;
@@ -1053,7 +1043,6 @@ virDBusMessageIterDecode(DBusMessageIter *rootiter,
iter, types,
nstruct, narray) < 0)
goto cleanup;
-VIR_FREE(contsig);
iter = newiter;
newiter = NULL;
types = t + 1;
@@ -1112,7 +1101,6 @@ virDBusMessageIterDecode(DBusMessageIter *rootiter,
iter, types,
nstruct, narray) < 0)
goto cleanup;
-VIR_FREE(contsig);
iter = newiter;
newiter = NULL;
types = t + 1;
@@ -1167,8 +1155,6 @@ virDBusMessageIterDecode(DBusMessageIter *rootiter,
cleanup:
virDBusTypeStackFree(&stack, &nstack);
-VIR_FREE(contsig);
-VIR_FREE(newiter);
return ret;
}
# undef GET_NEXT_VAL
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 14/18] use VIR_AUTOFREE in src/util/virfile.c
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar
---
src/util/virfile.c | 303 +
1 file changed, 99 insertions(+), 204 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 523241f..52b601d 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);
@@ -453,7 +450,7 @@ virFileRewrite(const char *path,
virFileRewriteFunc rewrite,
const void *opaque)
{
-char *newfile = NULL;
+VIR_AUTOFREE(char *) newfile = NULL;
int fd = -1;
int ret = -1;
@@ -494,10 +491,8 @@ virFileRewrite(const char *path,
cleanup:
VIR_FORCE_CLOSE(fd);
-if (newfile) {
+if (newfile)
unlink(newfile);
-VIR_FREE(newfile);
-}
return ret;
}
@@ -724,7 +719,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)
@@ -762,7 +757,6 @@ int virFileLoopDeviceAssociate(const char *file,
ret = 0;
cleanup:
-VIR_FREE(loname);
VIR_FORCE_CLOSE(fsfd);
if (ret == -1)
VIR_FORCE_CLOSE(lofd);
@@ -777,8 +771,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)
@@ -786,18 +779,15 @@ 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;
}
@@ -842,15 +832,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;
}
@@ -860,8 +848,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;
@@ -909,8 +897,6 @@ int virFileNBDDeviceAssociate(const char *file,
ret = 0;
cleanup:
-VIR_FREE(nbddev);
-VIR_FREE(qemunbd);
virCommandFree(cmd);
return ret;
}
@@ -957,7 +943,6 @@ int virFileDeleteTree(const char *dir)
{
DIR *dh;
struct dirent *de;
-char *filepath = NULL;
int ret = -1;
int direrr;
@@ -969,6 +954,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",
@@ -992,8 +978,6 @@ int virFileDeleteTree(const char *dir)
goto cleanup;
}
}
-
-VIR_FREE(filepath);
}
if (direrr < 0)
goto cleanup;
@@ -1008,7 +992,6 @@ int virFileDeleteTree(const char *dir)
ret = 0;
cleanup:
-VIR_FREE(filepath);
VIR_DIR_CLOSE(dh);
return ret;
}
@@ -1166,7 +1149,7 @@ static int
safezero_slow(int fd, off_t offset, off
[libvirt] [PATCH v1 15/18] use VIR_AUTOFREE in src/util/virconf.c
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar
---
src/util/virconf.c | 42 --
1 file changed, 12 insertions(+), 30 deletions(-)
diff --git a/src/util/virconf.c b/src/util/virconf.c
index e0a3fd1..7dd8820 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -745,9 +745,8 @@ virConfParse(const char *filename, const char *content, int
len,
virConfPtr
virConfReadFile(const char *filename, unsigned int flags)
{
-char *content;
+VIR_AUTOFREE(char *) content = NULL;
int len;
-virConfPtr conf;
VIR_DEBUG("filename=%s", NULLSTR(filename));
@@ -759,11 +758,7 @@ virConfReadFile(const char *filename, unsigned int flags)
if ((len = virFileReadAll(filename, MAX_CONFIG_FILE_SIZE, &content)) < 0)
return NULL;
-conf = virConfParse(filename, content, len, flags);
-
-VIR_FREE(content);
-
-return conf;
+return virConfParse(filename, content, len, flags);
}
/**
@@ -1451,7 +1446,7 @@ virConfWriteFile(const char *filename, virConfPtr conf)
virConfEntryPtr cur;
int ret;
int fd;
-char *content;
+VIR_AUTOFREE(char *) content = NULL;
unsigned int use;
if (conf == NULL)
@@ -1476,7 +1471,6 @@ virConfWriteFile(const char *filename, virConfPtr conf)
use = virBufferUse(&buf);
content = virBufferContentAndReset(&buf);
ret = safewrite(fd, content, use);
-VIR_FREE(content);
VIR_FORCE_CLOSE(fd);
if (ret != (int)use) {
virConfError(NULL, VIR_ERR_WRITE_FAILED, _("failed to save content"));
@@ -1504,7 +1498,7 @@ virConfWriteMem(char *memory, int *len, virConfPtr conf)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
virConfEntryPtr cur;
-char *content;
+VIR_AUTOFREE(char *) content = NULL;
unsigned int use;
if ((memory == NULL) || (len == NULL) || (*len <= 0) || (conf == NULL))
@@ -1524,11 +1518,9 @@ virConfWriteMem(char *memory, int *len, virConfPtr conf)
if ((int)use >= *len) {
*len = (int)use;
-VIR_FREE(content);
return -1;
}
memcpy(memory, content, use);
-VIR_FREE(content);
*len = use;
return use;
}
@@ -1542,16 +1534,13 @@ virConfLoadConfigPath(const char *name)
SYSCONFDIR, name) < 0)
return NULL;
} else {
-char *userdir = virGetUserConfigDirectory();
+VIR_AUTOFREE(char *) userdir = virGetUserConfigDirectory();
if (!userdir)
return NULL;
if (virAsprintf(&path, "%s/%s",
-userdir, name) < 0) {
-VIR_FREE(userdir);
+userdir, name) < 0)
return NULL;
-}
-VIR_FREE(userdir);
}
return path;
@@ -1560,26 +1549,19 @@ virConfLoadConfigPath(const char *name)
int
virConfLoadConfig(virConfPtr *conf, const char *name)
{
-char *path = NULL;
-int ret = -1;
+VIR_AUTOFREE(char *) path = NULL;
*conf = NULL;
if (!(path = virConfLoadConfigPath(name)))
-goto cleanup;
+return -1;
-if (!virFileExists(path)) {
-ret = 0;
-goto cleanup;
-}
+if (!virFileExists(path))
+return 0;
VIR_DEBUG("Loading config file '%s'", path);
if (!(*conf = virConfReadFile(path, 0)))
-goto cleanup;
-
-ret = 0;
+return -1;
- cleanup:
-VIR_FREE(path);
-return ret;
+return 0;
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 13/18] use VIR_AUTOFREE in src/util/virfdstream.c
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar
---
src/util/virfdstream.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
index e4973a2..e7befbc 100644
--- a/src/util/virfdstream.c
+++ b/src/util/virfdstream.c
@@ -431,7 +431,7 @@ virFDStreamThreadDoRead(virFDStreamDataPtr fdst,
virFDStreamMsgPtr msg = NULL;
int inData = 0;
long long sectionLen = 0;
-char *buf = NULL;
+VIR_AUTOFREE(char *) buf = NULL;
ssize_t got;
if (sparse && *dataLen == 0) {
@@ -496,7 +496,6 @@ virFDStreamThreadDoRead(virFDStreamDataPtr fdst,
return got;
error:
-VIR_FREE(buf);
virFDStreamMsgFree(msg);
return -1;
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 01/18] add macros for implementing automatic cleanup functionality
On Tue, 5 Jun 2018 at 21:00, Erik Skultety wrote:
>
> On Sun, Jun 03, 2018 at 01:41:59PM +0530, Sukrit Bhatnagar wrote:
> > New macros are added to src/util/viralloc.h which help in
> > adding cleanup attribute to variable declarations.
> >
> > Signed-off-by: Sukrit Bhatnagar
>
> If you recall the recent RFC thread [1], I agreed with Pavel that we should
> introduce complete changes to each file, so that we don't need to track was
> has
> been already done for what file and what is still missing, so the series would
> look something like this:
>
> 1) vir.c preparations (especially VIR_{AUTOPTR,AUTOCLEAR} might
> need
> that)
> 2) Use VIR_AUTOFREE across vir.c
> 3) Use VIR_AUTOPTR across vir.c
> 4) Use VIR_AUTOCLEAR across vir.c
>
> ...One more thing, the commit subject of every patch in the current series
> should probably look like this:
> util: : Use VIR_AUTOFREE instead of VIR_FREE for scalar types
>
> and the corresponding commit message:
> By making use of the GCC's __attribute__((cleanup)) handled by VIR_AUTOFREE
> macro, majority of the VIR_FREE calls can be dropped, which in turn leads to
> getting rid of most of our cleanup sections.
>
> [1] https://www.redhat.com/archives/libvir-list/2018-May/msg02391.html
I have some rudimentary queries regarding this approach:
- The in commit subject should be like "vircgroup" or just "cgroup"?
- Do I create a separate series of patch even for those files who do not require
VIR_AUTOPTR and VIR_AUTOCLEAR but only VIR_AUTOFREE,
like viraudit.c?
- Do I use the same commit subject and message, with minor changes of course,
for all patches in a series (AUTOFREE, AUTOPTR and AUTOCLEAR)?
- In the case of files like virauth.c which require changes in
virauthconfig.h instead
of virauth.h, do I batch all the files virauth* together?
I am assuming that the changes to header file vir.h
corresponding to vir.c will be in the same series if needed.
--
Thanks,
Sukrit
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 01/18] add macros for implementing automatic cleanup functionality
On Wed, 6 Jun 2018 at 21:25, Erik Skultety wrote:
>
> On Wed, Jun 06, 2018 at 06:42:29PM +0530, Sukrit Bhatnagar wrote:
> > On Tue, 5 Jun 2018 at 21:00, Erik Skultety wrote:
> > >
> > > On Sun, Jun 03, 2018 at 01:41:59PM +0530, Sukrit Bhatnagar wrote:
> > > > New macros are added to src/util/viralloc.h which help in
> > > > adding cleanup attribute to variable declarations.
> > > >
> > > > Signed-off-by: Sukrit Bhatnagar
> > >
> > > If you recall the recent RFC thread [1], I agreed with Pavel that we
> > > should
> > > introduce complete changes to each file, so that we don't need to track
> > > was has
> > > been already done for what file and what is still missing, so the series
> > > would
> > > look something like this:
> > >
> > > 1) vir.c preparations (especially VIR_{AUTOPTR,AUTOCLEAR}
> > > might need
> > > that)
> > > 2) Use VIR_AUTOFREE across vir.c
> > > 3) Use VIR_AUTOPTR across vir.c
> > > 4) Use VIR_AUTOCLEAR across vir.c
> > >
> > > ...One more thing, the commit subject of every patch in the current series
> > > should probably look like this:
> > > util: : Use VIR_AUTOFREE instead of VIR_FREE for scalar types
> > >
> > > and the corresponding commit message:
> > > By making use of the GCC's __attribute__((cleanup)) handled by
> > > VIR_AUTOFREE
> > > macro, majority of the VIR_FREE calls can be dropped, which in turn leads
> > > to
> > > getting rid of most of our cleanup sections.
> > >
> > > [1] https://www.redhat.com/archives/libvir-list/2018-May/msg02391.html
> >
> > I have some rudimentary queries regarding this approach:
> >
> > - The in commit subject should be like "vircgroup" or just
> > "cgroup"?
>
> preferably "cgroup" (you'll save 3 chars :)), but doesn't really matter, I
> think I pushed a patch yesterday where I actually added the 'vir' prefix, so
> whatever...
>
> > - Do I create a separate series of patch even for those files who do not
> > require
> > VIR_AUTOPTR and VIR_AUTOCLEAR but only VIR_AUTOFREE,
> > like viraudit.c?
>
> I'm getting the impression we're not on the same page yet, I suppose I should
> have added something like "..." in the example above or simply be more clear
> in
> general.
>
> So, let's be more specific, this series was 18 patches long and you need to
> add
> 2 more patches per file, so that would be 54 patches in a single series,
> that's
> big, so how about taking 5-10 files instead of 18, that would produce roughly
> 15-30 patches in a single series (a bit more manageable and as I mentioned
> somewhere else, complete changes that are fine can be merged independently) -
> so it's not going to be a separate series per file, rather a single series,
> following the pattern I mentioned in my previous response above, performing
> changes to up to N selected files, does it make more sense now?
Yes, it does now.
> > - Do I use the same commit subject and message, with minor changes of
> > course,
> > for all patches in a series (AUTOFREE, AUTOPTR and AUTOCLEAR)?
>
> Yep.
>
> > - In the case of files like virauth.c which require changes in
> > virauthconfig.h instead
> > of virauth.h, do I batch all the files virauth* together?
>
> Yes, well, it should be coupled based on the change you're making, but you
> just
> mentioned it - "required changes" - IOW the patch would not otherwise compile.
>
> >
> > I am assuming that the changes to header file vir.h
> > corresponding to vir.c will be in the same series if needed.
>
> See above, but yes.
>
> Let me know if there's still something which is not clear.
No other queries for now, thanks!
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 01/21] add macros for implementing automatic cleanup functionality
New macros are added to src/util/viralloc.h which help in
adding GCC's cleanup attribute to variable declarations.
---
src/util/viralloc.h | 68 +
1 file changed, 68 insertions(+)
diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 69d0f90..5804ca9 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -596,4 +596,72 @@ void virAllocTestInit(void);
int virAllocTestCount(void);
void virAllocTestOOM(int n, int m);
void virAllocTestHook(void (*func)(int, void*), void *data);
+
+# define VIR_AUTOPTR_FUNC_NAME(type) virAutoPtr##type
+# define VIR_AUTOCLEAR_FUNC_NAME(type) virAutoClear##type
+
+/**
+ * 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) \
+static inline void VIR_AUTOPTR_FUNC_NAME(type)(type *_ptr) \
+{ \
+(func)(*_ptr); \
+} \
+
+/**
+ * VIR_DEFINE_AUTOCLEAR_FUNC:
+ * @type: type of the variable to be freed automatically
+ * @func: cleanup function to be automatically called
+ *
+ * This macro defines a function for automatic clearing of
+ * a variable of type @type. This newly defined function
+ * works as a necessary wrapper around @func.
+ */
+# define VIR_DEFINE_AUTOCLEAR_FUNC(type, func) \
+static inline void VIR_AUTOCLEAR_FUNC_NAME(type)(type *_ptr) \
+{ \
+(func)(_ptr); \
+} \
+
+/**
+ * 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 type
+
+/**
+ * VIR_AUTOCLEAR:
+ * @type: type of the variable to be freed automatically
+ *
+ * Macro to automatically clear the variable declared
+ * with it by calling the function defined by
+ * VIR_DEFINE_AUTOCLEAR_FUNC when the variable goes out
+ * of scope.
+ */
+# define VIR_AUTOCLEAR(type) \
+__attribute__((cleanup(VIR_AUTOCLEAR_FUNC_NAME(type type
+
#endif /* __VIR_MEMORY_H_ */
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 00/21] use GCC's cleanup attribute in src/util
This series of patches introduces a new set of macros which help in using GCC's __attribute__((cleanup)) in the code. 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 (21): add macros for implementing automatic cleanup functionality util: arptable: use VIR_AUTOFREE instead of VIR_FREE for scalar types util: iohelper: 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: 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: identity: use VIR_AUTOFREE instead of VIR_FREE for scalar 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 src/util/iohelper.c | 4 +-- src/util/viralloc.h | 68 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/vireventpoll.c | 7 ++--- src/util/virfcp.c| 20 +- src/util/virfilecache.c | 35 - src/util/viridentity.c | 54 -- src/util/virjson.c | 45 +--- src/util/virjson.h | 3 +++ 15 files changed, 168 insertions(+), 195 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 02/21] util: arptable: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of the GCC's __attribute__((cleanup)) handled by VIR_AUTOFREE
macro, majority of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 04/21] util: audit: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of the GCC's __attribute__((cleanup)) handled by VIR_AUTOFREE
macro, majority of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 07/21] util: filecache: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of the GCC's __attribute__((cleanup)) handled by VIR_AUTOFREE
macro, majority of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 06/21] util: eventpoll: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of the GCC's __attribute__((cleanup)) handled by VIR_AUTOFREE
macro, majority of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 10/21] util: authconfig: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of the GCC's __attribute__((cleanup)) handled by VIR_AUTOFREE
macro, majority of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
---
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..9977e71 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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 03/21] util: iohelper: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of the GCC's __attribute__((cleanup)) handled by VIR_AUTOFREE
macro, majority of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 09/21] util: authconfig: remove redundant include directive
The include directive for viralloc.h is added in virauthconfig.h in the previous patch. --- 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 12/21] util: auth: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of the GCC's __attribute__((cleanup)) handled by VIR_AUTOFREE
macro, majority of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 14/21] util: json: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
Using the 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. --- src/util/virjson.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/util/virjson.h b/src/util/virjson.h index e4a82bd..593dcb7 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 @@ -45,6 +46,8 @@ typedef virJSONValue *virJSONValuePtr; void virJSONValueFree(virJSONValuePtr value); void virJSONValueHashFree(void *opaque, const void *name); +VIR_DEFINE_AUTOPTR_FUNC(virJSONValuePtr, virJSONValueFree) + virJSONType virJSONValueGetType(const virJSONValue *value); int virJSONValueObjectCreate(virJSONValuePtr *obj, ...) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 08/21] util: authconfig: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
Using the 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. --- src/util/virauthconfig.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/util/virauthconfig.h b/src/util/virauthconfig.h index ac0ceeb..375ccad 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(virAuthConfigPtr, virAuthConfigFree) + #endif /* __VIR_AUTHCONFIG_H__ */ -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 05/21] util: fcp: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of the GCC's __attribute__((cleanup)) handled by VIR_AUTOFREE
macro, majority of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 11/21] util: auth: remove redundant include directive
The include directive for viralloc.h is added in virauthconfig.h in a previous patch. --- 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 17/21] util: json: use VIR_AUTOPTR for aggregate types
By making use of the GCC's __attribute__((cleanup)) handled by VIR_AUTOPTR
macro, majority of the calls to *Free functions can be dropped, which in
turn leads to getting rid of most of our cleanup sections.
---
src/util/virjson.c | 15 ---
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 1391a3b..3b54d94 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -2033,16 +2033,12 @@ char *
virJSONStringReformat(const char *jsonstr,
bool pretty)
{
-virJSONValuePtr json;
-char *ret;
+VIR_AUTOPTR(virJSONValuePtr) json = NULL;
if (!(json = virJSONValueFromString(jsonstr)))
return NULL;
-ret = virJSONValueToString(json, pretty);
-
-virJSONValueFree(json);
-return ret;
+return virJSONValueToString(json, pretty);
}
@@ -2131,7 +2127,7 @@ virJSONValueObjectDeflattenWorker(const char *key,
virJSONValuePtr
virJSONValueObjectDeflatten(virJSONValuePtr json)
{
-virJSONValuePtr deflattened;
+VIR_AUTOPTR(virJSONValuePtr) deflattened = NULL;
virJSONValuePtr ret = NULL;
if (!(deflattened = virJSONValueNewObject()))
@@ -2140,12 +2136,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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 13/21] util: auth: use VIR_AUTOPTR for aggregate types
By making use of the GCC's __attribute__((cleanup)) handled by VIR_AUTOPTR
macro, majority of the calls to *Free functions can be dropped, which in
turn leads to getting rid of most of our cleanup sections.
---
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..f000d45 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(virAuthConfigPtr) 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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 16/21] util: json: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of the GCC's __attribute__((cleanup)) handled by VIR_AUTOFREE
macro, majority of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
---
src/util/virjson.c | 29 +
1 file changed, 9 insertions(+), 20 deletions(-)
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 92f3994..1391a3b 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -497,11 +497,10 @@ 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;
}
@@ -510,11 +509,10 @@ 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;
}
@@ -523,11 +521,10 @@ 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;
}
@@ -536,11 +533,10 @@ 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;
}
@@ -549,11 +545,10 @@ 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;
}
@@ -1171,10 +1166,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 +1185,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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 15/21] util: json: remove redundant include directive
The include directive for viralloc.h is added in virjson.h in the previous patch. --- 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 #include "virjson.h" -#include "viralloc.h" #include "virerror.h" #include "virlog.h" #include "virstring.h" -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 19/21] util: bitmap: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
Using the 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. --- src/util/virbitmap.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index 2464814..86bb84e 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 @@ -44,6 +45,8 @@ virBitmapPtr virBitmapNewEmpty(void) ATTRIBUTE_RETURN_CHECK; */ void virBitmapFree(virBitmapPtr bitmap); +VIR_DEFINE_AUTOPTR_FUNC(virBitmapPtr, virBitmapFree) + /* * Copy all bits from @src to @dst. The bitmap sizes * must be the same -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 18/21] util: identity: use VIR_AUTOFREE instead of VIR_FREE for scalar types
By making use of the GCC's __attribute__((cleanup)) handled by VIR_AUTOFREE
macro, majority of the VIR_FREE calls can be dropped, which in turn leads to
getting rid of most of our cleanup sections.
---
src/util/viridentity.c | 54 --
1 file changed, 22 insertions(+), 32 deletions(-)
diff --git a/src/util/viridentity.c b/src/util/viridentity.c
index 2f4307b..2060dd7 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,9 @@ virIdentityPtr virIdentityGetSystem(void)
}
#endif
- cleanup:
-VIR_FREE(username);
-VIR_FREE(groupname);
-return ret;
-
error:
virObjectUnref(ret);
-ret = NULL;
-goto cleanup;
+return NULL;
}
@@ -461,15 +455,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 +478,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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 20/21] util: bitmap: remove redundant include directive
The include directive for viralloc.h is added in virbitmap.h in the previous patch. --- 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 #include "virbitmap.h" -#include "viralloc.h" #include "virbuffer.h" #include "c-ctype.h" #include "count-one-bits.h" -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 21/21] util: bitmap: use VIR_AUTOPTR for aggregate types
By making use of the GCC's __attribute__((cleanup)) handled by VIR_AUTOPTR
macro, majority of the calls to *Free functions can be dropped, which in
turn leads to getting rid of most of our cleanup sections.
---
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..37e5c0e 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(virBitmapPtr) map = NULL;
if (!(map = virBitmapNewData(data, len)))
return NULL;
-ret = virBitmapFormat(map);
-virBitmapFree(map);
-return ret;
+return virBitmapFormat(map);
}
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [GSoC] Code design for scalar and external types
Hi, I am starting this discussion thread as a continuation of my GSoC weekly meeting with Erik and Pavel on 8th June. I was going through src/util/virstring.c for adding cleanup macros and saw that virStringListFree takes on char ** as an argument, and equivalently, we declare a list of strings as char **. For the cleanup function defined by VIR_DEFINE_AUTOPTR_FUNC, it is required that the associated type has a name like virSomethingPtr. It was also discussed that there are similar issues with DBus types, but VIR_AUTOFREE can work there as we use VIR_ALLOC. I honestly don't know much about that. We discussed that we have two solutions: - Create a virSomethingPtr by typedef-ing char** As Pavel told, GLib has typedef gchar** GStrv; which is used together with g_auto and it has g_strfreev(gchar **str_array) which is the same as we have virStringListFree() I have tried adding the following in src/util/virstrnig.h, and it seems to work fine: typedef char **virStringList; VIR_DEFINE_AUTOPTR_FUNC(virStringList, virStringListFree) We can use it as: VIR_AUTOPTR(virStringList) lines = NULL; There may be other scalar and external types where this problem occurs, and it is not good to create a typedef for each of them, but maybe we can make an exception for char ** and create a type for it. - Overload VIR_AUTOFREE macro by making it variadic As Erik told, we could make VIR_AUTOFREE a variadic macro whose varying parameter can be the Free function name. If left blank, we use virFree. I went ahead with trying it and after reading some posts on StackOverflow, I came up with this: #define _VIR_AUTOFREE_0(type) __attribute__((cleanup(virFree))) type #define _VIR_AUTOFREE_1(type, func) __attribute__((cleanup(func))) type #define _VIR_AUTOFREE_OVERLOADER(_1, _2, NAME, ...) NAME #define VIR_AUTOFREE(...) _VIR_AUTOFREE_OVERLOADER(__VA_ARGS__, _VIR_AUTOFREE_1, _VIR_AUTOFREE_0)(__VA_ARGS__) The required functionality is working as expected; passing only one argument will use virFree, and passing two arguments will use the function specified as 2nd argument. Passing more than 2 arguments will result in an error. The macros with _ prefix are meant to be for internal use only. Also, @func needs to be a wrapper around virStringListFree as it will take char ***, not just char **. We probably need to define a new function. Here we are specifying the Free function to use at the time of usage of the VIR_AUTOFREE macro, which may make the code look bad: VIR_AUTOFREE(char **, virStringListSomethingFree) lines = NULL; Suggestions and opinions are welcome. Thanks, Sukrit -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [GSoC] Code design for scalar and external types
On Mon, 11 Jun 2018 at 16:35, Pavel Hrdina wrote: > > On Mon, Jun 11, 2018 at 12:59:11PM +0200, Martin Kletzander wrote: > > On Mon, Jun 11, 2018 at 12:53:47PM +0200, Erik Skultety wrote: > > > On Mon, Jun 11, 2018 at 12:43:59PM +0200, Martin Kletzander wrote: > > > > On Mon, Jun 11, 2018 at 12:12:16PM +0200, Pavel Hrdina wrote: > > > > > On Sat, Jun 09, 2018 at 11:12:29PM +0200, Martin Kletzander wrote: > > > > > > On Sat, Jun 09, 2018 at 10:06:55PM +0530, Sukrit Bhatnagar wrote: > > > > > > > Hi, > > > > > > > > > > > > > > I am starting this discussion thread as a continuation of my GSoC > > > > > > > weekly meeting with Erik and Pavel on 8th June. > > > > > > > > > > > > > > I was going through src/util/virstring.c for adding cleanup > > > > > > > macros and > > > > > > > saw that virStringListFree takes on char ** as an argument, and > > > > > > > equivalently, we declare a list of strings as char **. > > > > > > > > > > > > > > For the cleanup function defined by VIR_DEFINE_AUTOPTR_FUNC, it is > > > > > > > required that the associated type has a name like virSomethingPtr. > > > > > > > > > > > > > > It was also discussed that there are similar issues with DBus > > > > > > > types, > > > > > > > but VIR_AUTOFREE can work there as we use VIR_ALLOC. I honestly > > > > > > > don't > > > > > > > know much about that. > > > > > > > > > > > > > > We discussed that we have two solutions: > > > > > > > > > > > > > > - Create a virSomethingPtr by typedef-ing char** > > > > > > > > > > > > > > As Pavel told, GLib has typedef gchar** GStrv; which is used > > > > > > > together > > > > > > > with g_auto and it has g_strfreev(gchar **str_array) which is the > > > > > > > same > > > > > > > as we have virStringListFree() > > > > > > > > > > > > > > I have tried adding the following in src/util/virstrnig.h, and it > > > > > > > seems to work fine: > > > > > > > > > > > > > > typedef char **virStringList; > > > > > > > VIR_DEFINE_AUTOPTR_FUNC(virStringList, virStringListFree) > > > > > > > > > > > > > > We can use it as: > > > > > > > VIR_AUTOPTR(virStringList) lines = NULL; > > > > > > > > > > > > > > There may be other scalar and external types where this problem > > > > > > > occurs, and it is not good to create a typedef for each of them, > > > > > > > but > > > > > > > maybe we can make an exception for char ** and create a type for > > > > > > > it. > > > > > > > > > > > > > > > > > > > > > - Overload VIR_AUTOFREE macro by making it variadic > > > > > > > > > > > > > > As Erik told, we could make VIR_AUTOFREE a variadic macro whose > > > > > > > varying parameter can be the Free function name. If left blank, > > > > > > > we use > > > > > > > virFree. > > > > > > > > > > > > > > I went ahead with trying it and after reading some posts on > > > > > > > StackOverflow, I came up with this: > > > > > > > > > > > > > > #define _VIR_AUTOFREE_0(type) __attribute__((cleanup(virFree))) > > > > > > > type > > > > > > > #define _VIR_AUTOFREE_1(type, func) > > > > > > > __attribute__((cleanup(func))) type > > > > > > > > > > > > > > #define _VIR_AUTOFREE_OVERLOADER(_1, _2, NAME, ...) NAME > > > > > > > #define VIR_AUTOFREE(...) _VIR_AUTOFREE_OVERLOADER(__VA_ARGS__, > > > > > > > _VIR_AUTOFREE_1, _VIR_AUTOFREE_0)(__VA_ARGS__) > > > > > > > > > > > > > > The required functionality is working as expected; passing only > > > > > > > one > > > > > > > argument will use virFree, and passing two arguments will use the > > > > > > &
Re: [libvirt] [GSoC] Code design for scalar and external types
On Mon, 18 Jun 2018 at 17:20, Andrea Bolognani wrote:
>
> On Mon, 2018-06-18 at 11:52 +0100, Daniel P. Berrangé wrote:
> > On Mon, Jun 18, 2018 at 12:24:39PM +0200, Pavel Hrdina wrote:
> > > # define VIR_DEFINE_AUTOCLEAR_FUNC(type, func) \
> > > static inline void VIR_AUTOCLEAR_FUNC_NAME(type)(type *_ptr) \
> > > { \
> > > (func)(_ptr); \
> > > }
> >
> > For anything where we define the impl of (func) these two macros
> > should never be used IMHO. We should just change the signature of
> > the real functions so that accept a double pointer straight away,
> > and thus not require the wrapper function. Yes, it this will
> > require updating existing code, but such a design change is
> > beneficial even without the cleanup macros, as it prevents the
> > possbility of double frees. I'd suggest we just do this kind of
> > change straightaway
>
> Agreed that we want to fix our own free functions.
>
> > > # define VIR_AUTOFREE(type) __attribute__((cleanup(virFree))) type
> >
> > I don't see the point in passing "type" in here we could simplify
> > this:
> >
> > #define VIR_AUTOFREE __attribute__((cleanup(virFree))
> >
> > VIR_AUTOFREE char *foo = NULL;
>
> Passing the type was suggested earlier to make usage consistent
> across all cases, eg.
>
> VIR_AUTOFREE(char *) str = NULL;
> VIR_AUTOPTR(virDomain) dom = NULL;
>
> and IIRC we ended up agreeing that it was a good idea overall.
>
> > > # define VIR_AUTOPTR(type) \
> > > __attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type
> > > VIR_AUTOPTR_TYPE_NAME(type)
> > >
> > > The usage would not require our internal vir*Ptr types and would
> > > be easy to use with external types as well:
> > >
> > > VIR_DEFINE_AUTOPTR_FUNC(virBitmap, virBitmapFree);
> > >
> > > VIR_AUTOPTR(virBitmap) map = NULL;
> > >
> > > VIR_DEFINE_AUTOPTR_FUNC(LIBSSH2_CHANNEL, libssh2_channel_free);
> >
> > Contrary to my general point above about VIR_DEFINE_AUTOPTR_FUNC,
> > this is a reasonable usage, because we don't control the signature
> > of the libssh2_channel_free function.
>
> This is why I suggested we might want to have two sets of
> macros, one for types we defined ourselves and one for types
> we bring in from external libraries.
>
> > > VIR_AUTOPTR(LIBSSH2_CHANNEL) channel = NULL;
> >
> > With my example above
> >
> >#define VIR_AUTOFREE_LIBSSH_CHANNEL VIR_AUTOFREE_FUNC(LIBSSH2_CHANNEL)
> >
> >VIR_AUTOFREE_LIBSSH_CHANNEL LIBSSH_CHANNEL *chan = NULL;
>
> This makes the declaration needlessly verbose, since you're
> repeating the type name twice; Pavel's approach would avoid
> that.
So, have we reached a decision about the design?
Fixing the existing vir*Free functions to take double pointers seems good to me,
as many have mentioned earlier. It will solve multiple problems at once.
I have almost 7 weeks left of GSoC, and I think it is totally doable.
On the other hand, making cleanup macros use double pointers also seems
great! But, aren't we borrowing too much design from GLib?
Anyway, both approaches take care of the double free problem.
TL;DR
I prefer cleanup macros which define functions with double pointers
over changing
existing vir*Free functions. It just looks more natural.
I apologize in advance for this long mail! Could not decide what
patches to make,
so I included a glimpse of the code in this mail.
This is just to start this discussion again and help with visualizing.
Here goes some example code. Most of it is paraphrasing what is already
discussed previously.
/**
* Cleanup macros defining functions with double pointers:
**/
#define VIR_AUTOPTR_TYPE_NAME(type) type##AutoPtr
#define VIR_AUTOPTR_FUNC_NAME(type) \
VIR_AUTOPTR_TYPE_NAME(type)##Free
#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; \
} \
#define VIR_AUTOPTR(type) \
__attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type \
VIR_AUTOPTR_TYPE_NAME(type)
/ Using this for virBitmap:
VIR_DEFINE_AUTOPTR_FUNC(virBitmap, virBitmapFree)
will generate:
typedef virBitmap *virBitmapAutoPtr;
static inline void virBitmapAutoPtrFree(virBitmap **_ptr)
{
if (*_ptr)
virBitmapFree(*_ptr);
*_ptr = NULL;
}
Then we use it like:
VIR_AUTOPTR(virBitmap) map = NULL;
which will become:
__attribute__((cleanup(virBitmapAutoPtrFree))) virBitmapAutoPtr map = NULL;
/ Using this for char ** (some of my ideas added):
typedef char *virString;
VIR_DEFINE_AUTOPTR_FUNC(virString, virStringListFree)
will generate:
typedef virString *virStringAutoPtr;
static inline void virStringAutoPtrFree(virString **_ptr)
{
if (*_ptr)
virStringListFree(*_ptr);
*_ptr = NULL;
}
T
[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
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[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
---
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
[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 --- 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 # 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[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 --- cfg.mk | 8 1 file changed, 8 insertions(+) diff --git a/cfg.mk b/cfg.mk index d9e90d5..7949fc8 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1077,6 +1077,14 @@ sc_require_attribute_cleanup_scalar_type_space: halt='there must be exactly one space between the type and asterisk inside VIR_AUTOFREE' \ $(_sc_search_regexp) +# Rule to ensure that there is only one variable declaration in +# a line when the variable is declared using a cleanup macro. +sc_require_attribute_cleanup_one_per_line: + @prohibit='VIR_AUTO(FREE|PTR)\(.+\) ([^\(]*(, .*)+|[^\(]*\(.*\)(, .*)+);' \ +in_vc_files='\.[chx]$$' \ +halt='there must be only one variable declaration per line when using cleanup macro' \ + $(_sc_search_regexp) + # We don't use this feature of maint.mk. prev_version_file = /dev/null -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[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 --- 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[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 --- 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[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 --- 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[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 --- 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[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 --- 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[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
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[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
---
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 impleme
[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
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[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
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 00/35] use GNU C's cleanup attribute in src/util (batch I)
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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[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
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[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 --- cfg.mk | 11 +++ 1 file changed, 11 insertions(+) diff --git a/cfg.mk b/cfg.mk index 6bebd0a..196d1b2 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1057,6 +1057,17 @@ sc_prohibit_backslash_alignment: halt='Do not attempt to right-align backslashes' \ $(_sc_search_regexp) +# Some syntax rules pertaining to the usage of cleanup macros +# implementing GNU C's cleanup attribute + +# Rule to ensure that varibales declared using a cleanup macro are +# always initialized. +sc_require_attribute_cleanup_initialization: + @prohibit='VIR_AUTO(FREE|PTR)\(.+\) [^=]+;' \ + in_vc_files='\.[chx]$$' \ + halt='variable declared with a cleanup macro must be initialized' \ + $(_sc_search_regexp) + # We don't use this feature of maint.mk. prev_version_file = /dev/null -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[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
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[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 --- 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 #include "virbitmap.h" -#include "viralloc.h" #include "virbuffer.h" #include "c-ctype.h" #include "count-one-bits.h" -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[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 --- 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 #include "virjson.h" -#include "viralloc.h" #include "virerror.h" #include "virlog.h" #include "virstring.h" -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[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
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[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 --- 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[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
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[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 --- 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[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
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[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
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[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 --- 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[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
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[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
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[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
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[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
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[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 --- 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 @@ -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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[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
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[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 --- 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 @@ -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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[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
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[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
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[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
---
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 04/35] cfg.mk: single variable declaration per line when using cleanup macro
On Tue, 10 Jul 2018 at 16:24, Erik Skultety wrote: > > On Sat, Jun 30, 2018 at 02:30:08PM +0530, Sukrit Bhatnagar wrote: > > Add rule to ensure that each variable declaration made using > > a cleanup macro is in its own separate line. > > > > Sometimes a variable might be initialized from a value returned > > by a macro or a function, which may take on more than one > > parameter, thereby introducing a comma, which might be mistaken > > for multiple declarations in a line. This rule takes care of > > that too. > > I can't think of an example or I'm just not seeing it, can you please give me > an example where you actually need the rule below? Because right now I don't > see a need for it. In src/util/virfile.c in virFileAbsPath function: ... VIR_AUTOFREE(char *) buf = getcwd(NULL, 0); ... -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 05/35] cfg.mk: no trailing semicolon at line invoking VIR_DEFINE_* cleanup macro
On Tue, 10 Jul 2018 at 16:30, Erik Skultety 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 > > --- > > 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. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 04/35] cfg.mk: single variable declaration per line when using cleanup macro
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 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 wrote: > > > > > > > > On Sat, Jun 30, 2018 at 02:30:08PM +0530, Sukrit Bhatnagar wrote: > > > > > Add rule to ensure that each variable declaration made using > > > > > a cleanup macro is in its own separate line. > > > > > > > > > > Sometimes a variable might be initialized from a value returned > > > > > by a macro or a function, which may take on more than one > > > > > parameter, thereby introducing a comma, which might be mistaken > > > > > for multiple declarations in a line. This rule takes care of > > > > > that too. > > > > > > > > I can't think of an example or I'm just not seeing it, can you please > > > > give me > > > > an example where you actually need the rule below? Because right now I > > > > don't > > > > see a need for it. > > > > > > In src/util/virfile.c in virFileAbsPath function: > > > ... > > > VIR_AUTOFREE(char *) buf = getcwd(NULL, 0); > > > ... > > > > I don't see anything wrong with it, it is properly initialized to some > > value, it doesn't have to be only NULL. > > > > Pavel > > Agreed, > > Erik > > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 01/35] util: alloc: add macros for implementing automatic cleanup functionality
On Thu, 12 Jul 2018 at 22:09, Erik Skultety 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
> > ---
> > 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?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 02/31] 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 Reviewed-by: Erik Skultety --- cfg.mk | 11 +++ 1 file changed, 11 insertions(+) diff --git a/cfg.mk b/cfg.mk index 6bebd0a..609ae86 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1057,6 +1057,17 @@ sc_prohibit_backslash_alignment: halt='Do not attempt to right-align backslashes' \ $(_sc_search_regexp) +# Some syntax rules pertaining to the usage of cleanup macros +# implementing GNU C's cleanup attribute + +# Rule to ensure that varibales declared using a cleanup macro are +# always initialized. +sc_require_attribute_cleanup_initialization: + @prohibit='VIR_AUTO(FREE|PTR)\(.+\) *[^=]+;' \ + in_vc_files='\.[chx]$$' \ + halt='variable declared with a cleanup macro must be initialized' \ + $(_sc_search_regexp) + # We don't use this feature of maint.mk. prev_version_file = /dev/null -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 01/31] 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
Reviewed-by: Erik Skultety
---
src/util/viralloc.h | 42 ++
1 file changed, 42 insertions(+)
diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 69d0f90..a23aa18 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -596,4 +596,46 @@ void virAllocTestInit(void);
int virAllocTestCount(void);
void virAllocTestOOM(int n, int m);
void virAllocTestHook(void (*func)(int, void*), void *data);
+
+# 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) \
+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 type *
+
#endif /* __VIR_MEMORY_H_ */
--
1.8.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 00/31] use GNU C's cleanup attribute in src/util (batch I)
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 (31): util: alloc: add macros for implementing automatic cleanup functionality cfg.mk: variable initialization when declared with cleanup macro 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 | 11 ++ src/util/iohelper.c | 4 +- src/util/viralloc.h | 42 ++ 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| 51 +++- 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, 292 insertions(+), 465 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 05/31] util: command: remove redundant include directive
viralloc.h is pulled in by vircommand.h Signed-off-by: Sukrit Bhatnagar Reviewed-by: Erik Skultety --- 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 03/31] 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. Alias virString to (char *) so that the new cleanup macros can be used for a list of strings (char **). 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 Reviewed-by: Erik Skultety --- src/util/virstring.h | 5 + 1 file changed, 5 insertions(+) diff --git a/src/util/virstring.h b/src/util/virstring.h index 607ae66..726e02b 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -25,6 +25,9 @@ # include # include "internal.h" +# include "viralloc.h" + +typedef char *virString; char **virStringSplitCount(const char *string, const char *delim, @@ -309,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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
