6:16 p.m.
Several functions was modified to become invariant. Change
their type to void and remove unnecessary checks of their
return values.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Artem Chernyshev (7):
cpu: : virCPUx86DataAddItem() to void
rpc: virnetserver: virNetServerAddService() to void
lxc: virLXCControllerAddConsole() to void
conf: virDomainNetUpdate() to void
virprocess: virProcessGetNamespaces() to void
node_device: udevTranslatePCIIds() to void
node_device: udevGetStringSysfsAttr() to void
src/conf/domain_conf.c | 3 +-
src/conf/domain_conf.h | 2 +-
src/cpu/cpu_x86.c | 179 +++++++++++------------------
src/cpu/cpu_x86.h | 4 +-
src/libvirt-lxc.c | 5 +-
src/libxl/libxl_capabilities.c | 13 +--
src/lxc/lxc_controller.c | 9 +-
src/lxc/lxc_driver.c | 6 +-
src/node_device/node_device_udev.c | 69 ++++-------
src/qemu/qemu_capabilities.c | 3 +-
src/qemu/qemu_driver.c | 3 +-
src/qemu/qemu_monitor_json.c | 7 +-
src/qemu/qemu_process.c | 3 +-
src/rpc/virnetserver.c | 25 +---
src/rpc/virnetserver.h | 2 +-
src/util/virprocess.c | 4 +-
src/util/virprocess.h | 2 +-
tests/virnetdaemontest.c | 6 +-
18 files changed, 118 insertions(+), 227 deletions(-)
--
2.43.0
6:16 p.m.
New subject: [PATCH 1/7] cpu: : virCPUx86DataAddItem() to void
virCPUx86DataAddItem() return value
is invariant, so change it type and remove all dependent
checks.
Functions changed to void:
virCPUx86DataAddItem()
x86DataAdd()
virCPUx86DataAdd()
x86DataAddSignature()
virCPUx86DataSetSignature()
libxlCapsAddCPUID()
cpuidSetLeaf4()
cpuidSetLeaf7()
cpuidSetLeafB()
cpuidSetLeafD()
cpuidSetLeafResID()
cpuidSetLeaf12()
cpuidSetLeaf14()
cpuidSetLeaf17()
In virCPUx86GetHost() still exists this statement
```
if (cpuidSet(CPUX86_BASIC, cpuData) < 0 ||
cpuidSet(CPUX86_EXTENDED, cpuData) < 0)
return -1;
```
Probably should change it as well.
Fixes: 592517636f ("util: alloc: Reimplement VIR_APPEND_ELEMENT_COPY using
virAppendElement")
Signed-off-by: Artem Chernyshev <artem.chernyshev(a)red-soft.ru>
---
src/cpu/cpu_x86.c | 179 +++++++++++++--------------------
src/cpu/cpu_x86.h | 4 +-
src/libxl/libxl_capabilities.c | 13 +--
src/qemu/qemu_capabilities.c | 3 +-
src/qemu/qemu_monitor_json.c | 7 +-
5 files changed, 77 insertions(+), 129 deletions(-)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 8d0e3947ce..6b2531b360 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -531,7 +531,7 @@ x86DataCopy(virCPUx86Data *dst, const virCPUx86Data *src)
}
-static int
+static void
virCPUx86DataAddItem(virCPUx86Data *data,
const virCPUx86DataItem *item)
{
@@ -547,12 +547,10 @@ virCPUx86DataAddItem(virCPUx86Data *data,
sizeof(virCPUx86DataItem),
virCPUx86DataSorter, NULL);
}
-
- return 0;
}
-static int
+static void
x86DataAdd(virCPUx86Data *data1,
const virCPUx86Data *data2)
{
@@ -560,12 +558,8 @@ x86DataAdd(virCPUx86Data *data1,
virCPUx86DataItem *item;
virCPUx86DataIteratorInit(&iter, data2);
- while ((item = virCPUx86DataNext(&iter))) {
- if (virCPUx86DataAddItem(data1, item) < 0)
- return -1;
- }
-
- return 0;
+ while ((item = virCPUx86DataNext(&iter)))
+ virCPUx86DataAddItem(data1, item);
}
@@ -814,13 +808,13 @@ x86DataToSignature(const virCPUx86Data *data)
}
-static int
+static void
x86DataAddSignature(virCPUx86Data *data,
uint32_t signature)
{
virCPUx86DataItem leaf1 = CPUID(.eax_in = 0x1, .eax = signature);
- return virCPUx86DataAddItem(data, &leaf1);
+ virCPUx86DataAddItem(data, &leaf1);
}
@@ -885,8 +879,7 @@ x86DataToCPU(const virCPUx86Data *data,
for (blocker = hvModel->blockers; *blocker; blocker++) {
if ((feature = x86FeatureFind(map, *blocker)) &&
!x86DataIsSubset(©, &feature->data))
- if (x86DataAdd(&modelData, &feature->data) < 0)
- return NULL;
+ x86DataAdd(&modelData, &feature->data);
}
}
@@ -1125,8 +1118,7 @@ x86ParseDataItemList(virCPUx86Data *cpudata,
}
}
- if (virCPUx86DataAddItem(cpudata, &item) < 0)
- return -1;
+ virCPUx86DataAddItem(cpudata, &item);
++i;
node = xmlNextElementSibling(node);
@@ -1381,8 +1373,7 @@ x86ModelFromCPU(const virCPUDef *cpu,
switch (fpol) {
case VIR_CPU_FEATURE_FORCE:
case VIR_CPU_FEATURE_REQUIRE:
- if (x86DataAdd(&model->data, &feature->data) < 0)
- return NULL;
+ x86DataAdd(&model->data, &feature->data);
break;
case VIR_CPU_FEATURE_DISABLE:
@@ -1394,8 +1385,8 @@ x86ModelFromCPU(const virCPUDef *cpu,
case VIR_CPU_FEATURE_LAST:
break;
}
- } else if (x86DataAdd(&model->data, &feature->data) < 0) {
- return NULL;
+ } else {
+ x86DataAdd(&model->data, &feature->data);
}
}
@@ -1641,8 +1632,7 @@ x86ModelParseFeatures(virCPUx86Model *model,
continue;
}
- if (x86DataAdd(&model->data, &feature->data))
- return -1;
+ x86DataAdd(&model->data, &feature->data);
}
model->removedFeatures = g_renew(char *, model->removedFeatures, nremoved +
1);
@@ -2384,8 +2374,8 @@ x86Encode(virArch arch,
if (!(data_vendor = virCPUDataNew(arch)))
return -1;
- if (v && virCPUx86DataAdd(data_vendor, &v->data) < 0)
- return -1;
+ if (v)
+ virCPUx86DataAdd(data_vendor, &v->data);
}
if (forced)
@@ -2452,23 +2442,20 @@ cpuidCall(virCPUx86CPUID *cpuid)
*
* Sub leaf n+1 is invalid if eax[4:0] in sub leaf n equals 0.
*/
-static int
+static void
cpuidSetLeaf4(virCPUData *data,
virCPUx86DataItem *subLeaf0)
{
virCPUx86DataItem item = *subLeaf0;
virCPUx86CPUID *cpuid = &item.data.cpuid;
- if (virCPUx86DataAdd(data, subLeaf0) < 0)
- return -1;
+ virCPUx86DataAdd(data, subLeaf0);
while (cpuid->eax & 0x1f) {
cpuid->ecx_in++;
cpuidCall(cpuid);
- if (virCPUx86DataAdd(data, &item) < 0)
- return -1;
+ virCPUx86DataAdd(data, &item);
}
- return 0;
}
@@ -2476,7 +2463,7 @@ cpuidSetLeaf4(virCPUData *data,
*
* Sub leaf n is invalid if n > eax in sub leaf 0.
*/
-static int
+static void
cpuidSetLeaf7(virCPUData *data,
virCPUx86DataItem *subLeaf0)
{
@@ -2484,16 +2471,13 @@ cpuidSetLeaf7(virCPUData *data,
virCPUx86CPUID *cpuid = &item.data.cpuid;
uint32_t sub;
- if (virCPUx86DataAdd(data, subLeaf0) < 0)
- return -1;
+ virCPUx86DataAdd(data, subLeaf0);
for (sub = 1; sub <= subLeaf0->data.cpuid.eax; sub++) {
cpuid->ecx_in = sub;
cpuidCall(cpuid);
- if (virCPUx86DataAdd(data, &item) < 0)
- return -1;
+ virCPUx86DataAdd(data, &item);
}
- return 0;
}
@@ -2504,7 +2488,7 @@ cpuidSetLeaf7(virCPUData *data,
* Some output values do not depend on ecx, thus sub leaf 0 provides
* meaningful data even if it was (theoretically) considered invalid.
*/
-static int
+static void
cpuidSetLeafB(virCPUData *data,
virCPUx86DataItem *subLeaf0)
{
@@ -2512,12 +2496,10 @@ cpuidSetLeafB(virCPUData *data,
virCPUx86CPUID *cpuid = &item.data.cpuid;
while (cpuid->ecx & 0xff00) {
- if (virCPUx86DataAdd(data, &item) < 0)
- return -1;
+ virCPUx86DataAdd(data, &item);
cpuid->ecx_in++;
cpuidCall(cpuid);
}
- return 0;
}
@@ -2529,7 +2511,7 @@ cpuidSetLeafB(virCPUData *data,
* Sub leaf n (32 <= n < 64) is invalid if edx[n-32] from sub leaf 0 is not set
* and edx[n-32] from sub leaf 1 is not set.
*/
-static int
+static void
cpuidSetLeafD(virCPUData *data,
virCPUx86DataItem *subLeaf0)
{
@@ -2539,13 +2521,11 @@ cpuidSetLeafD(virCPUData *data,
virCPUx86CPUID sub1;
uint32_t sub;
- if (virCPUx86DataAdd(data, subLeaf0) < 0)
- return -1;
+ virCPUx86DataAdd(data, subLeaf0);
cpuid->ecx_in = 1;
cpuidCall(cpuid);
- if (virCPUx86DataAdd(data, &item) < 0)
- return -1;
+ virCPUx86DataAdd(data, &item);
sub0 = subLeaf0->data.cpuid;
sub1 = *cpuid;
@@ -2561,10 +2541,8 @@ cpuidSetLeafD(virCPUData *data,
cpuid->ecx_in = sub;
cpuidCall(cpuid);
- if (virCPUx86DataAdd(data, &item) < 0)
- return -1;
+ virCPUx86DataAdd(data, &item);
}
- return 0;
}
@@ -2577,7 +2555,7 @@ cpuidSetLeafD(virCPUData *data,
* 0x0f: Sub leaf n is valid if edx[n] (= res[ResID]) from sub leaf 0 is set.
* 0x10: Sub leaf n is valid if ebx[n] (= res[ResID]) from sub leaf 0 is set.
*/
-static int
+static void
cpuidSetLeafResID(virCPUData *data,
virCPUx86DataItem *subLeaf0,
uint32_t res)
@@ -2586,18 +2564,15 @@ cpuidSetLeafResID(virCPUData *data,
virCPUx86CPUID *cpuid = &item.data.cpuid;
uint32_t sub;
- if (virCPUx86DataAdd(data, subLeaf0) < 0)
- return -1;
+ virCPUx86DataAdd(data, subLeaf0);
for (sub = 1; sub < 32; sub++) {
if (!(res & (1U << sub)))
continue;
cpuid->ecx_in = sub;
cpuidCall(cpuid);
- if (virCPUx86DataAdd(data, &item) < 0)
- return -1;
+ virCPUx86DataAdd(data, &item);
}
- return 0;
}
@@ -2606,7 +2581,7 @@ cpuidSetLeafResID(virCPUData *data,
* Sub leaves 0 and 1 is supported if ebx[2] from leaf 0x7 (SGX) is set.
* Sub leaves n >= 2 are valid as long as eax[3:0] != 0.
*/
-static int
+static void
cpuidSetLeaf12(virCPUData *data,
virCPUx86DataItem *subLeaf0)
{
@@ -2616,26 +2591,22 @@ cpuidSetLeaf12(virCPUData *data,
if (!(leaf7 = virCPUx86DataGet(&data->data.x86, &item)) ||
!(leaf7->data.cpuid.ebx & (1 << 2)))
- return 0;
+ return;
- if (virCPUx86DataAdd(data, subLeaf0) < 0)
- return -1;
+ virCPUx86DataAdd(data, subLeaf0);
cpuid->eax_in = 0x12;
cpuid->ecx_in = 1;
cpuidCall(cpuid);
- if (virCPUx86DataAdd(data, &item) < 0)
- return -1;
+ virCPUx86DataAdd(data, &item);
cpuid->ecx_in = 2;
cpuidCall(cpuid);
while (cpuid->eax & 0xf) {
- if (virCPUx86DataAdd(data, &item) < 0)
- return -1;
+ virCPUx86DataAdd(data, &item);
cpuid->ecx_in++;
cpuidCall(cpuid);
}
- return 0;
}
@@ -2643,7 +2614,7 @@ cpuidSetLeaf12(virCPUData *data,
*
* Sub leaf 0 reports the maximum supported sub leaf in eax.
*/
-static int
+static void
cpuidSetLeaf14(virCPUData *data,
virCPUx86DataItem *subLeaf0)
{
@@ -2651,16 +2622,13 @@ cpuidSetLeaf14(virCPUData *data,
virCPUx86CPUID *cpuid = &item.data.cpuid;
uint32_t sub;
- if (virCPUx86DataAdd(data, subLeaf0) < 0)
- return -1;
+ virCPUx86DataAdd(data, subLeaf0);
for (sub = 1; sub <= subLeaf0->data.cpuid.eax; sub++) {
cpuid->ecx_in = sub;
cpuidCall(cpuid);
- if (virCPUx86DataAdd(data, &item) < 0)
- return -1;
+ virCPUx86DataAdd(data, &item);
}
- return 0;
}
@@ -2669,7 +2637,7 @@ cpuidSetLeaf14(virCPUData *data,
* Sub leaf 0 is valid if eax >= 3.
* Sub leaf 0 reports the maximum supported sub leaf in eax.
*/
-static int
+static void
cpuidSetLeaf17(virCPUData *data,
virCPUx86DataItem *subLeaf0)
{
@@ -2678,25 +2646,21 @@ cpuidSetLeaf17(virCPUData *data,
uint32_t sub;
if (subLeaf0->data.cpuid.eax < 3)
- return 0;
+ return;
- if (virCPUx86DataAdd(data, subLeaf0) < 0)
- return -1;
+ virCPUx86DataAdd(data, subLeaf0);
for (sub = 1; sub <= subLeaf0->data.cpuid.eax; sub++) {
cpuid->ecx_in = sub;
cpuidCall(cpuid);
- if (virCPUx86DataAdd(data, &item) < 0)
- return -1;
+ virCPUx86DataAdd(data, &item);
}
- return 0;
}
static int
cpuidSet(uint32_t base, virCPUData *data)
{
- int rc;
uint32_t max;
uint32_t leaf;
virCPUx86DataItem item = CPUID(.eax_in = base);
@@ -2714,28 +2678,25 @@ cpuidSet(uint32_t base, virCPUData *data)
* which provide additional sub leaves for ecx_in > 0
*/
if (leaf == 0x4)
- rc = cpuidSetLeaf4(data, &item);
+ cpuidSetLeaf4(data, &item);
else if (leaf == 0x7)
- rc = cpuidSetLeaf7(data, &item);
+ cpuidSetLeaf7(data, &item);
else if (leaf == 0xb)
- rc = cpuidSetLeafB(data, &item);
+ cpuidSetLeafB(data, &item);
else if (leaf == 0xd)
- rc = cpuidSetLeafD(data, &item);
+ cpuidSetLeafD(data, &item);
else if (leaf == 0xf)
- rc = cpuidSetLeafResID(data, &item, cpuid->edx);
+ cpuidSetLeafResID(data, &item, cpuid->edx);
else if (leaf == 0x10)
- rc = cpuidSetLeafResID(data, &item, cpuid->ebx);
+ cpuidSetLeafResID(data, &item, cpuid->ebx);
else if (leaf == 0x12)
- rc = cpuidSetLeaf12(data, &item);
+ cpuidSetLeaf12(data, &item);
else if (leaf == 0x14)
- rc = cpuidSetLeaf14(data, &item);
+ cpuidSetLeaf14(data, &item);
else if (leaf == 0x17)
- rc = cpuidSetLeaf17(data, &item);
+ cpuidSetLeaf17(data, &item);
else
- rc = virCPUx86DataAdd(data, &item);
-
- if (rc < 0)
- return -1;
+ virCPUx86DataAdd(data, &item);
}
return 0;
@@ -2777,8 +2738,7 @@ virCPUx86GetHost(virCPUDef *cpu,
},
};
- if (virCPUx86DataAdd(cpuData, &item) < 0)
- return -1;
+ virCPUx86DataAdd(cpuData, &item);
}
}
@@ -2897,9 +2857,8 @@ virCPUx86Baseline(virCPUDef **cpus,
return NULL;
for (i = 0; features[i]; i++) {
- if ((feat = x86FeatureFind(map, features[i])) &&
- x86DataAdd(&featData->data.x86, &feat->data) < 0)
- return NULL;
+ if ((feat = x86FeatureFind(map, features[i])))
+ x86DataAdd(&featData->data.x86, &feat->data);
}
x86DataIntersect(&base_model->data, &featData->data.x86);
@@ -2911,9 +2870,8 @@ virCPUx86Baseline(virCPUDef **cpus,
return NULL;
}
- if (vendor &&
- virCPUx86DataAddItem(&base_model->data, &vendor->data) < 0)
- return NULL;
+ if (vendor)
+ virCPUx86DataAddItem(&base_model->data, &vendor->data);
if (x86Decode(cpu, &base_model->data, models,
(const char **) modelNames, migratable) < 0)
@@ -3172,15 +3130,13 @@ virCPUx86Translate(virCPUDef *cpu,
if (!(model = x86ModelFromCPU(cpu, map, -1)))
return -1;
- if (model->vendor &&
- virCPUx86DataAddItem(&model->data, &model->vendor->data) <
0)
- return -1;
+ if (model->vendor)
+ virCPUx86DataAddItem(&model->data, &model->vendor->data);
if (model->signatures && model->signatures->count > 0) {
virCPUx86Signature *sig = &model->signatures->items[0];
- if (x86DataAddSignature(&model->data,
- virCPUx86SignatureToCPUID(sig)) < 0)
- return -1;
+ x86DataAddSignature(&model->data,
+ virCPUx86SignatureToCPUID(sig));
}
if (x86Decode(translated, &model->data, models, NULL, false) < 0)
@@ -3298,15 +3254,15 @@ virCPUx86ValidateFeatures(virCPUDef *cpu)
}
-int
+void
virCPUx86DataAdd(virCPUData *cpuData,
const virCPUx86DataItem *item)
{
- return virCPUx86DataAddItem(&cpuData->data.x86, item);
+ virCPUx86DataAddItem(&cpuData->data.x86, item);
}
-int
+void
virCPUx86DataSetSignature(virCPUData *cpuData,
unsigned int family,
unsigned int model,
@@ -3314,7 +3270,7 @@ virCPUx86DataSetSignature(virCPUData *cpuData,
{
uint32_t signature = x86MakeSignature(family, model, stepping);
- return x86DataAddSignature(&cpuData->data.x86, signature);
+ x86DataAddSignature(&cpuData->data.x86, signature);
}
@@ -3339,7 +3295,9 @@ virCPUx86DataSetVendor(virCPUData *cpuData,
if (virCPUx86VendorToData(vendor, &item) < 0)
return -1;
- return virCPUx86DataAdd(cpuData, &item);
+ virCPUx86DataAdd(cpuData, &item);
+
+ return 0;
}
@@ -3358,8 +3316,7 @@ virCPUx86DataAddFeature(virCPUData *cpuData,
!(feature = x86FeatureFindInternal(name)))
return 0;
- if (x86DataAdd(&cpuData->data.x86, &feature->data) < 0)
- return -1;
+ x86DataAdd(&cpuData->data.x86, &feature->data);
return 0;
}
diff --git a/src/cpu/cpu_x86.h b/src/cpu/cpu_x86.h
index 416d052776..96fb8b7502 100644
--- a/src/cpu/cpu_x86.h
+++ b/src/cpu/cpu_x86.h
@@ -25,10 +25,10 @@
extern struct cpuArchDriver cpuDriverX86;
-int virCPUx86DataAdd(virCPUData *cpuData,
+void virCPUx86DataAdd(virCPUData *cpuData,
const virCPUx86DataItem *cpuid);
-int virCPUx86DataSetSignature(virCPUData *cpuData,
+void virCPUx86DataSetSignature(virCPUData *cpuData,
unsigned int family,
unsigned int model,
unsigned int stepping);
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index 177e8b988e..fc502f8ccf 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -55,7 +55,7 @@ struct guest_arch {
#define XEN_CAP_REGEX
"(xen|hvm)-[[:digit:]]+\\.[[:digit:]]+-(aarch64|armv7l|x86_32|x86_64|ia64|powerpc64)(p|be)?"
-static int
+static void
libxlCapsAddCPUID(virCPUData *data, virCPUx86CPUID *cpuid, ssize_t ncaps)
{
virCPUx86DataItem item = { 0 };
@@ -65,14 +65,8 @@ libxlCapsAddCPUID(virCPUData *data, virCPUx86CPUID *cpuid, ssize_t
ncaps)
for (i = 0; i < ncaps; i++) {
item.data.cpuid = cpuid[i];
- if (virCPUx86DataAdd(data, &item) < 0) {
- VIR_DEBUG("Failed to add CPUID(%x,%x)",
- cpuid[i].eax_in, cpuid[i].ecx_in);
- return -1;
- }
+ virCPUx86DataAdd(data, &item);
}
-
- return 0;
}
/*
@@ -119,8 +113,7 @@ libxlCapsNodeData(virCPUDef *cpu, libxl_hwcap hwcap)
return NULL;
ncaps = G_N_ELEMENTS(cpuid);
- if (libxlCapsAddCPUID(cpudata, cpuid, ncaps) < 0)
- return NULL;
+ libxlCapsAddCPUID(cpudata, cpuid, ncaps);
return g_steal_pointer(&cpudata);
}
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 83119e871a..eb6b345906 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3679,8 +3679,7 @@ virQEMUCapsGetCPUModelX86Data(virQEMUCaps *qemuCaps,
}
}
- if (virCPUx86DataSetSignature(data, sigFamily, sigModel, sigStepping) < 0)
- return NULL;
+ virCPUx86DataSetSignature(data, sigFamily, sigModel, sigStepping);
return g_steal_pointer(&data);
}
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 1574723624..622e977298 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6701,10 +6701,9 @@ qemuMonitorJSONParseCPUx86Features(virJSONValue *data)
item.type = VIR_CPU_X86_DATA_CPUID;
for (i = 0; i < virJSONValueArraySize(data); i++) {
- if (qemuMonitorJSONParseCPUx86FeatureWord(virJSONValueArrayGet(data, i),
- &item.data.cpuid) < 0 ||
- virCPUx86DataAdd(cpudata, &item) < 0)
- return NULL;
+ if (!qemuMonitorJSONParseCPUx86FeatureWord(virJSONValueArrayGet(data, i),
+ &item.data.cpuid))
+ virCPUx86DataAdd(cpudata, &item);
}
return g_steal_pointer(&cpudata);
--
2.43.0
10:08 a.m.
New subject: [PATCH 1/7] cpu: : virCPUx86DataAddItem() to void
On 1/4/24 01:16, Artem Chernyshev wrote:
virCPUx86DataAddItem() return value
is invariant, so change it type and remove all dependent
checks.
Functions changed to void:
virCPUx86DataAddItem()
x86DataAdd()
virCPUx86DataAdd()
x86DataAddSignature()
virCPUx86DataSetSignature()
libxlCapsAddCPUID()
cpuidSetLeaf4()
cpuidSetLeaf7()
cpuidSetLeafB()
cpuidSetLeafD()
cpuidSetLeafResID()
cpuidSetLeaf12()
cpuidSetLeaf14()
cpuidSetLeaf17()
In virCPUx86GetHost() still exists this statement
```
if (cpuidSet(CPUX86_BASIC, cpuData) < 0 ||
cpuidSet(CPUX86_EXTENDED, cpuData) < 0)
return -1;
```
Probably should change it as well.
Fixes: 592517636f ("util: alloc: Reimplement VIR_APPEND_ELEMENT_COPY using
virAppendElement")
It doesn't really fix anything. The code is not broken.
Signed-off-by: Artem Chernyshev <artem.chernyshev(a)red-soft.ru>
---
src/cpu/cpu_x86.c | 179 +++++++++++++--------------------
src/cpu/cpu_x86.h | 4 +-
src/libxl/libxl_capabilities.c | 13 +--
src/qemu/qemu_capabilities.c | 3 +-
src/qemu/qemu_monitor_json.c | 7 +-
5 files changed, 77 insertions(+), 129 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c
b/src/qemu/qemu_monitor_json.c
index 1574723624..622e977298 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6701,10 +6701,9 @@ qemuMonitorJSONParseCPUx86Features(virJSONValue *data)
item.type = VIR_CPU_X86_DATA_CPUID;
for (i = 0; i < virJSONValueArraySize(data); i++) {
- if (qemuMonitorJSONParseCPUx86FeatureWord(virJSONValueArrayGet(data, i),
- &item.data.cpuid) < 0 ||
- virCPUx86DataAdd(cpudata, &item) < 0)
- return NULL;
+ if (!qemuMonitorJSONParseCPUx86FeatureWord(virJSONValueArrayGet(data, i),
+ &item.data.cpuid))
+ virCPUx86DataAdd(cpudata, &item);
No. This changes semantics. Prior to your change, if
qemuMonitorJSONParseCPUx86FeatureWord() failed then 'return NULL' path
would be taken. After your change the failure is ignored and ...
}
return g_steal_pointer(&cpudata);
... this return path is taken.
Michal
2:59 a.m.
New subject: [PATCH 1/7] cpu: : virCPUx86DataAddItem() to void
On 1/4/24 01:16, Artem Chernyshev wrote:
It doesn't really fix anything. The code is not broken.
True, just mentioned commit which I addressed in my patch
No. This changes semantics. Prior to your change, if
qemuMonitorJSONParseCPUx86FeatureWord() failed then 'return NULL' path
would be taken. After your change the failure is ignored and ...
... this return path is taken.
Michal
Yes, my mistake. Should I send v2 or you already fix that?
Thank you,
Artem
6:16 p.m.
New subject: [PATCH 2/7] rpc: virnetserver: virNetServerAddService() to void
virNetServerAddService() return value
is invariant, so change it type and remove all dependent
checks.
Fixes: 7d2fd6ef01 ("Do not check return value of VIR_EXPAND_N")
Signed-off-by: Artem Chernyshev <artem.chernyshev(a)red-soft.ru>
---
src/lxc/lxc_controller.c | 3 +--
src/rpc/virnetserver.c | 25 +++++--------------------
src/rpc/virnetserver.h | 2 +-
tests/virnetdaemontest.c | 6 ++----
4 files changed, 9 insertions(+), 27 deletions(-)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 48f5c73fce..0a92a24fb7 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -947,8 +947,7 @@ static int virLXCControllerSetupServer(virLXCController *ctrl)
if (virSecurityManagerClearSocketLabel(ctrl->securityManager, ctrl->def) <
0)
goto error;
- if (virNetServerAddService(srv, svc) < 0)
- goto error;
+ virNetServerAddService(srv, svc);
g_clear_pointer(&svc, virObjectUnref);
if (!(ctrl->prog = virNetServerProgramNew(VIR_LXC_MONITOR_PROGRAM,
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 770476c1a6..a6c6443c55 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -513,10 +513,7 @@ virNetServerNewPostExecRestart(virJSONValue *object,
if (!(service = virNetServerServiceNewPostExecRestart(child)))
return NULL;
- if (virNetServerAddService(srv, service) < 0) {
- virObjectUnref(service);
- return NULL;
- }
+ virNetServerAddService(srv, service);
}
@@ -623,7 +620,7 @@ virNetServerPreExecRestart(virNetServer *srv)
}
-int
+void
virNetServerAddService(virNetServer *srv,
virNetServerService *svc)
{
@@ -633,7 +630,6 @@ virNetServerAddService(virNetServer *srv,
srv->services[srv->nservices-1] = virObjectRef(svc);
virNetServerServiceSetDispatcher(svc, virNetServerDispatchNewClient, srv);
- return 0;
}
@@ -669,10 +665,7 @@ virNetServerAddServiceActivation(virNetServer *srv,
if (!svc)
return -1;
- if (virNetServerAddService(srv, svc) < 0) {
- virObjectUnref(svc);
- return -1;
- }
+ virNetServerAddService(srv, svc);
}
/* Intentionally return 1 any time activation is present,
@@ -723,11 +716,7 @@ virNetServerAddServiceTCP(virNetServer *srv,
nrequests_client_max)))
return -1;
- if (virNetServerAddService(srv, svc) < 0) {
- virObjectUnref(svc);
- return -1;
- }
-
+ virNetServerAddService(srv, svc);
virObjectUnref(svc);
return 0;
@@ -772,11 +761,7 @@ virNetServerAddServiceUNIX(virNetServer *srv,
nrequests_client_max)))
return -1;
- if (virNetServerAddService(srv, svc) < 0) {
- virObjectUnref(svc);
- return -1;
- }
-
+ virNetServerAddService(srv, svc);
virObjectUnref(svc);
return 0;
diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h
index 9f0cf3a3fc..dc60e2c2b2 100644
--- a/src/rpc/virnetserver.h
+++ b/src/rpc/virnetserver.h
@@ -59,7 +59,7 @@ void virNetServerShutdownWait(virNetServer *srv);
virJSONValue *virNetServerPreExecRestart(virNetServer *srv);
-int virNetServerAddService(virNetServer *srv,
+void virNetServerAddService(virNetServer *srv,
virNetServerService *svc);
int virNetServerAddServiceTCP(virNetServer *srv,
virSystemdActivation *act,
diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c
index 110ec748f8..74c20479d4 100644
--- a/tests/virnetdaemontest.c
+++ b/tests/virnetdaemontest.c
@@ -127,10 +127,8 @@ testCreateServer(const char *server_name, const char *host, int
family)
5)))
goto error;
- if (virNetServerAddService(srv, svc1) < 0)
- goto error;
- if (virNetServerAddService(srv, svc2) < 0)
- goto error;
+ virNetServerAddService(srv, svc1);
+ virNetServerAddService(srv, svc2);
if (virNetSocketNewConnectSockFD(fdclient[0], &sk1) < 0)
goto error;
--
2.43.0
10:08 a.m.
New subject: [PATCH 2/7] rpc: virnetserver: virNetServerAddService() to void
On 1/4/24 01:16, Artem Chernyshev wrote:
virNetServerAddService() return value
is invariant, so change it type and remove all dependent
checks.
Fixes: 7d2fd6ef01 ("Do not check return value of VIR_EXPAND_N")
Signed-off-by: Artem Chernyshev <artem.chernyshev(a)red-soft.ru>
---
src/lxc/lxc_controller.c | 3 +--
src/rpc/virnetserver.c | 25 +++++--------------------
src/rpc/virnetserver.h | 2 +-
tests/virnetdaemontest.c | 6 ++----
4 files changed, 9 insertions(+), 27 deletions(-)
diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h
index 9f0cf3a3fc..dc60e2c2b2 100644
--- a/src/rpc/virnetserver.h
+++ b/src/rpc/virnetserver.h
@@ -59,7 +59,7 @@ void virNetServerShutdownWait(virNetServer *srv);
virJSONValue *virNetServerPreExecRestart(virNetServer *srv);
-int virNetServerAddService(virNetServer *srv,
+void virNetServerAddService(virNetServer *srv,
virNetServerService *svc);
Indentation. Here and in other patches too.
int virNetServerAddServiceTCP(virNetServer *srv,
virSystemdActivation *act,
Michal
6:16 p.m.
New subject: [PATCH 3/7] lxc: virLXCControllerAddConsole() to void
virLXCControllerAddConsole() return value
is invariant, so change it type and remove all dependent
checks.
Fixes: 7d2fd6ef01 ("Do not check return value of VIR_EXPAND_N")
Signed-off-by: Artem Chernyshev <artem.chernyshev(a)red-soft.ru>
---
src/lxc/lxc_controller.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 0a92a24fb7..f809aea6d0 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -313,7 +313,7 @@ static void virLXCControllerFree(virLXCController *ctrl)
}
-static int virLXCControllerAddConsole(virLXCController *ctrl,
+static void virLXCControllerAddConsole(virLXCController *ctrl,
int hostFd)
{
VIR_EXPAND_N(ctrl->consoles, ctrl->nconsoles, 1);
@@ -326,7 +326,6 @@ static int virLXCControllerAddConsole(virLXCController *ctrl,
ctrl->consoles[ctrl->nconsoles-1].epollFd = -1;
ctrl->consoles[ctrl->nconsoles-1].epollWatch = -1;
- return 0;
}
@@ -2655,8 +2654,7 @@ int main(int argc, char *argv[])
}
for (i = 0; i < nttyFDs; i++) {
- if (virLXCControllerAddConsole(ctrl, ttyFDs[i]) < 0)
- goto cleanup;
+ virLXCControllerAddConsole(ctrl, ttyFDs[i]);
ttyFDs[i] = -1;
}
--
2.43.0
6:17 p.m.
New subject: [PATCH 4/7] conf: virDomainNetUpdate() to void
virDomainNetUpdate() return value
is invariant, so change it type and remove all dependent
checks.
Fixes: 98f6f2081d ("util: alloc: Reimplement VIR_APPEND_ELEMENT using
virAppendElement")
Signed-off-by: Artem Chernyshev <artem.chernyshev(a)red-soft.ru>
---
src/conf/domain_conf.c | 3 +--
src/conf/domain_conf.h | 2 +-
src/lxc/lxc_driver.c | 3 +--
src/qemu/qemu_driver.c | 3 +--
4 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 82672b30a0..39e9879a8a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14690,7 +14690,7 @@ virDomainNetRemoveByObj(virDomainDef *def, virDomainNetDef *net)
}
-int
+void
virDomainNetUpdate(virDomainDef *def,
size_t netidx,
virDomainNetDef *newnet)
@@ -14725,7 +14725,6 @@ virDomainNetUpdate(virDomainDef *def,
}
def->nets[netidx] = newnet;
- return 0;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 14901b37ba..58b2c92f4c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3891,7 +3891,7 @@ virDomainNetDef *virDomainNetFind(virDomainDef *def, const char
*device);
virDomainNetDef *virDomainNetFindByName(virDomainDef *def, const char *ifname);
bool virDomainHasNet(virDomainDef *def, virDomainNetDef *net);
int virDomainNetInsert(virDomainDef *def, virDomainNetDef *net);
-int virDomainNetUpdate(virDomainDef *def, size_t netidx, virDomainNetDef *newnet);
+void virDomainNetUpdate(virDomainDef *def, size_t netidx, virDomainNetDef *newnet);
bool virDomainNetBackendIsEqual(virDomainNetBackend *src,
virDomainNetBackend *dst);
int virDomainNetDHCPInterfaces(virDomainDef *def, virDomainInterfacePtr **ifaces);
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 25cbfc57c9..3e9c1395bf 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3084,8 +3084,7 @@ lxcDomainUpdateDeviceConfig(virDomainDef *vmdef,
false) < 0)
return -1;
- if (virDomainNetUpdate(vmdef, idx, net) < 0)
- return -1;
+ virDomainNetUpdate(vmdef, idx, net);
virDomainNetDefFree(oldDev.data.net);
dev->data.net = NULL;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0b7b320648..ee7774f115 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7145,8 +7145,7 @@ qemuDomainUpdateDeviceConfig(virDomainDef *vmdef,
false) < 0)
return -1;
- if (virDomainNetUpdate(vmdef, pos, net))
- return -1;
+ virDomainNetUpdate(vmdef, pos, net);
virDomainNetDefFree(oldDev.data.net);
dev->data.net = NULL;
--
2.43.0
6:17 p.m.
New subject: [PATCH 5/7] virprocess: virProcessGetNamespaces() to void
virProcessGetNamespaces() return value
is invariant, so change it type and remove all dependent
checks.
Fixes: 7d2fd6ef01 ("Do not check return value of VIR_EXPAND_N")
Signed-off-by: Artem Chernyshev <artem.chernyshev(a)red-soft.ru>
---
src/libvirt-lxc.c | 5 +----
src/lxc/lxc_driver.c | 3 +--
src/qemu/qemu_process.c | 3 +--
src/util/virprocess.c | 4 +---
src/util/virprocess.h | 2 +-
5 files changed, 5 insertions(+), 12 deletions(-)
diff --git a/src/libvirt-lxc.c b/src/libvirt-lxc.c
index ecc1207ca9..99bb450702 100644
--- a/src/libvirt-lxc.c
+++ b/src/libvirt-lxc.c
@@ -140,10 +140,7 @@ virDomainLxcEnterNamespace(virDomainPtr domain,
if (noldfdlist && oldfdlist) {
size_t nfds;
- if (virProcessGetNamespaces(getpid(),
- &nfds,
- oldfdlist) < 0)
- goto error;
+ virProcessGetNamespaces(getpid(), &nfds, oldfdlist);
*noldfdlist = nfds;
}
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 3e9c1395bf..c281998652 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -4691,8 +4691,7 @@ static int lxcDomainLxcOpenNamespace(virDomainPtr dom,
goto endjob;
}
- if (virProcessGetNamespaces(priv->initpid, &nfds, fdlist) < 0)
- goto endjob;
+ virProcessGetNamespaces(priv->initpid, &nfds, fdlist);
ret = nfds;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 696d526a5d..7770e7e7eb 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2737,8 +2737,7 @@ qemuProcessStartPRDaemonHook(void *opaque)
int ret = -1;
if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) {
- if (virProcessGetNamespaces(vm->pid, &nfds, &fds) < 0)
- return ret;
+ virProcessGetNamespaces(vm->pid, &nfds, &fds);
if (nfds > 0 &&
virProcessSetNamespaces(nfds, fds) < 0)
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index b6fb17db83..74233e2169 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -652,7 +652,7 @@ int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids)
}
-int virProcessGetNamespaces(pid_t pid,
+void virProcessGetNamespaces(pid_t pid,
size_t *nfdlist,
int **fdlist)
{
@@ -673,8 +673,6 @@ int virProcessGetNamespaces(pid_t pid,
(*fdlist)[(*nfdlist)-1] = fd;
}
}
-
- return 0;
}
diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index 6008cca4af..cc6abc6e4b 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -70,7 +70,7 @@ int virProcessGetPids(pid_t pid, size_t *npids, pid_t **pids);
int virProcessGetStartTime(pid_t pid,
unsigned long long *timestamp);
-int virProcessGetNamespaces(pid_t pid,
+void virProcessGetNamespaces(pid_t pid,
size_t *nfdlist,
int **fdlist);
--
2.43.0
6:17 p.m.
New subject: [PATCH 6/7] node_device: udevTranslatePCIIds() to void
udevTranslatePCIIds() return value
is invariant, so change it type and remove all dependent
checks.
Fixes: 7f1f0453fc ("node_device: use g_strdup instead of VIR_STRDUP")
Signed-off-by: Artem Chernyshev <artem.chernyshev(a)red-soft.ru>
---
src/node_device/node_device_udev.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 911325600e..8d38aec070 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -332,7 +332,7 @@ udevGenerateDeviceName(struct udev_device *device,
static virMutex pciaccessMutex = VIR_MUTEX_INITIALIZER;
-static int
+static void
udevTranslatePCIIds(unsigned int vendor,
unsigned int product,
char **vendor_string,
@@ -356,8 +356,6 @@ udevTranslatePCIIds(unsigned int vendor,
*vendor_string = g_strdup(vendor_name);
*product_string = g_strdup(device_name);
-
- return 0;
}
@@ -398,12 +396,10 @@ udevProcessPCI(struct udev_device *device,
if (udevGetUintSysfsAttr(device, "device", &pci_dev->product, 16)
< 0)
goto cleanup;
- if (udevTranslatePCIIds(pci_dev->vendor,
- pci_dev->product,
- &pci_dev->vendor_name,
- &pci_dev->product_name) != 0) {
- goto cleanup;
- }
+ udevTranslatePCIIds(pci_dev->vendor,
+ pci_dev->product,
+ &pci_dev->vendor_name,
+ &pci_dev->product_name);
udevGenerateDeviceName(device, def, NULL);
--
2.43.0
6:17 p.m.
New subject: [PATCH 7/7] node_device: udevGetStringSysfsAttr() to void
udevGetStringSysfsAttr() return value
is invariant, so change it type and remove all dependent
checks.
Fixes: 7f1f0453fc ("node_device: use g_strdup instead of VIR_STRDUP")
Signed-off-by: Artem Chernyshev <artem.chernyshev(a)red-soft.ru>
---
src/node_device/node_device_udev.c | 55 +++++++++---------------------
1 file changed, 16 insertions(+), 39 deletions(-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 8d38aec070..b610bc9c16 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -245,7 +245,7 @@ udevGetDeviceSysfsAttr(struct udev_device *udev_device,
}
-static int
+static void
udevGetStringSysfsAttr(struct udev_device *udev_device,
const char *attr_name,
char **value)
@@ -256,8 +256,6 @@ udevGetStringSysfsAttr(struct udev_device *udev_device,
if (*value != NULL && (STREQ(*value, "")))
VIR_FREE(*value);
-
- return 0;
}
@@ -536,10 +534,9 @@ udevProcessUSBDevice(struct udev_device *device,
"ID_VENDOR_FROM_DATABASE",
&usb_dev->vendor_name);
- if (!usb_dev->vendor_name &&
+ if (!usb_dev->vendor_name)
udevGetStringSysfsAttr(device, "manufacturer",
- &usb_dev->vendor_name) < 0)
- return -1;
+ &usb_dev->vendor_name);
if (udevGetUintProperty(device, "ID_MODEL_ID", &usb_dev->product,
16) < 0)
return -1;
@@ -548,10 +545,9 @@ udevProcessUSBDevice(struct udev_device *device,
"ID_MODEL_FROM_DATABASE",
&usb_dev->product_name);
- if (!usb_dev->product_name &&
+ if (!usb_dev->product_name)
udevGetStringSysfsAttr(device, "product",
- &usb_dev->product_name) < 0)
- return -1;
+ &usb_dev->product_name);
udevGenerateDeviceName(device, def, NULL);
@@ -602,9 +598,7 @@ udevProcessNetworkInterface(struct udev_device *device,
udevGetStringProperty(device, "INTERFACE", &net->ifname);
- if (udevGetStringSysfsAttr(device, "address",
- &net->address) < 0)
- return -1;
+ udevGetStringSysfsAttr(device, "address", &net->address);
if (udevGetUintSysfsAttr(device, "addr_len", &net->address_len, 0)
< 0)
return -1;
@@ -898,8 +892,7 @@ udevProcessDASD(struct udev_device *device,
{
virNodeDevCapStorage *storage = &def->caps->data.storage;
- if (udevGetStringSysfsAttr(device, "device/uid", &storage->serial)
< 0)
- return -1;
+ udevGetStringSysfsAttr(device, "device/uid", &storage->serial);
return udevProcessDisk(device, def);
}
@@ -972,13 +965,11 @@ udevProcessStorage(struct udev_device *device,
udevGetStringProperty(device, "ID_BUS", &storage->bus);
udevGetStringProperty(device, "ID_SERIAL", &storage->serial);
- if (udevGetStringSysfsAttr(device, "device/vendor",
&storage->vendor) < 0)
- goto cleanup;
+ udevGetStringSysfsAttr(device, "device/vendor", &storage->vendor);
if (def->caps->data.storage.vendor)
virTrimSpaces(def->caps->data.storage.vendor, NULL);
- if (udevGetStringSysfsAttr(device, "device/model", &storage->model)
< 0)
- goto cleanup;
+ udevGetStringSysfsAttr(device, "device/model", &storage->model);
if (def->caps->data.storage.model)
virTrimSpaces(def->caps->data.storage.model, NULL);
/* There is no equivalent of the hotpluggable property in libudev,
@@ -1951,31 +1942,17 @@ udevGetDMIData(virNodeDevCapSystem *syscap)
}
}
- if (udevGetStringSysfsAttr(device, "product_name",
- &syscap->product_name) < 0)
- goto cleanup;
- if (udevGetStringSysfsAttr(device, "sys_vendor",
- &hardware->vendor_name) < 0)
- goto cleanup;
- if (udevGetStringSysfsAttr(device, "product_version",
- &hardware->version) < 0)
- goto cleanup;
- if (udevGetStringSysfsAttr(device, "product_serial",
- &hardware->serial) < 0)
- goto cleanup;
+ udevGetStringSysfsAttr(device, "product_name",
&syscap->product_name);
+ udevGetStringSysfsAttr(device, "sys_vendor",
&hardware->vendor_name);
+ udevGetStringSysfsAttr(device, "product_version",
&hardware->version);
+ udevGetStringSysfsAttr(device, "product_serial",
&hardware->serial);
if (virGetHostUUID(hardware->uuid))
goto cleanup;
- if (udevGetStringSysfsAttr(device, "bios_vendor",
- &firmware->vendor_name) < 0)
- goto cleanup;
- if (udevGetStringSysfsAttr(device, "bios_version",
- &firmware->version) < 0)
- goto cleanup;
- if (udevGetStringSysfsAttr(device, "bios_date",
- &firmware->release_date) < 0)
- goto cleanup;
+ udevGetStringSysfsAttr(device, "bios_vendor",
&firmware->vendor_name);
+ udevGetStringSysfsAttr(device, "bios_version", &firmware->version);
+ udevGetStringSysfsAttr(device, "bios_date",
&firmware->release_date);
cleanup:
if (device != NULL)
--
2.43.0
10:08 a.m.
New subject: [PATCH 7/7] node_device: udevGetStringSysfsAttr() to void
On 1/4/24 01:17, Artem Chernyshev wrote:
udevGetStringSysfsAttr() return value
is invariant, so change it type and remove all dependent
checks.
Fixes: 7f1f0453fc ("node_device: use g_strdup instead of VIR_STRDUP")
Signed-off-by: Artem Chernyshev <artem.chernyshev(a)red-soft.ru>
---
src/node_device/node_device_udev.c | 55 +++++++++---------------------
1 file changed, 16 insertions(+), 39 deletions(-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 8d38aec070..b610bc9c16 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -245,7 +245,7 @@ udevGetDeviceSysfsAttr(struct udev_device *udev_device,
}
-static int
+static void
udevGetStringSysfsAttr(struct udev_device *udev_device,
const char *attr_name,
char **value)
@@ -256,8 +256,6 @@ udevGetStringSysfsAttr(struct udev_device *udev_device,
if (*value != NULL && (STREQ(*value, "")))
VIR_FREE(*value);
-
- return 0;
}
@@ -536,10 +534,9 @@ udevProcessUSBDevice(struct udev_device *device,
"ID_VENDOR_FROM_DATABASE",
&usb_dev->vendor_name);
- if (!usb_dev->vendor_name &&
+ if (!usb_dev->vendor_name)
udevGetStringSysfsAttr(device, "manufacturer",
- &usb_dev->vendor_name) < 0)
- return -1;
+ &usb_dev->vendor_name);
We can take this opportunity and write the code such that it follows our
coding style: https://libvirt.org/coding-style.html#curly-braces
Since the body is a multiline statement it has to be wrapped in curly
braces.
Michal
10:08 a.m.
On 1/4/24 01:16, Artem Chernyshev wrote:
Several functions was modified to become invariant. Change
their type to void and remove unnecessary checks of their
return values.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Artem Chernyshev (7):
cpu: : virCPUx86DataAddItem() to void
rpc: virnetserver: virNetServerAddService() to void
lxc: virLXCControllerAddConsole() to void
conf: virDomainNetUpdate() to void
virprocess: virProcessGetNamespaces() to void
node_device: udevTranslatePCIIds() to void
node_device: udevGetStringSysfsAttr() to void
src/conf/domain_conf.c | 3 +-
src/conf/domain_conf.h | 2 +-
src/cpu/cpu_x86.c | 179 +++++++++++------------------
src/cpu/cpu_x86.h | 4 +-
src/libvirt-lxc.c | 5 +-
src/libxl/libxl_capabilities.c | 13 +--
src/lxc/lxc_controller.c | 9 +-
src/lxc/lxc_driver.c | 6 +-
src/node_device/node_device_udev.c | 69 ++++-------
src/qemu/qemu_capabilities.c | 3 +-
src/qemu/qemu_driver.c | 3 +-
src/qemu/qemu_monitor_json.c | 7 +-
src/qemu/qemu_process.c | 3 +-
src/rpc/virnetserver.c | 25 +---
src/rpc/virnetserver.h | 2 +-
src/util/virprocess.c | 4 +-
src/util/virprocess.h | 2 +-
tests/virnetdaemontest.c | 6 +-
18 files changed, 118 insertions(+), 227 deletions(-)
Fixed small nits I've raised during review and pushed.
Congratulations on your first libvirt contribution!
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
11:48 a.m.
Thank you and sorry for those miss-alignments with coding style. Could you check my first
submission as well?
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/DC...
Artem
322
days inactive
323
days old
13 comments
2 participants
participants (2)
-
Artem Chernyshev -
Michal Prívozník