Re: [libvirt] [PATCH v4 07/13] Adapt to VIR_STRDUP and VIR_STRNDUP in src/rpc/*
On 05/20/2013 01:55 PM, Michal Privoznik wrote:
---
src/rpc/gendispatch.pl | 21 ++++--------
src/rpc/virnetclient.c | 16 ++++-----
src/rpc/virnetmessage.c | 27 +++++++++------
src/rpc/virnetsaslcontext.c | 6 ++--
src/rpc/virnetserver.c | 6 ++--
src/rpc/virnetserverclient.c | 10 ++----
src/rpc/virnetservermdns.c | 6 ++--
src/rpc/virnetsocket.c | 10 +++---
src/rpc/virnetsshsession.c | 78 +++++++++++++++++++++-----------------------
src/rpc/virnettlscontext.c | 26 +++++++--------
10 files changed, 92 insertions(+), 114 deletions(-)
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
[...snip...]
diff --git a/src/rpc/virnetmessage.c b/src/rpc/virnetmessage.c
index b2c6e5b..8483cd5 100644
--- a/src/rpc/virnetmessage.c
+++ b/src/rpc/virnetmessage.c
@@ -29,6 +29,7 @@
#include "virlog.h"
#include "virfile.h"
#include "virutil.h"
+#include "virstring.h"
#define VIR_FROM_THIS VIR_FROM_RPC
@@ -514,22 +515,28 @@ void virNetMessageSaveError(virNetMessageErrorPtr rerr)
if (verr) {
rerr->code = verr->code;
rerr->domain = verr->domain;
- if (verr->message && VIR_ALLOC(rerr->message) == 0)
- *rerr->message = strdup(verr->message);
+ if (verr->message && VIR_ALLOC(rerr->message) == 0 &&
+ VIR_STRDUP_QUIET(*rerr->message, verr->message) < 0)
+ VIR_FREE(rerr->message);
rerr->level = verr->level;
- if (verr->str1 && VIR_ALLOC(rerr->str1) == 0)
- *rerr->str1 = strdup(verr->str1);
- if (verr->str2 && VIR_ALLOC(rerr->str2) == 0)
- *rerr->str2 = strdup(verr->str2);
- if (verr->str3 && VIR_ALLOC(rerr->str3) == 0)
- *rerr->str3 = strdup(verr->str3);
+ if (verr->str1 && VIR_ALLOC(rerr->str1) == 0 &&
+ VIR_STRDUP_QUIET(*rerr->str1, verr->str1) < 0)
+ VIR_FREE(verr->str1);
+ if (verr->str2 && VIR_ALLOC(rerr->str2) == 0 &&
+ VIR_STRDUP_QUIET(*rerr->str2, verr->str2) < 0)
+ VIR_FREE(verr->str2);
+ if (verr->str3 && VIR_ALLOC(rerr->str3) == 0 &&
+ VIR_STRDUP_QUIET(*rerr->str3, verr->str3) < 0)
+ VIR_FREE(verr->str2);
Coverity has a complaint in here:
525 if (verr->str2 && VIR_ALLOC(rerr->str2) == 0 &&
526 VIR_STRDUP_QUIET(*rerr->str2, verr->str2) < 0)
(1) Event original: "verr->str2" looks like the original copy.
Also see events: [copy_paste_error]
527 VIR_FREE(verr->str2);
528 if (verr->str3 && VIR_ALLOC(rerr->str3) == 0 &&
529 VIR_STRDUP_QUIET(*rerr->str3, verr->str3) < 0)
(2) Event copy_paste_error: "str2" in "verr->str2" looks like a
copy-paste error. Should it say "str3" instead?
Also see events: [original]
530 VIR_FREE(verr->str2);
531 rerr->int1 = verr->int1;
532 rerr->int2 = verr->int2;
The complaint is only on the last VIR_FREE(verr->str2); as a cut-n-paste
of the one on line 527; however, it seems to me all those VIR_FREE()'s
should be on "rerr" and not "verr", right?
That is, there's a VIR_ALLOC(rerr->str#) for each section, then on
failure shouldn't it be VIR_FREE(rerr->str#)?
John
rerr->int1 = verr->int1;
rerr->int2 = verr->int2;
} else {
rerr->code = VIR_ERR_INTERNAL_ERROR;
rerr->domain = VIR_FROM_RPC;
- if (VIR_ALLOC(rerr->message) == 0)
- *rerr->message = strdup(_("Library function returned error but did
not set virError"));
+ if (VIR_ALLOC(rerr->message) == 0 &&
+ VIR_STRDUP_QUIET(*rerr->message,
+ _("Library function returned error but did not set
virError")) < 0)
+ VIR_FREE(rerr->message);
rerr->level = VIR_ERR_ERROR;
}
}
[...snip...]