On Tue, Jun 13, 2017 at 12:35:10PM -0400, John Ferlan wrote:
On 05/29/2017 10:31 AM, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
Notes: new in v2
src/conf/domain_conf.c | 46 +++++++++++++++++++---------------------- src/conf/domain_conf.h | 9 ++++---- src/security/security_dac.c | 26 ++++++++++------------- src/security/security_manager.c | 4 ++-- src/security/security_selinux.c | 24 +++++++++------------ 5 files changed, 49 insertions(+), 60 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c7e20b8ba1..68dc2832cb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2076,12 +2076,21 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
void virDomainChrSourceDefFree(virDomainChrSourceDefPtr def) { + size_t i; + if (!def) return;
virDomainChrSourceDefClear(def); virObjectUnref(def->privateData);
+ if (def->seclabels) { + for (i = 0; i < def->nseclabels; i++) + virSecurityDeviceLabelDefFree(def->seclabels[i]); + VIR_FREE(def->seclabels); + } + + VIR_FREE(def); }
@@ -2150,8 +2159,6 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
void virDomainChrDefFree(virDomainChrDefPtr def) { - size_t i; - if (!def) return;
@@ -2176,12 +2183,6 @@ void virDomainChrDefFree(virDomainChrDefPtr def) virDomainChrSourceDefFree(def->source); virDomainDeviceInfoClear(&def->info);
- if (def->seclabels) { - for (i = 0; i < def->nseclabels; i++) - virSecurityDeviceLabelDefFree(def->seclabels[i]); - VIR_FREE(def->seclabels); - } - VIR_FREE(def); }
@@ -10688,8 +10689,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, if (chr_def) {
Is the @chr_def check necessary still? I assume it's there because chr_def can be passed as NULL in some cases.
Looks like all this was added as part of commit 'f8b08d0e'
The chr_def would be NULL for Smartcard, RNG, and Redirdev callers which each now doesn't pass a NULL to virDomainChrSourceDefFormat, so that leads me to believe that the @chr_def should be removed.
But this hunk is from virDomainChrSourceDefParseXML() function.
The rest looks good, so
Reviewed-by: John Ferlan <jferlan@redhat.com>
Thanks Pavel