4:11 p.m.
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
4:11 p.m.
New subject: [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 <skrtbhtngr(a)gmail.com>
---
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
3:19 p.m.
New subject: [libvirt] [PATCH v1 01/18] add macros for implementing automatic cleanup functionality
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 <skrtbhtngr(a)gmail.com>
---
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
- we are only going to use this with a single variable (or object if you will),
so 'variables' doesn't sound right, how about:
"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. The newly
+ * defined function calls the corresponding pre-defined
"This newly defined function works as a necessary wrapper around @func"
+ * 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.
Same comments as above...
+ */
+# 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
here too...
+ *
+ * Macro to automatically free the memory allocated to
+ * the variable(s) declared with it by calling virFree
s/(s)//
+ * 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
same here...
+ * 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
and here...
+ * 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
Sorry, I missed these in the RFC, now that I read it again, I thought of a few
wording improvements, functionally though, still fine and you have my R-b.
Erik
11:30 p.m.
New subject: [libvirt] [PATCH v1 01/18] add macros for implementing automatic cleanup functionality
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 <skrtbhtngr(a)gmail.com>
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<some_file>.c preparations (especially VIR_{AUTOPTR,AUTOCLEAR} might need
that)
2) Use VIR_AUTOFREE across vir<some_file>.c
3) Use VIR_AUTOPTR across vir<some_file>.c
4) Use VIR_AUTOCLEAR across vir<some_file>.c
...One more thing, the commit subject of every patch in the current series
should probably look like this:
util: <module>: 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
Erik
---
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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
9:12 p.m.
New subject: [libvirt] [PATCH v1 01/18] add macros for implementing automatic cleanup functionality
On Tue, 5 Jun 2018 at 21:00, Erik Skultety <eskultet(a)redhat.com> 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 <skrtbhtngr(a)gmail.com>
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<some_file>.c preparations (especially VIR_{AUTOPTR,AUTOCLEAR} might need
that)
2) Use VIR_AUTOFREE across vir<some_file>.c
3) Use VIR_AUTOPTR across vir<some_file>.c
4) Use VIR_AUTOCLEAR across vir<some_file>.c
...One more thing, the commit subject of every patch in the current series
should probably look like this:
util: <module>: 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 <module> 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<some_file>.h
corresponding to vir<some_file>.c will be in the same series if needed.
--
Thanks,
Sukrit
11:55 p.m.
New subject: [libvirt] [PATCH v1 01/18] add macros for implementing automatic cleanup functionality
On Wed, Jun 06, 2018 at 06:42:29PM +0530, Sukrit Bhatnagar wrote:
On Tue, 5 Jun 2018 at 21:00, Erik Skultety
<eskultet(a)redhat.com> 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 <skrtbhtngr(a)gmail.com>
>
> 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<some_file>.c preparations (especially VIR_{AUTOPTR,AUTOCLEAR} might
need
> that)
> 2) Use VIR_AUTOFREE across vir<some_file>.c
> 3) Use VIR_AUTOPTR across vir<some_file>.c
> 4) Use VIR_AUTOCLEAR across vir<some_file>.c
>
> ...One more thing, the commit subject of every patch in the current series
> should probably look like this:
> util: <module>: 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 <module> 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?
- 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<some_file>.h
corresponding to vir<some_file>.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.
Erik
12:40 a.m.
New subject: [libvirt] [PATCH v1 01/18] add macros for implementing automatic cleanup functionality
On Wed, 6 Jun 2018 at 21:25, Erik Skultety <eskultet(a)redhat.com> wrote:
On Wed, Jun 06, 2018 at 06:42:29PM +0530, Sukrit Bhatnagar wrote:
> On Tue, 5 Jun 2018 at 21:00, Erik Skultety <eskultet(a)redhat.com> 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 <skrtbhtngr(a)gmail.com>
> >
> > 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<some_file>.c preparations (especially VIR_{AUTOPTR,AUTOCLEAR}
might need
> > that)
> > 2) Use VIR_AUTOFREE across vir<some_file>.c
> > 3) Use VIR_AUTOPTR across vir<some_file>.c
> > 4) Use VIR_AUTOCLEAR across vir<some_file>.c
> >
> > ...One more thing, the commit subject of every patch in the current series
> > should probably look like this:
> > util: <module>: 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 <module> 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<some_file>.h
> corresponding to vir<some_file>.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!
4:12 p.m.
New subject: [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 <skrtbhtngr(a)gmail.com>
---
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
4:12 p.m.
New subject: [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 <skrtbhtngr(a)gmail.com>
---
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
3:34 p.m.
New subject: [libvirt] [PATCH v1 03/18] use VIR_AUTOFREE in src/util/virauth.c
On Sun, Jun 03, 2018 at 01:42:01PM +0530, Sukrit Bhatnagar wrote:
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
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;
+ }
This should be left untouched (minus the @ret variable), it's actually better
not to copy the VIR_DEBUG message on multiple places. The same goes for the hunk
below...
Erik
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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
4:12 p.m.
New subject: [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 <skrtbhtngr(a)gmail.com>
---
src/util/virauthconfig.c | 34 ++++++++++++----------------------
1 file changed, 12 insertions(+), 22 deletions(-)
diff --git a/src/util/virauthconfig.c b/src/util/virauthconfig.c
index 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
4:12 p.m.
New subject: [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 <skrtbhtngr(a)gmail.com>
---
src/util/iohelper.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/util/iohelper.c b/src/util/iohelper.c
index bb8a8dd..f7794dc 100644
--- a/src/util/iohelper.c
+++ b/src/util/iohelper.c
@@ -46,7 +46,7 @@
static int
runIO(const char *path, int fd, int oflags)
{
- void *base = NULL; /* Location to be freed */
+ VIR_AUTOFREE(void *) base = NULL; /* Location to be freed */
char *buf = NULL; /* Aligned location within base */
size_t buflen = 1024*1024;
intptr_t alignMask = 64*1024 - 1;
@@ -174,8 +174,6 @@ runIO(const char *path, int fd, int oflags)
virReportSystemError(errno, _("Unable to close %s"), path);
ret = -1;
}
-
- VIR_FREE(base);
return ret;
}
--
1.8.3.1
4:12 p.m.
New subject: [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 <skrtbhtngr(a)gmail.com>
---
src/util/viraudit.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/util/viraudit.c b/src/util/viraudit.c
index 0085dc3..a49d458 100644
--- a/src/util/viraudit.c
+++ b/src/util/viraudit.c
@@ -97,7 +97,7 @@ void virAuditSend(virLogSourcePtr source,
virAuditRecordType type ATTRIBUTE_UNUSED, bool success,
const char *fmt, ...)
{
- char *str = NULL;
+ VIR_AUTOFREE(char *) str = NULL;
va_list args;
/* Duplicate later checks, to short circuit & avoid printf overhead
@@ -144,7 +144,6 @@ void virAuditSend(virLogSourcePtr source,
}
}
#endif
- VIR_FREE(str);
}
void virAuditClose(void)
--
1.8.3.1
4:12 p.m.
New subject: [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 <skrtbhtngr(a)gmail.com>
---
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
5:08 p.m.
New subject: [libvirt] [PATCH v1 07/18] use VIR_AUTOFREE in src/util/virbuffer.c
On Sun, Jun 03, 2018 at 01:42:05PM +0530, Sukrit Bhatnagar wrote:
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
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;
During the design discussion, I said I was fine with ^this usage and I still
kinda am, but at the same time Andrea argued in the same thread [1] that we
should stay consistent here in terms that every virSomething should have a
corresponding virSomethingFree method even if it ends up just calling VIR_FREE.
And he's right in that it could easily turn against us, simply because it can
look a bit fishy that we're calling plain VIR_FREE on the structure pointer
rather than having a dedicated cleanup for the compound type at first glance, so
you have to double check whether it's really okay to use it this way.
So unless someone else comes and says that they're absolutely fine with making
exceptions like this (although consistency is key to a readable and
maintainable code), I'd say let's shoot for consistency, create a static
virBufferEscapePairFree helper and declare VIR_AUTOFREE as a macro taking care
of automatic freeing of *scalar* resources.
[1] https://www.redhat.com/archives/libvir-list/2018-May/msg01910.html
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;
so we basically replaced one goto statement for 2 statements where the va_end
statement is very easily to be forgotten... the cleanup label should stay in
place here, calling va_end and return since the frees will be taken care of.
Erik
}
}
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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
4:12 p.m.
New subject: [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 <skrtbhtngr(a)gmail.com>
---
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) < 0)
@@ -800,18 +787,14 @@ virCgroupSetValueStr(virCgroupPtr group,
virReportSystemError(errno,
_("Invalid value '%s' for
'%s'"),
value, tmp + 1);
- goto cleanup;
+ return -1;
}
virReportSystemError(errno,
_("Unable to write to '%s'"), keypath);
- goto cleanup;
+ return -1;
}
- ret = 0;
-
- cleanup:
- VIR_FREE(keypath);
- return ret;
+ return 0;
}
@@ -821,8 +804,8 @@ virCgroupGetValueStr(virCgroupPtr group,
const char *key,
char **value)
{
- char *keypath = NULL;
- int ret = -1, rc;
+ VIR_AUTOFREE(char *) keypath = NULL;
+ int rc;
*value = NULL;
@@ -834,18 +817,14 @@ virCgroupGetValueStr(virCgroupPtr group,
if ((rc = virFileReadAll(keypath, 1024*1024, value)) < 0) {
virReportSystemError(errno,
_("Unable to read from '%s'"), keypath);
- goto cleanup;
+ return -1;
}
/* Terminated with '\n' has sometimes harmful effects to the caller */
if (rc > 0 && (*value)[rc - 1] == '\n')
(*value)[rc - 1] = '\0';
- ret = 0;
-
- cleanup:
- VIR_FREE(keypath);
- return ret;
+ return 0;
}
@@ -856,8 +835,8 @@ virCgroupGetValueForBlkDev(virCgroupPtr group,
const char *path,
char **value)
{
- char *prefix = NULL;
- char *str = NULL;
+ VIR_AUTOFREE(char *) prefix = NULL;
+ VIR_AUTOFREE(char *) str = NULL;
char **lines = NULL;
int ret = -1;
@@ -875,8 +854,6 @@ virCgroupGetValueForBlkDev(virCgroupPtr group,
ret = 0;
error:
- VIR_FREE(str);
- VIR_FREE(prefix);
virStringListFree(lines);
return ret;
}
@@ -888,17 +865,12 @@ virCgroupSetValueU64(virCgroupPtr group,
const char *key,
unsigned long long int value)
{
- char *strval = NULL;
- int ret;
+ VIR_AUTOFREE(char *) strval = NULL;
if (virAsprintf(&strval, "%llu", value) < 0)
return -1;
- ret = virCgroupSetValueStr(group, controller, key, strval);
-
- VIR_FREE(strval);
-
- return ret;
+ return virCgroupSetValueStr(group, controller, key, strval);
}
@@ -908,17 +880,12 @@ virCgroupSetValueI64(virCgroupPtr group,
const char *key,
long long int value)
{
- char *strval = NULL;
- int ret;
+ VIR_AUTOFREE(char *) strval = NULL;
if (virAsprintf(&strval, "%lld", value) < 0)
return -1;
- ret = virCgroupSetValueStr(group, controller, key, strval);
-
- VIR_FREE(strval);
-
- return ret;
+ return virCgroupSetValueStr(group, controller, key, strval);
}
@@ -928,24 +895,19 @@ virCgroupGetValueI64(virCgroupPtr group,
const char *key,
long long int *value)
{
- char *strval = NULL;
- int ret = -1;
+ VIR_AUTOFREE(char *) strval = NULL;
if (virCgroupGetValueStr(group, controller, key, &strval) < 0)
- goto cleanup;
+ return -1;
if (virStrToLong_ll(strval, NULL, 10, value) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to parse '%s' as an integer"),
strval);
- goto cleanup;
+ return -1;
}
- ret = 0;
-
- cleanup:
- VIR_FREE(strval);
- return ret;
+ return 0;
}
@@ -955,24 +917,19 @@ virCgroupGetValueU64(virCgroupPtr group,
const char *key,
unsigned long long int *value)
{
- char *strval = NULL;
- int ret = -1;
+ VIR_AUTOFREE(char *) strval = NULL;
if (virCgroupGetValueStr(group, controller, key, &strval) < 0)
- goto cleanup;
+ return -1;
if (virStrToLong_ull(strval, NULL, 10, value) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to parse '%s' as an integer"),
strval);
- goto cleanup;
+ return -1;
}
- ret = 0;
-
- cleanup:
- VIR_FREE(strval);
- return ret;
+ return 0;
}
@@ -988,7 +945,7 @@ virCgroupCpuSetInherit(virCgroupPtr parent, virCgroupPtr group)
VIR_DEBUG("Setting up inheritance %s -> %s", parent->path,
group->path);
for (i = 0; i < ARRAY_CARDINALITY(inherit_values); i++) {
- char *value;
+ VIR_AUTOFREE(char *) value = NULL;
if (virCgroupGetValueStr(parent,
VIR_CGROUP_CONTROLLER_CPUSET,
@@ -1001,11 +958,8 @@ virCgroupCpuSetInherit(virCgroupPtr parent, virCgroupPtr group)
if (virCgroupSetValueStr(group,
VIR_CGROUP_CONTROLLER_CPUSET,
inherit_values[i],
- value) < 0) {
- VIR_FREE(value);
+ value) < 0)
return -1;
- }
- VIR_FREE(value);
}
return 0;
@@ -1044,11 +998,10 @@ virCgroupMakeGroup(virCgroupPtr parent,
unsigned int flags)
{
size_t i;
- int ret = -1;
VIR_DEBUG("Make group %s", group->path);
for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
- char *path = NULL;
+ VIR_AUTOFREE(char *) path = NULL;
/* We must never mkdir() in systemd's hierarchy */
if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) {
@@ -1074,10 +1027,8 @@ virCgroupMakeGroup(virCgroupPtr parent,
if (!virFileExists(path)) {
if (!create ||
mkdir(path, 0755) < 0) {
- if (errno == EEXIST) {
- VIR_FREE(path);
+ if (errno == EEXIST)
continue;
- }
/* With a kernel that doesn't support multi-level directory
* for blkio controller, libvirt will fail and disable all
* other controllers even though they are available. So
@@ -1085,24 +1036,20 @@ virCgroupMakeGroup(virCgroupPtr parent,
if (i == VIR_CGROUP_CONTROLLER_BLKIO) {
VIR_DEBUG("Ignoring mkdir failure with blkio controller. Kernel
probably too old");
VIR_FREE(group->controllers[i].mountPoint);
- VIR_FREE(path);
continue;
} else {
virReportSystemError(errno,
_("Failed to create controller %s for
group"),
virCgroupControllerTypeToString(i));
- VIR_FREE(path);
- goto cleanup;
+ return -1;
}
}
if (group->controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint != NULL
&&
(i == VIR_CGROUP_CONTROLLER_CPUSET ||
STREQ(group->controllers[i].mountPoint,
group->controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint)))
{
- if (virCgroupCpuSetInherit(parent, group) < 0) {
- VIR_FREE(path);
- goto cleanup;
- }
+ if (virCgroupCpuSetInherit(parent, group) < 0)
+ return -1;
}
/*
* Note that virCgroupSetMemoryUseHierarchy should always be
@@ -1113,21 +1060,14 @@ virCgroupMakeGroup(virCgroupPtr parent,
(i == VIR_CGROUP_CONTROLLER_MEMORY ||
STREQ(group->controllers[i].mountPoint,
group->controllers[VIR_CGROUP_CONTROLLER_MEMORY].mountPoint)))
{
- if (virCgroupSetMemoryUseHierarchy(group) < 0) {
- VIR_FREE(path);
- goto cleanup;
- }
+ if (virCgroupSetMemoryUseHierarchy(group) < 0)
+ return -1;
}
}
-
- VIR_FREE(path);
}
VIR_DEBUG("Done making controllers for group");
- ret = 0;
-
- cleanup:
- return ret;
+ return 0;
}
@@ -1339,9 +1279,9 @@ virCgroupNewPartition(const char *path,
virCgroupPtr *group)
{
int ret = -1;
- char *parentPath = NULL;
+ VIR_AUTOFREE(char *) parentPath = NULL;
virCgroupPtr parent = NULL;
- char *newPath = NULL;
+ VIR_AUTOFREE(char *) newPath = NULL;
VIR_DEBUG("path=%s create=%d controllers=%x",
path, create, controllers);
@@ -1381,8 +1321,6 @@ virCgroupNewPartition(const char *path,
if (ret != 0)
virCgroupFree(group);
virCgroupFree(&parent);
- VIR_FREE(parentPath);
- VIR_FREE(newPath);
return ret;
}
@@ -1421,18 +1359,17 @@ virCgroupNewDomainPartition(virCgroupPtr partition,
bool create,
virCgroupPtr *group)
{
- int ret = -1;
- char *grpname = NULL;
+ VIR_AUTOFREE(char *)grpname = NULL;
if (virAsprintf(&grpname, "%s.libvirt-%s",
name, driver) < 0)
- goto cleanup;
+ return -1;
if (virCgroupPartitionEscape(&grpname) < 0)
- goto cleanup;
+ return -1;
if (virCgroupNew(-1, grpname, partition, -1, group) < 0)
- goto cleanup;
+ return -1;
/*
* Create a cgroup with memory.use_hierarchy enabled to
@@ -1448,14 +1385,10 @@ virCgroupNewDomainPartition(virCgroupPtr partition,
VIR_CGROUP_MEM_HIERACHY) < 0) {
virCgroupRemove(*group);
virCgroupFree(group);
- goto cleanup;
+ return -1;
}
- ret = 0;
-
- cleanup:
- VIR_FREE(grpname);
- return ret;
+ return 0;
}
@@ -1477,27 +1410,26 @@ virCgroupNewThread(virCgroupPtr domain,
bool create,
virCgroupPtr *group)
{
- int ret = -1;
- char *name = NULL;
+ VIR_AUTOFREE(char *) name = NULL;
int controllers;
switch (nameval) {
case VIR_CGROUP_THREAD_VCPU:
if (virAsprintf(&name, "vcpu%d", id) < 0)
- goto cleanup;
+ return -1;
break;
case VIR_CGROUP_THREAD_EMULATOR:
if (VIR_STRDUP(name, "emulator") < 0)
- goto cleanup;
+ return -1;
break;
case VIR_CGROUP_THREAD_IOTHREAD:
if (virAsprintf(&name, "iothread%d", id) < 0)
- goto cleanup;
+ return -1;
break;
case VIR_CGROUP_THREAD_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected name value %d"), nameval);
- goto cleanup;
+ return -1;
}
controllers = ((1 << VIR_CGROUP_CONTROLLER_CPU) |
@@ -1505,18 +1437,15 @@ virCgroupNewThread(virCgroupPtr domain,
(1 << VIR_CGROUP_CONTROLLER_CPUSET));
if (virCgroupNew(-1, name, domain, controllers, group) < 0)
- goto cleanup;
+ return -1;
if (virCgroupMakeGroup(domain, *group, create, VIR_CGROUP_NONE) < 0) {
virCgroupRemove(*group);
virCgroupFree(group);
- goto cleanup;
+ return -1;
}
- ret = 0;
- cleanup:
- VIR_FREE(name);
- return ret;
+ return 0;
}
@@ -1577,7 +1506,7 @@ virCgroupNewMachineSystemd(const char *name,
int ret = -1;
int rv;
virCgroupPtr init, parent = NULL;
- char *path = NULL;
+ VIR_AUTOFREE(char *) path = NULL;
char *offset;
VIR_DEBUG("Trying to setup machine '%s' via systemd", name);
@@ -1662,7 +1591,6 @@ virCgroupNewMachineSystemd(const char *name,
ret = 0;
cleanup:
virCgroupFree(&parent);
- VIR_FREE(path);
return ret;
}
@@ -1894,9 +1822,10 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group,
long long *requests_write)
{
long long stats_val;
- char *str1 = NULL, *str2 = NULL, *p1, *p2;
+ VIR_AUTOFREE(char *) str1 = NULL;
+ VIR_AUTOFREE(char *) str2 = NULL;
+ char *p1, *p2;
size_t i;
- int ret = -1;
const char *value_names[] = {
"Read ",
@@ -1919,12 +1848,12 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group,
if (virCgroupGetValueStr(group,
VIR_CGROUP_CONTROLLER_BLKIO,
"blkio.throttle.io_service_bytes", &str1) <
0)
- goto cleanup;
+ return -1;
if (virCgroupGetValueStr(group,
VIR_CGROUP_CONTROLLER_BLKIO,
"blkio.throttle.io_serviced", &str2) < 0)
- goto cleanup;
+ return -1;
/* sum up all entries of the same kind, from all devices */
for (i = 0; i < ARRAY_CARDINALITY(value_names); i++) {
@@ -1938,7 +1867,7 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group,
_("Cannot parse byte %sstat '%s'"),
value_names[i],
p1);
- goto cleanup;
+ return -1;
}
if (stats_val < 0 ||
@@ -1947,7 +1876,7 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group,
virReportError(VIR_ERR_OVERFLOW,
_("Sum of byte %sstat overflows"),
value_names[i]);
- goto cleanup;
+ return -1;
}
*bytes_ptrs[i] += stats_val;
}
@@ -1959,7 +1888,7 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group,
_("Cannot parse %srequest stat '%s'"),
value_names[i],
p2);
- goto cleanup;
+ return -1;
}
if (stats_val < 0 ||
@@ -1968,18 +1897,13 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group,
virReportError(VIR_ERR_OVERFLOW,
_("Sum of %srequest stat overflows"),
value_names[i]);
- goto cleanup;
+ return -1;
}
*requests_ptrs[i] += stats_val;
}
}
- ret = 0;
-
- cleanup:
- VIR_FREE(str2);
- VIR_FREE(str1);
- return ret;
+ return 0;
}
@@ -2003,9 +1927,11 @@ virCgroupGetBlkioIoDeviceServiced(virCgroupPtr group,
long long *requests_read,
long long *requests_write)
{
- char *str1 = NULL, *str2 = NULL, *str3 = NULL, *p1, *p2;
+ VIR_AUTOFREE(char *) str1 = NULL;
+ VIR_AUTOFREE(char *) str2 = NULL;
+ VIR_AUTOFREE(char *) str3 = NULL;
+ char *p1, *p2;
size_t i;
- int ret = -1;
const char *value_names[] = {
"Read ",
@@ -2023,28 +1949,28 @@ virCgroupGetBlkioIoDeviceServiced(virCgroupPtr group,
if (virCgroupGetValueStr(group,
VIR_CGROUP_CONTROLLER_BLKIO,
"blkio.throttle.io_service_bytes", &str1) <
0)
- goto cleanup;
+ return -1;
if (virCgroupGetValueStr(group,
VIR_CGROUP_CONTROLLER_BLKIO,
"blkio.throttle.io_serviced", &str2) < 0)
- goto cleanup;
+ return -1;
if (!(str3 = virCgroupGetBlockDevString(path)))
- goto cleanup;
+ return -1;
if (!(p1 = strstr(str1, str3))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Cannot find byte stats for block device
'%s'"),
str3);
- goto cleanup;
+ return -1;
}
if (!(p2 = strstr(str2, str3))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Cannot find request stats for block device
'%s'"),
str3);
- goto cleanup;
+ return -1;
}
for (i = 0; i < ARRAY_CARDINALITY(value_names); i++) {
@@ -2052,38 +1978,32 @@ virCgroupGetBlkioIoDeviceServiced(virCgroupPtr group,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Cannot find byte %sstats for block device
'%s'"),
value_names[i], str3);
- goto cleanup;
+ return -1;
}
if (virStrToLong_ll(p1 + strlen(value_names[i]), &p1, 10, bytes_ptrs[i]) <
0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Cannot parse %sstat '%s'"),
value_names[i], p1 + strlen(value_names[i]));
- goto cleanup;
+ return -1;
}
if (!(p2 = strstr(p2, value_names[i]))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Cannot find request %sstats for block device
'%s'"),
value_names[i], str3);
- goto cleanup;
+ return -1;
}
if (virStrToLong_ll(p2 + strlen(value_names[i]), &p2, 10, requests_ptrs[i])
< 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Cannot parse %sstat '%s'"),
value_names[i], p2 + strlen(value_names[i]));
- goto cleanup;
+ return -1;
}
}
- ret = 0;
-
- cleanup:
- VIR_FREE(str3);
- VIR_FREE(str2);
- VIR_FREE(str1);
- return ret;
+ return 0;
}
@@ -2139,24 +2059,19 @@ virCgroupSetBlkioDeviceReadIops(virCgroupPtr group,
const char *path,
unsigned int riops)
{
- char *str = NULL;
- char *blkstr = NULL;
- int ret = -1;
+ VIR_AUTOFREE(char *) str = NULL;
+ VIR_AUTOFREE(char *) blkstr = NULL;
if (!(blkstr = virCgroupGetBlockDevString(path)))
return -1;
if (virAsprintf(&str, "%s%u", blkstr, riops) < 0)
- goto error;
+ return -1;
- ret = virCgroupSetValueStr(group,
+ return virCgroupSetValueStr(group,
VIR_CGROUP_CONTROLLER_BLKIO,
"blkio.throttle.read_iops_device",
str);
- error:
- VIR_FREE(blkstr);
- VIR_FREE(str);
- return ret;
}
@@ -2173,24 +2088,19 @@ virCgroupSetBlkioDeviceWriteIops(virCgroupPtr group,
const char *path,
unsigned int wiops)
{
- char *str = NULL;
- char *blkstr = NULL;
- int ret = -1;
+ VIR_AUTOFREE(char *) str = NULL;
+ VIR_AUTOFREE(char *) blkstr = NULL;
if (!(blkstr = virCgroupGetBlockDevString(path)))
return -1;
if (virAsprintf(&str, "%s%u", blkstr, wiops) < 0)
- goto error;
+ return -1;
- ret = virCgroupSetValueStr(group,
+ return virCgroupSetValueStr(group,
VIR_CGROUP_CONTROLLER_BLKIO,
"blkio.throttle.write_iops_device",
str);
- error:
- VIR_FREE(blkstr);
- VIR_FREE(str);
- return ret;
}
@@ -2207,24 +2117,19 @@ virCgroupSetBlkioDeviceReadBps(virCgroupPtr group,
const char *path,
unsigned long long rbps)
{
- char *str = NULL;
- char *blkstr = NULL;
- int ret = -1;
+ VIR_AUTOFREE(char *) str = NULL;
+ VIR_AUTOFREE(char *) blkstr = NULL;
if (!(blkstr = virCgroupGetBlockDevString(path)))
return -1;
if (virAsprintf(&str, "%s%llu", blkstr, rbps) < 0)
- goto error;
+ return -1;
- ret = virCgroupSetValueStr(group,
+ return virCgroupSetValueStr(group,
VIR_CGROUP_CONTROLLER_BLKIO,
"blkio.throttle.read_bps_device",
str);
- error:
- VIR_FREE(blkstr);
- VIR_FREE(str);
- return ret;
}
/**
@@ -2240,24 +2145,19 @@ virCgroupSetBlkioDeviceWriteBps(virCgroupPtr group,
const char *path,
unsigned long long wbps)
{
- char *str = NULL;
- char *blkstr = NULL;
- int ret = -1;
+ VIR_AUTOFREE(char *) str = NULL;
+ VIR_AUTOFREE(char *) blkstr = NULL;
if (!(blkstr = virCgroupGetBlockDevString(path)))
return -1;
if (virAsprintf(&str, "%s%llu", blkstr, wbps) < 0)
- goto error;
+ return -1;
- ret = virCgroupSetValueStr(group,
+ return virCgroupSetValueStr(group,
VIR_CGROUP_CONTROLLER_BLKIO,
"blkio.throttle.write_bps_device",
str);
- error:
- VIR_FREE(blkstr);
- VIR_FREE(str);
- return ret;
}
@@ -2275,24 +2175,19 @@ virCgroupSetBlkioDeviceWeight(virCgroupPtr group,
const char *path,
unsigned int weight)
{
- char *str = NULL;
- char *blkstr = NULL;
- int ret = -1;
+ VIR_AUTOFREE(char *) str = NULL;
+ VIR_AUTOFREE(char *) blkstr = NULL;
if (!(blkstr = virCgroupGetBlockDevString(path)))
return -1;
if (virAsprintf(&str, "%s%d", blkstr, weight) < 0)
- goto error;
+ return -1;
- ret = virCgroupSetValueStr(group,
+ return virCgroupSetValueStr(group,
VIR_CGROUP_CONTROLLER_BLKIO,
"blkio.weight_device",
str);
- error:
- VIR_FREE(blkstr);
- VIR_FREE(str);
- return ret;
}
/**
@@ -2308,15 +2203,14 @@ virCgroupGetBlkioDeviceReadIops(virCgroupPtr group,
const char *path,
unsigned int *riops)
{
- char *str = NULL;
- int ret = -1;
+ VIR_AUTOFREE(char *) str = NULL;
if (virCgroupGetValueForBlkDev(group,
VIR_CGROUP_CONTROLLER_BLKIO,
"blkio.throttle.read_iops_device",
path,
&str) < 0)
- goto error;
+ return -1;
if (!str) {
*riops = 0;
@@ -2324,13 +2218,10 @@ virCgroupGetBlkioDeviceReadIops(virCgroupPtr group,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to parse '%s' as an integer"),
str);
- goto error;
+ return -1;
}
- ret = 0;
- error:
- VIR_FREE(str);
- return ret;
+ return 0;
}
/**
@@ -2346,15 +2237,14 @@ virCgroupGetBlkioDeviceWriteIops(virCgroupPtr group,
const char *path,
unsigned int *wiops)
{
- char *str = NULL;
- int ret = -1;
+ VIR_AUTOFREE(char *) str = NULL;
if (virCgroupGetValueForBlkDev(group,
VIR_CGROUP_CONTROLLER_BLKIO,
"blkio.throttle.write_iops_device",
path,
&str) < 0)
- goto error;
+ return -1;
if (!str) {
*wiops = 0;
@@ -2362,13 +2252,10 @@ virCgroupGetBlkioDeviceWriteIops(virCgroupPtr group,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to parse '%s' as an integer"),
str);
- goto error;
+ return -1;
}
- ret = 0;
- error:
- VIR_FREE(str);
- return ret;
+ return 0;
}
/**
@@ -2384,15 +2271,14 @@ virCgroupGetBlkioDeviceReadBps(virCgroupPtr group,
const char *path,
unsigned long long *rbps)
{
- char *str = NULL;
- int ret = -1;
+ VIR_AUTOFREE(char *) str = NULL;
if (virCgroupGetValueForBlkDev(group,
VIR_CGROUP_CONTROLLER_BLKIO,
"blkio.throttle.read_bps_device",
path,
&str) < 0)
- goto error;
+ return -1;
if (!str) {
*rbps = 0;
@@ -2400,13 +2286,10 @@ virCgroupGetBlkioDeviceReadBps(virCgroupPtr group,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to parse '%s' as an integer"),
str);
- goto error;
+ return -1;
}
- ret = 0;
- error:
- VIR_FREE(str);
- return ret;
+ return 0;
}
/**
@@ -2422,15 +2305,14 @@ virCgroupGetBlkioDeviceWriteBps(virCgroupPtr group,
const char *path,
unsigned long long *wbps)
{
- char *str = NULL;
- int ret = -1;
+ VIR_AUTOFREE(char *) str = NULL;
if (virCgroupGetValueForBlkDev(group,
VIR_CGROUP_CONTROLLER_BLKIO,
"blkio.throttle.write_bps_device",
path,
&str) < 0)
- goto error;
+ return -1;
if (!str) {
*wbps = 0;
@@ -2438,13 +2320,10 @@ virCgroupGetBlkioDeviceWriteBps(virCgroupPtr group,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to parse '%s' as an integer"),
str);
- goto error;
+ return -1;
}
- ret = 0;
- error:
- VIR_FREE(str);
- return ret;
+ return 0;
}
/**
@@ -2460,15 +2339,14 @@ virCgroupGetBlkioDeviceWeight(virCgroupPtr group,
const char *path,
unsigned int *weight)
{
- char *str = NULL;
- int ret = -1;
+ VIR_AUTOFREE(char *) str = NULL;
if (virCgroupGetValueForBlkDev(group,
VIR_CGROUP_CONTROLLER_BLKIO,
"blkio.weight_device",
path,
&str) < 0)
- goto error;
+ return -1;
if (!str) {
*weight = 0;
@@ -2476,13 +2354,10 @@ virCgroupGetBlkioDeviceWeight(virCgroupPtr group,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to parse '%s' as an integer"),
str);
- goto error;
+ return -1;
}
- ret = 0;
- error:
- VIR_FREE(str);
- return ret;
+ return 0;
}
@@ -2941,36 +2816,29 @@ int
virCgroupAllowDevice(virCgroupPtr group, char type, int major, int minor,
int perms)
{
- int ret = -1;
- char *devstr = NULL;
- char *majorstr = NULL;
- char *minorstr = NULL;
+ VIR_AUTOFREE(char *) devstr = NULL;
+ VIR_AUTOFREE(char *) majorstr = NULL;
+ VIR_AUTOFREE(char *) minorstr = NULL;
if ((major < 0 && VIR_STRDUP(majorstr, "*") < 0) ||
(major >= 0 && virAsprintf(&majorstr, "%i", major) <
0))
- goto cleanup;
+ return -1;
if ((minor < 0 && VIR_STRDUP(minorstr, "*") < 0) ||
(minor >= 0 && virAsprintf(&minorstr, "%i", minor) <
0))
- goto cleanup;
+ return -1;
if (virAsprintf(&devstr, "%c %s:%s %s", type, majorstr, minorstr,
virCgroupGetDevicePermsString(perms)) < 0)
- goto cleanup;
+ return -1;
if (virCgroupSetValueStr(group,
VIR_CGROUP_CONTROLLER_DEVICES,
"devices.allow",
devstr) < 0)
- goto cleanup;
-
- ret = 0;
+ return -1;
- cleanup:
- VIR_FREE(devstr);
- VIR_FREE(majorstr);
- VIR_FREE(minorstr);
- return ret;
+ return 0;
}
@@ -3032,36 +2900,29 @@ int
virCgroupDenyDevice(virCgroupPtr group, char type, int major, int minor,
int perms)
{
- int ret = -1;
- char *devstr = NULL;
- char *majorstr = NULL;
- char *minorstr = NULL;
+ VIR_AUTOFREE(char *) devstr = NULL;
+ VIR_AUTOFREE(char *) majorstr = NULL;
+ VIR_AUTOFREE(char *) minorstr = NULL;
if ((major < 0 && VIR_STRDUP(majorstr, "*") < 0) ||
(major >= 0 && virAsprintf(&majorstr, "%i", major) <
0))
- goto cleanup;
+ return -1;
if ((minor < 0 && VIR_STRDUP(minorstr, "*") < 0) ||
(minor >= 0 && virAsprintf(&minorstr, "%i", minor) <
0))
- goto cleanup;
+ return -1;
if (virAsprintf(&devstr, "%c %s:%s %s", type, majorstr, minorstr,
virCgroupGetDevicePermsString(perms)) < 0)
- goto cleanup;
+ return -1;
if (virCgroupSetValueStr(group,
VIR_CGROUP_CONTROLLER_DEVICES,
"devices.deny",
devstr) < 0)
- goto cleanup;
-
- ret = 0;
+ return -1;
- cleanup:
- VIR_FREE(devstr);
- VIR_FREE(majorstr);
- VIR_FREE(minorstr);
- return ret;
+ return 0;
}
@@ -3131,10 +2992,10 @@ virCgroupGetPercpuVcpuSum(virCgroupPtr group,
{
int ret = -1;
ssize_t i = -1;
- char *buf = NULL;
virCgroupPtr group_vcpu = NULL;
while ((i = virBitmapNextSetBit(guestvcpus, i)) >= 0) {
+ VIR_AUTOFREE(char *) buf = NULL;
char *pos;
unsigned long long tmp;
ssize_t j;
@@ -3159,13 +3020,11 @@ virCgroupGetPercpuVcpuSum(virCgroupPtr group,
}
virCgroupFree(&group_vcpu);
- VIR_FREE(buf);
}
ret = 0;
cleanup:
virCgroupFree(&group_vcpu);
- VIR_FREE(buf);
return ret;
}
@@ -3202,8 +3061,8 @@ virCgroupGetPercpuStats(virCgroupPtr group,
size_t i;
int need_cpus, total_cpus;
char *pos;
- char *buf = NULL;
- unsigned long long *sum_cpu_time = NULL;
+ VIR_AUTOFREE(char *) buf = NULL;
+ VIR_AUTOFREE(unsigned long long *) sum_cpu_time = NULL;
virTypedParameterPtr ent;
int param_idx;
unsigned long long cpu_time;
@@ -3289,8 +3148,6 @@ virCgroupGetPercpuStats(virCgroupPtr group,
cleanup:
virBitmapFree(cpumap);
- VIR_FREE(sum_cpu_time);
- VIR_FREE(buf);
return ret;
}
@@ -3461,7 +3318,7 @@ virCgroupRemoveRecursively(char *grppath)
/* This is best-effort cleanup: we want to log failures with just
* VIR_ERROR instead of normal virReportError */
while ((direrr = virDirRead(grpdir, &ent, NULL)) > 0) {
- char *path;
+ VIR_AUTOFREE(char *) path = NULL;
if (ent->d_type != DT_DIR) continue;
@@ -3470,7 +3327,6 @@ virCgroupRemoveRecursively(char *grppath)
break;
}
rc = virCgroupRemoveRecursively(path);
- VIR_FREE(path);
if (rc != 0)
break;
}
@@ -3508,10 +3364,11 @@ virCgroupRemove(virCgroupPtr group)
{
int rc = 0;
size_t i;
- char *grppath = NULL;
VIR_DEBUG("Removing cgroup %s", group->path);
for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
+ VIR_AUTOFREE(char *) grppath = NULL;
+
/* Skip over controllers not mounted */
if (!group->controllers[i].mountPoint)
continue;
@@ -3533,7 +3390,6 @@ virCgroupRemove(virCgroupPtr group)
VIR_DEBUG("Removing cgroup %s and all child cgroups", grppath);
rc = virCgroupRemoveRecursively(grppath);
- VIR_FREE(grppath);
}
VIR_DEBUG("Done removing cgroup %s", group->path);
@@ -3549,7 +3405,7 @@ virCgroupKillInternal(virCgroupPtr group, int signum,
virHashTablePtr pids)
{
int ret = -1;
bool killedAny = false;
- char *keypath = NULL;
+ VIR_AUTOFREE(char *) keypath = NULL;
bool done = false;
FILE *fp = NULL;
VIR_DEBUG("group=%p path=%s signum=%d pids=%p",
@@ -3613,7 +3469,6 @@ virCgroupKillInternal(virCgroupPtr group, int signum,
virHashTablePtr pids)
ret = killedAny ? 1 : 0;
cleanup:
- VIR_FREE(keypath);
VIR_FORCE_FCLOSE(fp);
return ret;
@@ -3678,7 +3533,7 @@ virCgroupKillRecursiveInternal(virCgroupPtr group,
int ret = -1;
int rc;
bool killedAny = false;
- char *keypath = NULL;
+ VIR_AUTOFREE(char *) keypath = NULL;
DIR *dp = NULL;
virCgroupPtr subgroup = NULL;
struct dirent *ent;
@@ -3732,7 +3587,6 @@ virCgroupKillRecursiveInternal(virCgroupPtr group,
cleanup:
virCgroupFree(&subgroup);
- VIR_FREE(keypath);
VIR_DIR_CLOSE(dp);
return ret;
}
@@ -3846,9 +3700,8 @@ int
virCgroupGetCpuacctStat(virCgroupPtr group, unsigned long long *user,
unsigned long long *sys)
{
- char *str;
+ VIR_AUTOFREE(char *) str = NULL;
char *p;
- int ret = -1;
static double scale = -1.0;
if (virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_CPUACCT,
@@ -3860,14 +3713,14 @@ virCgroupGetCpuacctStat(virCgroupPtr group, unsigned long long
*user,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Cannot parse user stat '%s'"),
p);
- goto cleanup;
+ return -1;
}
if (!(p = STRSKIP(p, "\nsystem ")) ||
virStrToLong_ull(p, NULL, 10, sys) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Cannot parse sys stat '%s'"),
p);
- goto cleanup;
+ return -1;
}
/* times reported are in system ticks (generally 100 Hz), but that
* rate can theoretically vary between machines. Scale things
@@ -3877,17 +3730,14 @@ virCgroupGetCpuacctStat(virCgroupPtr group, unsigned long long
*user,
if (ticks_per_sec == -1) {
virReportSystemError(errno, "%s",
_("Cannot determine system clock HZ"));
- goto cleanup;
+ return -1;
}
scale = 1000000000.0 / ticks_per_sec;
}
*user *= scale;
*sys *= scale;
- ret = 0;
- cleanup:
- VIR_FREE(str);
- return ret;
+ return 0;
}
@@ -3913,10 +3763,9 @@ int
virCgroupBindMount(virCgroupPtr group, const char *oldroot,
const char *mountopts)
{
- int ret = -1;
size_t i;
- char *opts = NULL;
- char *root = NULL;
+ VIR_AUTOFREE(char *) opts = NULL;
+ VIR_AUTOFREE(char *) root = NULL;
if (!(root = virCgroupIdentifyRoot(group)))
return -1;
@@ -3927,18 +3776,18 @@ virCgroupBindMount(virCgroupPtr group, const char *oldroot,
virReportSystemError(errno,
_("Unable to create directory %s"),
root);
- goto cleanup;
+ return -1;
}
if (virAsprintf(&opts,
"mode=755,size=65536%s", mountopts) < 0)
- goto cleanup;
+ return -1;
if (mount("tmpfs", root, "tmpfs", MS_NOSUID|MS_NODEV|MS_NOEXEC,
opts) < 0) {
virReportSystemError(errno,
_("Failed to mount %s on %s type %s"),
"tmpfs", root, "tmpfs");
- goto cleanup;
+ return -1;
}
for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
@@ -3946,11 +3795,11 @@ virCgroupBindMount(virCgroupPtr group, const char *oldroot,
continue;
if (!virFileExists(group->controllers[i].mountPoint)) {
- char *src;
+ VIR_AUTOFREE(char *) src = NULL;
if (virAsprintf(&src, "%s%s",
oldroot,
group->controllers[i].mountPoint) < 0)
- goto cleanup;
+ return -1;
VIR_DEBUG("Create mount point '%s'",
group->controllers[i].mountPoint);
@@ -3958,8 +3807,7 @@ virCgroupBindMount(virCgroupPtr group, const char *oldroot,
virReportSystemError(errno,
_("Unable to create directory %s"),
group->controllers[i].mountPoint);
- VIR_FREE(src);
- goto cleanup;
+ return -1;
}
if (mount(src, group->controllers[i].mountPoint, NULL, MS_BIND,
@@ -3967,11 +3815,8 @@ virCgroupBindMount(virCgroupPtr group, const char *oldroot,
virReportSystemError(errno,
_("Failed to bind cgroup '%s' on
'%s'"),
src, group->controllers[i].mountPoint);
- VIR_FREE(src);
- goto cleanup;
+ return -1;
}
-
- VIR_FREE(src);
}
if (group->controllers[i].linkPoint) {
@@ -3984,16 +3829,12 @@ virCgroupBindMount(virCgroupPtr group, const char *oldroot,
_("Unable to symlink directory %s to
%s"),
group->controllers[i].mountPoint,
group->controllers[i].linkPoint);
- goto cleanup;
+ return -1;
}
}
}
- ret = 0;
- cleanup:
- VIR_FREE(root);
- VIR_FREE(opts);
- return ret;
+ return 0;
}
@@ -4004,11 +3845,11 @@ int virCgroupSetOwner(virCgroupPtr cgroup,
{
int ret = -1;
size_t i;
- char *base = NULL, *entry = NULL;
DIR *dh = NULL;
int direrr;
for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
+ VIR_AUTOFREE(char *) base = NULL;
struct dirent *de;
if (!((1 << i) & controllers))
@@ -4025,6 +3866,8 @@ int virCgroupSetOwner(virCgroupPtr cgroup,
goto cleanup;
while ((direrr = virDirRead(dh, &de, base)) > 0) {
+ VIR_AUTOFREE(char *) entry = NULL;
+
if (virAsprintf(&entry, "%s/%s", base, de->d_name) < 0)
goto cleanup;
@@ -4034,8 +3877,6 @@ int virCgroupSetOwner(virCgroupPtr cgroup,
entry, uid, gid);
goto cleanup;
}
-
- VIR_FREE(entry);
}
if (direrr < 0)
goto cleanup;
@@ -4047,7 +3888,6 @@ int virCgroupSetOwner(virCgroupPtr cgroup,
goto cleanup;
}
- VIR_FREE(base);
VIR_DIR_CLOSE(dh);
}
@@ -4055,8 +3895,6 @@ int virCgroupSetOwner(virCgroupPtr cgroup,
cleanup:
VIR_DIR_CLOSE(dh);
- VIR_FREE(entry);
- VIR_FREE(base);
return ret;
}
@@ -4071,8 +3909,7 @@ int virCgroupSetOwner(virCgroupPtr cgroup,
bool
virCgroupSupportsCpuBW(virCgroupPtr cgroup)
{
- char *path = NULL;
- bool ret = false;
+ VIR_AUTOFREE(char *) path = NULL;
if (!cgroup)
return false;
@@ -4080,21 +3917,17 @@ virCgroupSupportsCpuBW(virCgroupPtr cgroup)
if (virCgroupPathOfController(cgroup, VIR_CGROUP_CONTROLLER_CPU,
"cpu.cfs_period_us", &path) < 0) {
virResetLastError();
- goto cleanup;
+ return false;
}
- ret = virFileExists(path);
-
- cleanup:
- VIR_FREE(path);
- return ret;
+ return virFileExists(path);
}
int
virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller)
{
- int ret = -1;
- char *content = NULL;
+ int ret;
+ VIR_AUTOFREE(char *) content = NULL;
if (!cgroup)
return -1;
@@ -4104,7 +3937,6 @@ virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller)
if (ret == 0 && content[0] == '\0')
ret = 1;
- VIR_FREE(content);
return ret;
}
--
1.8.3.1
7:23 p.m.
New subject: [libvirt] [PATCH v1 08/18] use VIR_AUTOFREE in src/util/vircgroup.c
On Sun, Jun 03, 2018 at 01:42:06PM +0530, Sukrit Bhatnagar wrote:
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/vircgroup.c | 526 ++++++++++++++++++---------------------------------
1 file changed, 179 insertions(+), 347 deletions(-)
...
@@ -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;
@linksrc is being used only in an 'if' block further in the function body
(the context of which is not seen in the patch), so I'd move the declaration to
that block which will allow us to ditch the VIR_FREE(linksrc) in the same
block.
...
@@ -1339,9 +1279,9 @@ virCgroupNewPartition(const char *path,
virCgroupPtr *group)
{
int ret = -1;
- char *parentPath = NULL;
+ VIR_AUTOFREE(char *) parentPath = NULL;
virCgroupPtr parent = NULL;
- char *newPath = NULL;
+ VIR_AUTOFREE(char *) newPath = NULL;
move ^this one line up, so the AUTOFREEs are nicely packed...
VIR_DEBUG("path=%s create=%d controllers=%x",
path, create, controllers);
@@ -1381,8 +1321,6 @@ virCgroupNewPartition(const char *path,
if (ret != 0)
virCgroupFree(group);
virCgroupFree(&parent);
- VIR_FREE(parentPath);
- VIR_FREE(newPath);
return ret;
}
...
@@ -1577,7 +1506,7 @@ virCgroupNewMachineSystemd(const char *name,
int ret = -1;
int rv;
virCgroupPtr init, parent = NULL;
- char *path = NULL;
+ VIR_AUTOFREE(char *) path = NULL;
char *offset;
VIR_DEBUG("Trying to setup machine '%s' via systemd", name);
@@ -1662,7 +1591,6 @@ virCgroupNewMachineSystemd(const char *name,
ret = 0;
cleanup:
virCgroupFree(&parent);
- VIR_FREE(path);
return ret;
}
@@ -1894,9 +1822,10 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group,
long long *requests_write)
{
long long stats_val;
- char *str1 = NULL, *str2 = NULL, *p1, *p2;
+ VIR_AUTOFREE(char *) str1 = NULL;
+ VIR_AUTOFREE(char *) str2 = NULL;
+ char *p1, *p2;
since you're changing it already, p1 and p2 should be on separate lines...
size_t i;
- int ret = -1;
const char *value_names[] = {
"Read ",
@@ -1919,12 +1848,12 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group,
if (virCgroupGetValueStr(group,
VIR_CGROUP_CONTROLLER_BLKIO,
"blkio.throttle.io_service_bytes", &str1)
< 0)
- goto cleanup;
+ return -1;
if (virCgroupGetValueStr(group,
VIR_CGROUP_CONTROLLER_BLKIO,
"blkio.throttle.io_serviced", &str2) < 0)
- goto cleanup;
+ return -1;
/* sum up all entries of the same kind, from all devices */
for (i = 0; i < ARRAY_CARDINALITY(value_names); i++) {
@@ -1938,7 +1867,7 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group,
_("Cannot parse byte %sstat '%s'"),
value_names[i],
p1);
- goto cleanup;
+ return -1;
}
if (stats_val < 0 ||
@@ -1947,7 +1876,7 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group,
virReportError(VIR_ERR_OVERFLOW,
_("Sum of byte %sstat overflows"),
value_names[i]);
- goto cleanup;
+ return -1;
}
*bytes_ptrs[i] += stats_val;
}
@@ -1959,7 +1888,7 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group,
_("Cannot parse %srequest stat '%s'"),
value_names[i],
p2);
- goto cleanup;
+ return -1;
}
if (stats_val < 0 ||
@@ -1968,18 +1897,13 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group,
virReportError(VIR_ERR_OVERFLOW,
_("Sum of %srequest stat overflows"),
value_names[i]);
- goto cleanup;
+ return -1;
}
*requests_ptrs[i] += stats_val;
}
}
- ret = 0;
-
- cleanup:
- VIR_FREE(str2);
- VIR_FREE(str1);
- return ret;
+ return 0;
}
@@ -2003,9 +1927,11 @@ virCgroupGetBlkioIoDeviceServiced(virCgroupPtr group,
long long *requests_read,
long long *requests_write)
{
- char *str1 = NULL, *str2 = NULL, *str3 = NULL, *p1, *p2;
+ VIR_AUTOFREE(char *) str1 = NULL;
+ VIR_AUTOFREE(char *) str2 = NULL;
+ VIR_AUTOFREE(char *) str3 = NULL;
+ char *p1, *p2;
here too..separate lines...
...
@@ -3131,10 +2992,10 @@ virCgroupGetPercpuVcpuSum(virCgroupPtr group,
{
int ret = -1;
ssize_t i = -1;
- char *buf = NULL;
virCgroupPtr group_vcpu = NULL;
while ((i = virBitmapNextSetBit(guestvcpus, i)) >= 0) {
+ VIR_AUTOFREE(char *) buf = NULL;
char *pos;
more cleanup possible here, since there's no point in having @pos at all, on
the contrary, it's a bit confusing what it's being used for...so @pos should be
dropped in a separate patch and @buf should be used directly, beware though:
virStrToLong_ull(buf, &buf, 10, &tmp) would leak memory, so
virStrToLong_ull(buf, NULL, 10, &tmp) should be used instead...
...
int
virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller)
{
- int ret = -1;
^This is okay, so don't change it...
- char *content = NULL;
+ int ret;
+ VIR_AUTOFREE(char *) content = NULL;
if (!cgroup)
return -1;
@@ -4104,7 +3937,6 @@ virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller)
if (ret == 0 && content[0] == '\0')
ret = 1;
- VIR_FREE(content);
return ret;
}
Erik
4:12 p.m.
New subject: [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 <skrtbhtngr(a)gmail.com>
---
src/util/virfcp.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/src/util/virfcp.c b/src/util/virfcp.c
index 7660ba7..b703744 100644
--- a/src/util/virfcp.c
+++ b/src/util/virfcp.c
@@ -40,16 +40,12 @@
bool
virFCIsCapableRport(const char *rport)
{
- bool ret = false;
- char *path = NULL;
+ VIR_AUTOFREE(char *) path = NULL;
if (virBuildPath(&path, SYSFS_FC_RPORT_PATH, rport) < 0)
return false;
- ret = virFileExists(path);
- VIR_FREE(path);
-
- return ret;
+ return virFileExists(path);
}
int
@@ -57,8 +53,8 @@ virFCReadRportValue(const char *rport,
const char *entry,
char **result)
{
- int ret = -1;
- char *buf = NULL, *p = NULL;
+ VIR_AUTOFREE(char *) buf = NULL;
+ char *p = NULL;
if (virFileReadValueString(&buf, "%s/%s/%s",
SYSFS_FC_RPORT_PATH, rport, entry) < 0) {
@@ -69,13 +65,9 @@ virFCReadRportValue(const char *rport,
*p = '\0';
if (VIR_STRDUP(*result, buf) < 0)
- goto cleanup;
-
- ret = 0;
+ return -1;
- cleanup:
- VIR_FREE(buf);
- return ret;
+ return 0;
}
#else
--
1.8.3.1
4:12 p.m.
New subject: [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 <skrtbhtngr(a)gmail.com>
---
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) {
if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host,
"%s,%s,%s",
mac, ipstr, name) < 0)
- goto error;
+ return -1;
} else if (name && !mac) {
if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host,
"%s,%s",
name, ipstr) < 0)
- goto error;
+ return -1;
} else {
if (virAsprintf(&hostsfile->hosts[hostsfile->nhosts].host,
"%s,%s",
mac, ipstr) < 0)
- goto error;
+ return -1;
}
- VIR_FREE(ipstr);
hostsfile->nhosts++;
return 0;
-
- error:
- VIR_FREE(ipstr);
- return -1;
}
static dnsmasqHostsfile *
@@ -391,11 +369,10 @@ hostsfileWrite(const char *path,
dnsmasqDhcpHost *hosts,
unsigned int nhosts)
{
- char *tmp;
+ VIR_AUTOFREE(char *) tmp = NULL;
FILE *f;
bool istmp = true;
size_t i;
- int rc = 0;
/* even if there are 0 hosts, create a 0 length file, to allow
* for runtime addition.
@@ -406,39 +383,30 @@ hostsfileWrite(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].host, f) == EOF || 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
@@ -708,17 +676,12 @@ dnsmasqCapsSetFromBuffer(dnsmasqCapsPtr caps, const char *buf)
static int
dnsmasqCapsSetFromFile(dnsmasqCapsPtr caps, const char *path)
{
- int ret = -1;
- char *buf = NULL;
+ VIR_AUTOFREE(char *) buf = NULL;
if (virFileReadAll(path, 1024 * 1024, &buf) < 0)
- goto cleanup;
-
- ret = dnsmasqCapsSetFromBuffer(caps, buf);
+ return -1;
- cleanup:
- VIR_FREE(buf);
- return ret;
+ return dnsmasqCapsSetFromBuffer(caps, buf);
}
static int
@@ -727,7 +690,9 @@ dnsmasqCapsRefreshInternal(dnsmasqCapsPtr caps, bool force)
int ret = -1;
struct stat sb;
virCommandPtr cmd = NULL;
- char *help = NULL, *version = NULL, *complete = NULL;
+ VIR_AUTOFREE(char *) help = NULL;
+ VIR_AUTOFREE(char *) version = NULL;
+ VIR_AUTOFREE(char *) complete = NULL;
if (!caps || caps->noRefresh)
return 0;
@@ -779,9 +744,6 @@ dnsmasqCapsRefreshInternal(dnsmasqCapsPtr caps, bool force)
cleanup:
virCommandFree(cmd);
- VIR_FREE(help);
- VIR_FREE(version);
- VIR_FREE(complete);
return ret;
}
--
1.8.3.1
4:12 p.m.
New subject: [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 <skrtbhtngr(a)gmail.com>
---
src/util/vireventpoll.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/util/vireventpoll.c b/src/util/vireventpoll.c
index 81ecab4..13d278d 100644
--- a/src/util/vireventpoll.c
+++ b/src/util/vireventpoll.c
@@ -618,7 +618,7 @@ static void virEventPollCleanupHandles(void)
*/
int virEventPollRunOnce(void)
{
- struct pollfd *fds = NULL;
+ VIR_AUTOFREE(struct pollfd *) fds = NULL;
int ret, timeout, nfds;
virMutexLock(&eventLoop.lock);
@@ -645,7 +645,7 @@ int virEventPollRunOnce(void)
goto retry;
virReportSystemError(errno, "%s",
_("Unable to poll on file handles"));
- goto error_unlocked;
+ return -1;
}
EVENT_DEBUG("Poll got %d event(s)", ret);
@@ -662,13 +662,10 @@ int virEventPollRunOnce(void)
eventLoop.running = 0;
virMutexUnlock(&eventLoop.lock);
- VIR_FREE(fds);
return 0;
error:
virMutexUnlock(&eventLoop.lock);
- error_unlocked:
- VIR_FREE(fds);
return -1;
}
--
1.8.3.1
4:12 p.m.
New subject: [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 <skrtbhtngr(a)gmail.com>
---
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
9:32 p.m.
New subject: [libvirt] [PATCH v1 12/18] use VIR_AUTOFREE in src/util/virdbus.c
On Sun, Jun 03, 2018 at 01:42:10PM +0530, Sukrit Bhatnagar wrote:
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
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;
Doh!...and here we are facing a potential exception to the rule I mentioned in
one of the previous patches about compound types and VIR_AUTOFREE...Sigh,
although possible, I really don't think we want to introduce *Free helpers for
external types like ^this...that's honestly my only argument to support my
earlier claim about the VIR_AUTOFREE usage with scalar types only.
It's not a "libvirt-native" type, so let's close our eyes and forget
that we're
doing this.
Erik
4:12 p.m.
New subject: [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 <skrtbhtngr(a)gmail.com>
---
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
4:12 p.m.
New subject: [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 <skrtbhtngr(a)gmail.com>
---
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_t len)
{
int r;
- char *buf;
+ VIR_AUTOFREE(char *) buf = NULL;
unsigned long long remain, bytes;
if (lseek(fd, offset, SEEK_SET) < 0)
@@ -1187,15 +1170,12 @@ safezero_slow(int fd, off_t offset, off_t len)
bytes = remain;
r = safewrite(fd, buf, bytes);
- if (r < 0) {
- VIR_FREE(buf);
+ if (r < 0)
return -1;
- }
/* safewrite() guarantees all data will be written */
remain -= bytes;
}
- VIR_FREE(buf);
return 0;
}
@@ -1558,8 +1538,7 @@ virFileRelLinkPointsTo(const char *directory,
const char *checkLink,
const char *checkDest)
{
- char *candidate;
- int ret;
+ VIR_AUTOFREE(char *) candidate = NULL;
if (*checkLink == '/')
return virFileLinkPointsTo(checkLink, checkDest);
@@ -1571,9 +1550,8 @@ virFileRelLinkPointsTo(const char *directory,
}
if (virAsprintf(&candidate, "%s/%s", directory, checkLink) < 0)
return -1;
- ret = virFileLinkPointsTo(candidate, checkDest);
- VIR_FREE(candidate);
- return ret;
+
+ return virFileLinkPointsTo(candidate, checkDest);
}
@@ -1897,44 +1875,39 @@ virFileIsExecutable(const char *file)
*/
int virFileIsMountPoint(const char *file)
{
- char *parent = NULL;
- int ret = -1;
+ VIR_AUTOFREE(char *) parent = NULL;
+ int ret;
struct stat sb1, sb2;
if (!(parent = mdir_name(file))) {
virReportOOMError();
- goto cleanup;
+ return -1;
}
VIR_DEBUG("Comparing '%s' to '%s'", file, parent);
if (stat(file, &sb1) < 0) {
if (errno == ENOENT)
- ret = 0;
+ return 0;
else
virReportSystemError(errno,
_("Cannot stat '%s'"),
file);
- goto cleanup;
+ return -1;
}
if (stat(parent, &sb2) < 0) {
virReportSystemError(errno,
_("Cannot stat '%s'"),
parent);
- goto cleanup;
+ return -1;
}
- if (!S_ISDIR(sb1.st_mode)) {
- ret = 0;
- goto cleanup;
- }
+ if (!S_ISDIR(sb1.st_mode))
+ return 0;
ret = sb1.st_dev != sb2.st_dev;
VIR_DEBUG("Is mount %d", ret);
-
- cleanup:
- VIR_FREE(parent);
return ret;
}
@@ -2118,7 +2091,7 @@ virFileAccessibleAs(const char *path, int mode,
pid_t pid = 0;
int status, ret = 0;
int forkRet = 0;
- gid_t *groups;
+ VIR_AUTOFREE(gid_t *) groups = NULL;
int ngroups;
if (uid == geteuid() &&
@@ -2131,13 +2104,10 @@ virFileAccessibleAs(const char *path, int mode,
pid = virFork();
- if (pid < 0) {
- VIR_FREE(groups);
+ if (pid < 0)
return -1;
- }
if (pid) { /* parent */
- VIR_FREE(groups);
if (virProcessWait(pid, &status, false) < 0) {
/* virProcessWait() already reported error */
errno = EINTR;
@@ -2242,7 +2212,7 @@ virFileOpenForked(const char *path, int openflags, mode_t mode,
int recvfd_errno = 0;
int fd = -1;
int pair[2] = { -1, -1 };
- gid_t *groups;
+ VIR_AUTOFREE(gid_t *) groups = NULL;
int ngroups;
bool created = false;
@@ -2260,16 +2230,12 @@ virFileOpenForked(const char *path, int openflags, mode_t mode,
virReportSystemError(errno,
_("failed to create socket needed for
'%s'"),
path);
- VIR_FREE(groups);
return ret;
}
pid = virFork();
- if (pid < 0) {
- ret = -errno;
- VIR_FREE(groups);
- return ret;
- }
+ if (pid < 0)
+ return -errno;
if (pid == 0) {
@@ -2334,7 +2300,6 @@ virFileOpenForked(const char *path, int openflags, mode_t mode,
/* parent */
- VIR_FREE(groups);
VIR_FORCE_CLOSE(pair[1]);
do {
@@ -2535,7 +2500,7 @@ virFileRemove(const char *path,
{
pid_t pid;
int status = 0, ret = 0;
- gid_t *groups;
+ VIR_AUTOFREE(gid_t *) groups = NULL;
int ngroups;
if (!virFileRemoveNeedsSetuid(path, uid, gid)) {
@@ -2560,15 +2525,11 @@ virFileRemove(const char *path,
pid = virFork();
- if (pid < 0) {
- ret = -errno;
- VIR_FREE(groups);
- return ret;
- }
+ if (pid < 0)
+ return -errno;
if (pid) { /* parent */
/* wait for child to complete, and retrieve its exit code */
- VIR_FREE(groups);
if (virProcessWait(pid, &status, 0) < 0) {
/* virProcessWait() reports errno on waitpid failure, so we'll just
@@ -2700,7 +2661,7 @@ virDirCreate(const char *path,
struct stat st;
pid_t pid;
int status = 0, ret = 0;
- gid_t *groups;
+ VIR_AUTOFREE(gid_t *) groups = NULL;
int ngroups;
bool created = false;
@@ -2736,15 +2697,11 @@ virDirCreate(const char *path,
pid = virFork();
- if (pid < 0) {
- ret = -errno;
- VIR_FREE(groups);
- return ret;
- }
+ if (pid < 0)
+ return -errno;
if (pid) { /* parent */
/* wait for child to complete, and retrieve its exit code */
- VIR_FREE(groups);
if (virProcessWait(pid, &status, 0) < 0) {
/* virProcessWait() reports errno on waitpid failure, so we'll just
@@ -3045,19 +3002,14 @@ int
virFileMakePathWithMode(const char *path,
mode_t mode)
{
- int ret = -1;
- char *tmp;
+ VIR_AUTOFREE(char *) tmp = NULL;
if (VIR_STRDUP(tmp, path) < 0) {
errno = ENOMEM;
- goto cleanup;
+ return -1;
}
- ret = virFileMakePathHelper(tmp, mode);
-
- cleanup:
- VIR_FREE(tmp);
- return ret;
+ return virFileMakePathHelper(tmp, mode);
}
@@ -3065,8 +3017,7 @@ int
virFileMakeParentPath(const char *path)
{
char *p;
- char *tmp;
- int ret = -1;
+ VIR_AUTOFREE(char *) tmp = NULL;
VIR_DEBUG("path=%s", path);
@@ -3077,15 +3028,11 @@ virFileMakeParentPath(const char *path)
if ((p = strrchr(tmp, '/')) == NULL) {
errno = EINVAL;
- goto cleanup;
+ return -1;
}
*p = '\0';
- ret = virFileMakePathHelper(tmp, 0777);
-
- cleanup:
- VIR_FREE(tmp);
- return ret;
+ return virFileMakePathHelper(tmp, 0777);
}
@@ -3119,7 +3066,7 @@ virFileOpenTty(int *ttymaster, char **ttyName, int rawmode)
* doesn't have to worry about that mess? */
int ret = -1;
int slave = -1;
- char *name = NULL;
+ VIR_AUTOFREE(char *) name = NULL;
/* Unfortunately, we can't use the name argument of openpty, since
* there is no guarantee on how large the buffer has to be.
@@ -3180,7 +3127,6 @@ virFileOpenTty(int *ttymaster, char **ttyName, int rawmode)
if (ret != 0)
VIR_FORCE_CLOSE(*ttymaster);
VIR_FORCE_CLOSE(slave);
- VIR_FREE(name);
return ret;
}
@@ -3280,7 +3226,7 @@ virFileSkipRoot(const char *path)
int
virFileAbsPath(const char *path, char **abspath)
{
- char *buf;
+ VIR_AUTOFREE(char *) buf = NULL;
if (path[0] == '/') {
if (VIR_STRDUP(*abspath, path) < 0)
@@ -3290,11 +3236,8 @@ virFileAbsPath(const char *path, char **abspath)
if (buf == NULL)
return -1;
- if (virAsprintf(abspath, "%s/%s", buf, path) < 0) {
- VIR_FREE(buf);
+ if (virAsprintf(abspath, "%s/%s", buf, path) < 0)
return -1;
- }
- VIR_FREE(buf);
}
return 0;
@@ -3394,7 +3337,7 @@ virFileRemoveLastComponent(char *path)
int virFilePrintf(FILE *fp, const char *msg, ...)
{
va_list vargs;
- char *str;
+ VIR_AUTOFREE(char *) str = NULL;
int ret;
va_start(vargs, msg);
@@ -3408,8 +3351,6 @@ int virFilePrintf(FILE *fp, const char *msg, ...)
ret = -1;
}
- VIR_FREE(str);
-
cleanup:
va_end(vargs);
@@ -3445,7 +3386,8 @@ int
virFileIsSharedFSType(const char *path,
int fstypes)
{
- char *dirpath, *p;
+ VIR_AUTOFREE(char *) dirpath = NULL;
+ char *p;
struct statfs sb;
int statfs_ret;
@@ -3465,7 +3407,6 @@ virFileIsSharedFSType(const char *path,
if ((p = strrchr(dirpath, '/')) == NULL) {
virReportSystemError(EINVAL,
_("Invalid relative path '%s'"), path);
- VIR_FREE(dirpath);
return -1;
}
@@ -3478,8 +3419,6 @@ virFileIsSharedFSType(const char *path,
} while ((statfs_ret < 0) && (p != dirpath));
- VIR_FREE(dirpath);
-
if (statfs_ret < 0) {
virReportSystemError(errno,
_("cannot determine filesystem for
'%s'"),
@@ -3546,18 +3485,18 @@ virFileGetHugepageSize(const char *path,
static int
virFileGetDefaultHugepageSize(unsigned long long *size)
{
- int ret = -1;
- char *meminfo, *c, *n, *unit;
+ VIR_AUTOFREE(char *) meminfo = NULL;
+ char *c, *n, *unit;
if (virFileReadAll(PROC_MEMINFO, 4096, &meminfo) < 0)
- goto cleanup;
+ return -1;
if (!(c = strstr(meminfo, HUGEPAGESIZE_STR))) {
virReportError(VIR_ERR_NO_SUPPORT,
_("%s not found in %s"),
HUGEPAGESIZE_STR,
PROC_MEMINFO);
- goto cleanup;
+ return -1;
}
c += strlen(HUGEPAGESIZE_STR);
@@ -3570,13 +3509,10 @@ virFileGetDefaultHugepageSize(unsigned long long *size)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to parse %s %s"),
HUGEPAGESIZE_STR, c);
- goto cleanup;
+ return -1;
}
- ret = 0;
- cleanup:
- VIR_FREE(meminfo);
- return ret;
+ return 0;
}
# define PROC_MOUNTS "/proc/mounts"
@@ -3589,7 +3525,7 @@ virFileFindHugeTLBFS(virHugeTLBFSPtr *ret_fs,
FILE *f = NULL;
struct mntent mb;
char mntbuf[1024];
- virHugeTLBFSPtr fs = NULL;
+ VIR_AUTOFREE(virHugeTLBFSPtr) fs = NULL;
size_t nfs = 0;
unsigned long long default_hugepagesz = 0;
@@ -3634,7 +3570,6 @@ virFileFindHugeTLBFS(virHugeTLBFSPtr *ret_fs,
endmntent(f);
while (nfs)
VIR_FREE(fs[--nfs].mnt_dir);
- VIR_FREE(fs);
return ret;
}
@@ -3870,10 +3805,8 @@ virFileCopyACLs(const char *src,
int
virFileComparePaths(const char *p1, const char *p2)
{
- int ret = -1;
- char *res1, *res2;
-
- res1 = res2 = NULL;
+ VIR_AUTOFREE(char *) res1 = NULL;
+ VIR_AUTOFREE(char *) res2 = NULL;
/* Assume p1 and p2 are symlinks, so try to resolve and canonicalize them.
* Canonicalization fails for example on file systems names like 'proc' or
@@ -3882,19 +3815,13 @@ virFileComparePaths(const char *p1, const char *p2)
*/
ignore_value(virFileResolveLink(p1, &res1));
if (!res1 && VIR_STRDUP(res1, p1) < 0)
- goto cleanup;
+ return -1;
ignore_value(virFileResolveLink(p2, &res2));
if (!res2 && VIR_STRDUP(res2, p2) < 0)
- goto cleanup;
-
- ret = STREQ_NULLABLE(res1, res2);
-
- cleanup:
- VIR_FREE(res1);
- VIR_FREE(res2);
+ return -1;
- return ret;
+ return STREQ_NULLABLE(res1, res2);
}
@@ -4038,25 +3965,22 @@ virFileInData(int fd ATTRIBUTE_UNUSED,
int
virFileReadValueInt(int *value, const char *format, ...)
{
- int ret = -1;
- char *str = NULL;
- char *path = NULL;
+ VIR_AUTOFREE(char *) str = NULL;
+ VIR_AUTOFREE(char *) path = NULL;
va_list ap;
va_start(ap, format);
if (virVasprintf(&path, format, ap) < 0) {
va_end(ap);
- goto cleanup;
+ return -1;
}
va_end(ap);
- if (!virFileExists(path)) {
- ret = -2;
- goto cleanup;
- }
+ if (!virFileExists(path))
+ return -2;
if (virFileReadAll(path, INT_BUFSIZE_BOUND(*value), &str) < 0)
- goto cleanup;
+ return -1;
virStringTrimOptionalNewline(str);
@@ -4064,14 +3988,10 @@ virFileReadValueInt(int *value, const char *format, ...)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid integer value '%s' in file
'%s'"),
str, path);
- goto cleanup;
+ return -1;
}
- ret = 0;
- cleanup:
- VIR_FREE(path);
- VIR_FREE(str);
- return ret;
+ return 0;
}
@@ -4088,25 +4008,22 @@ virFileReadValueInt(int *value, const char *format, ...)
int
virFileReadValueUint(unsigned int *value, const char *format, ...)
{
- int ret = -1;
- char *str = NULL;
- char *path = NULL;
+ VIR_AUTOFREE(char *) str = NULL;
+ VIR_AUTOFREE(char *) path = NULL;
va_list ap;
va_start(ap, format);
if (virVasprintf(&path, format, ap) < 0) {
va_end(ap);
- goto cleanup;
+ return -1;
}
va_end(ap);
- if (!virFileExists(path)) {
- ret = -2;
- goto cleanup;
- }
+ if (!virFileExists(path))
+ return -2;
if (virFileReadAll(path, INT_BUFSIZE_BOUND(*value), &str) < 0)
- goto cleanup;
+ return -1;
virStringTrimOptionalNewline(str);
@@ -4114,14 +4031,10 @@ virFileReadValueUint(unsigned int *value, const char *format,
...)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid unsigned integer value '%s' in file
'%s'"),
str, path);
- goto cleanup;
+ return -1;
}
- ret = 0;
- cleanup:
- VIR_FREE(path);
- VIR_FREE(str);
- return ret;
+ return 0;
}
@@ -4138,26 +4051,23 @@ virFileReadValueUint(unsigned int *value, const char *format,
...)
int
virFileReadValueScaledInt(unsigned long long *value, const char *format, ...)
{
- int ret = -1;
- char *str = NULL;
+ VIR_AUTOFREE(char *) str = NULL;
+ VIR_AUTOFREE(char *) path = NULL;
char *endp = NULL;
- char *path = NULL;
va_list ap;
va_start(ap, format);
if (virVasprintf(&path, format, ap) < 0) {
va_end(ap);
- goto cleanup;
+ return -1;
}
va_end(ap);
- if (!virFileExists(path)) {
- ret = -2;
- goto cleanup;
- }
+ if (!virFileExists(path))
+ return -2;
if (virFileReadAll(path, INT_BUFSIZE_BOUND(*value), &str) < 0)
- goto cleanup;
+ return -1;
virStringTrimOptionalNewline(str);
@@ -4165,14 +4075,10 @@ virFileReadValueScaledInt(unsigned long long *value, const char
*format, ...)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid unsigned scaled integer value '%s' in file
'%s'"),
str, path);
- goto cleanup;
+ return -1;
}
- ret = virScaleInteger(value, endp, 1024, ULLONG_MAX);
- cleanup:
- VIR_FREE(path);
- VIR_FREE(str);
- return ret;
+ return virScaleInteger(value, endp, 1024, ULLONG_MAX);
}
/* Arbitrarily sized number, feel free to change, but the function should be
@@ -4192,37 +4098,30 @@ virFileReadValueScaledInt(unsigned long long *value, const char
*format, ...)
int
virFileReadValueBitmap(virBitmapPtr *value, const char *format, ...)
{
- int ret = -1;
- char *str = NULL;
- char *path = NULL;
+ VIR_AUTOFREE(char *) str = NULL;
+ VIR_AUTOFREE(char *) path = NULL;
va_list ap;
va_start(ap, format);
if (virVasprintf(&path, format, ap) < 0) {
va_end(ap);
- goto cleanup;
+ return -1;
}
va_end(ap);
- if (!virFileExists(path)) {
- ret = -2;
- goto cleanup;
- }
+ if (!virFileExists(path))
+ return -2;
if (virFileReadAll(path, VIR_FILE_READ_VALUE_STRING_MAX, &str) < 0)
- goto cleanup;
+ return -1;
virStringTrimOptionalNewline(str);
*value = virBitmapParseUnlimited(str);
if (!*value)
- goto cleanup;
+ return -1;
- ret = 0;
- cleanup:
- VIR_FREE(path);
- VIR_FREE(str);
- return ret;
+ return 0;
}
/**
@@ -4240,30 +4139,26 @@ virFileReadValueBitmap(virBitmapPtr *value, const char *format,
...)
int
virFileReadValueString(char **value, const char *format, ...)
{
- int ret = -1;
- char *str = NULL;
- char *path = NULL;
+ int ret;
+ VIR_AUTOFREE(char *) str = NULL;
+ VIR_AUTOFREE(char *) path = NULL;
va_list ap;
va_start(ap, format);
if (virVasprintf(&path, format, ap) < 0) {
va_end(ap);
- goto cleanup;
+ return -1;
}
va_end(ap);
- if (!virFileExists(path)) {
- ret = -2;
- goto cleanup;
- }
+ if (!virFileExists(path))
+ return -2;
ret = virFileReadAll(path, VIR_FILE_READ_VALUE_STRING_MAX, value);
if (*value)
virStringTrimOptionalNewline(*value);
- cleanup:
- VIR_FREE(path);
- VIR_FREE(str);
+
return ret;
}
--
1.8.3.1
10:07 p.m.
New subject: [libvirt] [PATCH v1 14/18] use VIR_AUTOFREE in src/util/virfile.c
On Sun, Jun 03, 2018 at 01:42:12PM +0530, Sukrit Bhatnagar wrote:
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virfile.c | 303 +++++++++++++++++------------------------------------
1 file changed, 99 insertions(+), 204 deletions(-)
...
@@ -3546,18 +3485,18 @@ virFileGetHugepageSize(const char *path,
static int
virFileGetDefaultHugepageSize(unsigned long long *size)
{
- int ret = -1;
- char *meminfo, *c, *n, *unit;
+ VIR_AUTOFREE(char *) meminfo = NULL;
+ char *c, *n, *unit;
one variable per line...
if (virFileReadAll(PROC_MEMINFO, 4096, &meminfo) < 0)
- goto cleanup;
+ return -1;
if (!(c = strstr(meminfo, HUGEPAGESIZE_STR))) {
virReportError(VIR_ERR_NO_SUPPORT,
_("%s not found in %s"),
HUGEPAGESIZE_STR,
PROC_MEMINFO);
- goto cleanup;
+ return -1;
}
c += strlen(HUGEPAGESIZE_STR);
@@ -3570,13 +3509,10 @@ virFileGetDefaultHugepageSize(unsigned long long *size)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to parse %s %s"),
HUGEPAGESIZE_STR, c);
- goto cleanup;
+ return -1;
}
- ret = 0;
- cleanup:
- VIR_FREE(meminfo);
- return ret;
+ return 0;
}
# define PROC_MOUNTS "/proc/mounts"
@@ -3589,7 +3525,7 @@ virFileFindHugeTLBFS(virHugeTLBFSPtr *ret_fs,
FILE *f = NULL;
struct mntent mb;
char mntbuf[1024];
- virHugeTLBFSPtr fs = NULL;
+ VIR_AUTOFREE(virHugeTLBFSPtr) fs = NULL;
compound type (internal), needs a dedicated free helper...
size_t nfs = 0;
unsigned long long default_hugepagesz = 0;
@@ -3634,7 +3570,6 @@ virFileFindHugeTLBFS(virHugeTLBFSPtr *ret_fs,
endmntent(f);
while (nfs)
VIR_FREE(fs[--nfs].mnt_dir);
...especially because of ^this...
- VIR_FREE(fs);
return ret;
}
Erik
4:12 p.m.
New subject: [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 <skrtbhtngr(a)gmail.com>
---
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
4:12 p.m.
New subject: [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 <skrtbhtngr(a)gmail.com>
---
src/util/virfilecache.c | 35 +++++++++++------------------------
1 file changed, 11 insertions(+), 24 deletions(-)
diff --git a/src/util/virfilecache.c b/src/util/virfilecache.c
index 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
4:12 p.m.
New subject: [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 <skrtbhtngr(a)gmail.com>
---
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
11:01 p.m.
New subject: [libvirt] [PATCH v1 17/18] use VIR_AUTOFREE in src/util/virfirewall.c
On Sun, Jun 03, 2018 at 01:42:15PM +0530, Sukrit Bhatnagar wrote:
Modify code to use VIR_AUTOFREE macro wherever required.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
src/util/virfirewall.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
...
@@ -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);
let's couple ^this VIR_AUTOFREE with the one above...
Erik
4:12 p.m.
New subject: [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 <skrtbhtngr(a)gmail.com>
---
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
2339
days inactive
2342
days old
29 comments
2 participants
participants (2)
-
Erik Skultety -
Sukrit Bhatnagar