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...]