[libvirt] [PATCH] tests: Lower stack usage below 4096 bytes
Make virtTestLoadFile allocate the buffer to read the file into. Fix logic error in virtTestLoadFile, stop reading on the an empty line. Use virFileReadLimFD in virtTestCaptureProgramOutput. --- m4/virt-compile-warnings.m4 | 10 ++--- tests/commandhelper.c | 5 +- tests/commandtest.c | 14 ++---- tests/conftest.c | 29 +++++++++---- tests/cputest.c | 66 ++++++++++++++++++------------- tests/esxutilstest.c | 2 + tests/interfacexml2xmltest.c | 33 ++++++++++------ tests/networkxml2xmltest.c | 51 ++++++++++++++--------- tests/nodedevxml2xmltest.c | 34 ++++++++++------ tests/nodeinfotest.c | 67 ++++++++++++++++++++----------- tests/nwfilterxml2xmltest.c | 54 +++++++++++++++---------- tests/qemuargv2xmltest.c | 43 ++++++++++++-------- tests/qemuhelptest.c | 13 +++--- tests/qemuxml2argvtest.c | 56 ++++++++++++++++---------- tests/qemuxml2xmltest.c | 43 +++++++++++--------- tests/sexpr2xmltest.c | 51 ++++++++++++++--------- tests/storagepoolxml2xmltest.c | 51 ++++++++++++++--------- tests/storagevolxml2xmltest.c | 65 ++++++++++++++++++------------- tests/testutils.c | 69 ++++++++++++++------------------ tests/testutils.h | 8 +--- tests/virshtest.c | 65 +++++++++++++----------------- tests/vmx2xmltest.c | 45 ++++++++++++--------- tests/xencapstest.c | 38 ++++++++--------- tests/xmconfigtest.c | 85 +++++++++++++++++++++++---------------- tests/xml2sexprtest.c | 53 +++++++++++++++---------- tests/xml2vmxtest.c | 45 ++++++++++++--------- 26 files changed, 624 insertions(+), 471 deletions(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 295fd9b..305036f 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -88,12 +88,10 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ # that one off, so we need to manually enable this again gl_WARN_ADD([-Wjump-misses-init]) - # This should be < 256 really, but with PATH_MAX everywhere - # we have doom, even with 4096. In fact we have some functions - # with several PATH_MAX sized variables :-( We should kill off - # all PATH_MAX usage and then lower this limit - gl_WARN_ADD([-Wframe-larger-than=65700]) - dnl gl_WARN_ADD([-Wframe-larger-than=4096]) + # This should be < 256 really. Currently we're down to 4096, + # but using 1024 bytes sized buffers (mostly for virStrerror) + # stops us from going down further + gl_WARN_ADD([-Wframe-larger-than=4096]) dnl gl_WARN_ADD([-Wframe-larger-than=256]) # Extra special flags diff --git a/tests/commandhelper.c b/tests/commandhelper.c index 46c00f4..99ba22e 100644 --- a/tests/commandhelper.c +++ b/tests/commandhelper.c @@ -99,8 +99,8 @@ int main(int argc, char **argv) { } fprintf(log, "DAEMON:%s\n", getpgrp() == getsid(0) ? "yes" : "no"); - char cwd[1024]; - if (!getcwd(cwd, sizeof(cwd))) + char *cwd = NULL; + if (!(cwd = getcwd(NULL, 0))) return EXIT_FAILURE; if (strlen(cwd) > strlen("/commanddata") && STREQ(cwd + strlen(cwd) - strlen("/commanddata"), "/commanddata")) @@ -137,5 +137,6 @@ int main(int argc, char **argv) { return EXIT_SUCCESS; error: + free(cwd); return EXIT_FAILURE; } diff --git a/tests/commandtest.c b/tests/commandtest.c index c313a2c..a728d10 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -47,21 +47,18 @@ mymain(int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED) #else static char *progname; +static char cwd[PATH_MAX]; static char *abs_srcdir; static int checkoutput(const char *testname) { int ret = -1; - char cwd[1024]; char *expectname = NULL; char *expectlog = NULL; char *actualname = NULL; char *actuallog = NULL; - if (!getcwd(cwd, sizeof(cwd))) - return -1; - if (virAsprintf(&expectname, "%s/commanddata/%s.log", abs_srcdir, testname) < 0) goto cleanup; @@ -545,12 +542,12 @@ cleanup: static int test15(const void *unused ATTRIBUTE_UNUSED) { virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); - char *cwd = NULL; + char *wd = NULL; int ret = -1; - if (virAsprintf(&cwd, "%s/commanddata", abs_srcdir) < 0) + if (virAsprintf(&wd, "%s/commanddata", abs_srcdir) < 0) goto cleanup; - virCommandSetWorkingDirectory(cmd, cwd); + virCommandSetWorkingDirectory(cmd, wd); if (virCommandRun(cmd, NULL) < 0) { virErrorPtr err = virGetLastError(); @@ -561,7 +558,7 @@ static int test15(const void *unused ATTRIBUTE_UNUSED) ret = checkoutput("test15"); cleanup: - VIR_FREE(cwd); + VIR_FREE(wd); virCommandFree(cmd); return ret; @@ -757,7 +754,6 @@ static int mymain(int argc, char **argv) { int ret = 0; - char cwd[PATH_MAX]; int fd; abs_srcdir = getenv("abs_srcdir"); diff --git a/tests/conftest.c b/tests/conftest.c index a7977bb..6514f4d 100644 --- a/tests/conftest.c +++ b/tests/conftest.c @@ -6,32 +6,43 @@ #include <string.h> #include <errno.h> #include "conf.h" +#include "memory.h" -int main(int argc, char **argv) { - int ret; +int main(int argc, char **argv) +{ + int ret, exit_code = EXIT_FAILURE; virConfPtr conf; int len = 10000; - char buffer[10000]; + char *buffer = NULL; if (argc != 2) { fprintf(stderr, "Usage: %s conf_file\n", argv[0]); - exit(EXIT_FAILURE); + goto cleanup; } + if (VIR_ALLOC_N(buffer, len) < 0) { + fprintf(stderr, "out of memory\n"); + goto cleanup; + } conf = virConfReadFile(argv[1], 0); if (conf == NULL) { fprintf(stderr, "Failed to process %s\n", argv[1]); - exit(EXIT_FAILURE); + goto cleanup; } - ret = virConfWriteMem(&buffer[0], &len, conf); + ret = virConfWriteMem(buffer, &len, conf); if (ret < 0) { fprintf(stderr, "Failed to serialize %s back\n", argv[1]); - exit(EXIT_FAILURE); + goto cleanup; } virConfFree(conf); if (fwrite(buffer, 1, len, stdout) != len) { fprintf(stderr, "Write failed: %s\n", strerror (errno)); - exit(EXIT_FAILURE); + goto cleanup; } - exit(EXIT_SUCCESS); + + exit_code = EXIT_SUCCESS; + +cleanup: + VIR_FREE(buffer); + return exit_code; } diff --git a/tests/cputest.c b/tests/cputest.c index bd06958..831fa81 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -40,11 +40,11 @@ #include "cpu/cpu_map.h" static const char *progname; +static char cwd[PATH_MAX]; static const char *abs_srcdir; static const char *abs_top_srcdir; #define VIR_FROM_THIS VIR_FROM_CPU -#define MAX_FILE 4096 enum compResultShadow { ERROR = VIR_CPU_COMPARE_ERROR, @@ -91,14 +91,13 @@ struct data { static virCPUDefPtr cpuTestLoadXML(const char *arch, const char *name) { - char xml[PATH_MAX]; + char *xml = NULL; xmlDocPtr doc = NULL; xmlXPathContextPtr ctxt = NULL; virCPUDefPtr cpu = NULL; - snprintf(xml, PATH_MAX, - "%s/cputestdata/%s-%s.xml", - abs_srcdir, arch, name); + if (virAsprintf(&xml, "%s/cputestdata/%s-%s.xml", abs_srcdir, arch, name) < 0) + goto cleanup; if (!(doc = virXMLParseFile(xml)) || !(ctxt = xmlXPathNewContext(doc))) @@ -110,6 +109,7 @@ cpuTestLoadXML(const char *arch, const char *name) cleanup: xmlXPathFreeContext(ctxt); xmlFreeDoc(doc); + free(xml); return cpu; } @@ -119,7 +119,7 @@ cpuTestLoadMultiXML(const char *arch, const char *name, unsigned int *count) { - char xml[PATH_MAX]; + char *xml = NULL; xmlDocPtr doc = NULL; xmlXPathContextPtr ctxt = NULL; xmlNodePtr *nodes = NULL; @@ -127,9 +127,8 @@ cpuTestLoadMultiXML(const char *arch, int n; int i; - snprintf(xml, PATH_MAX, - "%s/cputestdata/%s-%s.xml", - abs_srcdir, arch, name); + if (virAsprintf(&xml, "%s/cputestdata/%s-%s.xml", abs_srcdir, arch, name) < 0) + goto cleanup; if (!(doc = virXMLParseFile(xml)) || !(ctxt = xmlXPathNewContext(doc))) @@ -151,6 +150,7 @@ cpuTestLoadMultiXML(const char *arch, *count = n; cleanup: + free(xml); free(nodes); xmlXPathFreeContext(ctxt); xmlFreeDoc(doc); @@ -172,17 +172,16 @@ cpuTestCompareXML(const char *arch, const virCPUDefPtr cpu, const char *name) { - char xml[PATH_MAX]; - char expected[MAX_FILE]; - char *expectedPtr = &(expected[0]); + char *xml = NULL; + char *expected = NULL; char *actual = NULL; int ret = -1; - snprintf(xml, PATH_MAX, - "%s/cputestdata/%s-%s.xml", - abs_srcdir, arch, name); + if (virAsprintf(&xml, "%s/cputestdata/%s-%s.xml", + abs_srcdir, arch, name) < 0) + goto cleanup; - if (virtTestLoadFile(xml, &expectedPtr, MAX_FILE) < 0) + if (virtTestLoadFile(xml, &expected) < 0) goto cleanup; if (!(actual = virCPUDefFormat(cpu, NULL, 0))) @@ -196,6 +195,8 @@ cpuTestCompareXML(const char *arch, ret = 0; cleanup: + free(xml); + free(expected); free(actual); return ret; } @@ -335,7 +336,7 @@ cpuTestBaseline(const void *arg) virCPUDefPtr *cpus = NULL; virCPUDefPtr baseline = NULL; unsigned int ncpus = 0; - char result[PATH_MAX]; + char *result = NULL; unsigned int i; if (!(cpus = cpuTestLoadMultiXML(data->arch, data->name, &ncpus))) @@ -355,7 +356,9 @@ cpuTestBaseline(const void *arg) if (!baseline) goto cleanup; - snprintf(result, PATH_MAX, "%s-result", data->name); + if (virAsprintf(&result, "%s-result", data->name) < 0) + goto cleanup; + if (cpuTestCompareXML(data->arch, baseline, result) < 0) goto cleanup; @@ -384,6 +387,7 @@ cleanup: free(cpus); } virCPUDefFree(baseline); + free(result); return ret; } @@ -395,7 +399,7 @@ cpuTestUpdate(const void *arg) int ret = -1; virCPUDefPtr host = NULL; virCPUDefPtr cpu = NULL; - char result[PATH_MAX]; + char *result = NULL; if (!(host = cpuTestLoadXML(data->arch, data->host)) || !(cpu = cpuTestLoadXML(data->arch, data->name))) @@ -404,12 +408,15 @@ cpuTestUpdate(const void *arg) if (cpuUpdate(cpu, host) < 0) goto cleanup; - snprintf(result, PATH_MAX, "%s+%s", data->host, data->name); + if (virAsprintf(&result, "%s+%s", data->host, data->name) < 0) + goto cleanup; + ret = cpuTestCompareXML(data->arch, cpu, result); cleanup: virCPUDefFree(host); virCPUDefFree(cpu); + free(result); return ret; } @@ -467,10 +474,10 @@ static int (*cpuTest[])(const void *) = { static int cpuTestRun(const char *name, const struct data *data) { - char label[PATH_MAX]; + char *label = NULL; - snprintf(label, PATH_MAX, "CPU %s(%s): %s", - apis[data->api], data->arch, name); + if (virAsprintf(&label, "CPU %s(%s): %s", apis[data->api], data->arch, name) < 0) + return -1; free(virtTestLogContentAndReset()); @@ -482,9 +489,12 @@ cpuTestRun(const char *name, const struct data *data) fprintf(stderr, "\n%s\n", log); free(log); } + + free(label); return -1; } + free(label); return 0; } @@ -497,8 +507,7 @@ static int mymain(int argc, char **argv) { int ret = 0; - char cwd[PATH_MAX]; - char map[PATH_MAX]; + char *map = NULL; progname = argv[0]; @@ -515,9 +524,11 @@ mymain(int argc, char **argv) if (!abs_top_srcdir) abs_top_srcdir = ".."; - snprintf(map, PATH_MAX, "%s/src/cpu/cpu_map.xml", abs_top_srcdir); - if (cpuMapOverride(map) < 0) + if (virAsprintf(&map, "%s/src/cpu/cpu_map.xml", abs_top_srcdir) < 0 || + cpuMapOverride(map) < 0) { + free(map); return EXIT_FAILURE; + } #define DO_TEST(arch, api, name, host, cpu, \ models, nmodels, preferred, result) \ @@ -627,6 +638,7 @@ mymain(int argc, char **argv) DO_TEST_GUESTDATA("x86", "host", "guest", models, "qemu64", 0); DO_TEST_GUESTDATA("x86", "host", "guest", nomodel, NULL, -1); + free(map); return (ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE); } diff --git a/tests/esxutilstest.c b/tests/esxutilstest.c index b6bf87b..78761a1 100644 --- a/tests/esxutilstest.c +++ b/tests/esxutilstest.c @@ -276,6 +276,7 @@ testEscapeDatastoreItem(const void *data ATTRIBUTE_UNUSED) } } + VIR_FREE(escaped); return 0; } @@ -317,6 +318,7 @@ testConvertWindows1252ToUTF8(const void *data ATTRIBUTE_UNUSED) } } + VIR_FREE(utf8); return 0; } diff --git a/tests/interfacexml2xmltest.c b/tests/interfacexml2xmltest.c index 094ccf5..26a19ab 100644 --- a/tests/interfacexml2xmltest.c +++ b/tests/interfacexml2xmltest.c @@ -14,19 +14,19 @@ #include "testutilsqemu.h" static char *progname; +static char cwd[PATH_MAX]; static char *abs_srcdir; -#define MAX_FILE 4096 - -static int testCompareXMLToXMLFiles(const char *xml) { - char xmlData[MAX_FILE]; - char *xmlPtr = &(xmlData[0]); +static int +testCompareXMLToXMLFiles(const char *xml) +{ + char *xmlData = NULL; char *actual = NULL; int ret = -1; virInterfaceDefPtr dev = NULL; - if (virtTestLoadFile(xml, &xmlPtr, MAX_FILE) < 0) + if (virtTestLoadFile(xml, &xmlData) < 0) goto fail; if (!(dev = virInterfaceDefParseString(xmlData))) @@ -43,16 +43,26 @@ static int testCompareXMLToXMLFiles(const char *xml) { ret = 0; fail: + free(xmlData); free(actual); virInterfaceDefFree(dev); return ret; } -static int testCompareXMLToXMLHelper(const void *data) { - char xml[PATH_MAX]; - snprintf(xml, PATH_MAX, "%s/interfaceschemadata/%s.xml", - abs_srcdir, (const char*)data); - return testCompareXMLToXMLFiles(xml); +static int +testCompareXMLToXMLHelper(const void *data) +{ + int result = -1; + char *xml = NULL; + + if (virAsprintf(&xml, "%s/interfaceschemadata/%s.xml", + abs_srcdir, (const char*)data) < 0) + return -1; + + result = testCompareXMLToXMLFiles(xml); + + free (xml); + return result; } @@ -60,7 +70,6 @@ static int mymain(int argc, char **argv) { int ret = 0; - char cwd[PATH_MAX]; progname = argv[0]; diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c index 7805548..6faeab2 100644 --- a/tests/networkxml2xmltest.c +++ b/tests/networkxml2xmltest.c @@ -14,23 +14,21 @@ #include "testutilsqemu.h" static char *progname; +static char cwd[PATH_MAX]; static char *abs_srcdir; -#define MAX_FILE 4096 - - -static int testCompareXMLToXMLFiles(const char *inxml, const char *outxml) { - char inXmlData[MAX_FILE]; - char *inXmlPtr = &(inXmlData[0]); - char outXmlData[MAX_FILE]; - char *outXmlPtr = &(outXmlData[0]); +static int +testCompareXMLToXMLFiles(const char *inxml, const char *outxml) +{ + char *inXmlData = NULL; + char *outXmlData = NULL; char *actual = NULL; int ret = -1; virNetworkDefPtr dev = NULL; - if (virtTestLoadFile(inxml, &inXmlPtr, MAX_FILE) < 0) + if (virtTestLoadFile(inxml, &inXmlData) < 0) goto fail; - if (virtTestLoadFile(outxml, &outXmlPtr, MAX_FILE) < 0) + if (virtTestLoadFile(outxml, &outXmlData) < 0) goto fail; if (!(dev = virNetworkDefParseString(inXmlData))) @@ -47,27 +45,40 @@ static int testCompareXMLToXMLFiles(const char *inxml, const char *outxml) { ret = 0; fail: + free(inXmlData); + free(outXmlData); free(actual); virNetworkDefFree(dev); return ret; } -static int testCompareXMLToXMLHelper(const void *data) { - char inxml[PATH_MAX]; - char outxml[PATH_MAX]; - snprintf(inxml, PATH_MAX, "%s/networkxml2xmlin/%s.xml", - abs_srcdir, (const char*)data); - snprintf(outxml, PATH_MAX, "%s/networkxml2xmlout/%s.xml", - abs_srcdir, (const char*)data); - return testCompareXMLToXMLFiles(inxml, outxml); -} +static int +testCompareXMLToXMLHelper(const void *data) +{ + int result = -1; + char *inxml = NULL; + char *outxml = NULL; + + if (virAsprintf(&inxml, "%s/networkxml2xmlin/%s.xml", + abs_srcdir, (const char*)data) < 0 || + virAsprintf(&outxml, "%s/networkxml2xmlout/%s.xml", + abs_srcdir, (const char*)data) < 0) { + goto cleanup; + } + + result = testCompareXMLToXMLFiles(inxml, outxml); +cleanup: + free(inxml); + free(outxml); + + return result; +} static int mymain(int argc, char **argv) { int ret = 0; - char cwd[PATH_MAX]; progname = argv[0]; diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c index 2bc6743..e58883b 100644 --- a/tests/nodedevxml2xmltest.c +++ b/tests/nodedevxml2xmltest.c @@ -14,19 +14,18 @@ #include "testutilsqemu.h" static char *progname; +static char cwd[PATH_MAX]; static char *abs_srcdir; -#define MAX_FILE 4096 - - -static int testCompareXMLToXMLFiles(const char *xml) { - char xmlData[MAX_FILE]; - char *xmlPtr = &(xmlData[0]); +static int +testCompareXMLToXMLFiles(const char *xml) +{ + char *xmlData = NULL; char *actual = NULL; int ret = -1; virNodeDeviceDefPtr dev = NULL; - if (virtTestLoadFile(xml, &xmlPtr, MAX_FILE) < 0) + if (virtTestLoadFile(xml, &xmlData) < 0) goto fail; if (!(dev = virNodeDeviceDefParseString(xmlData, EXISTING_DEVICE))) @@ -43,16 +42,26 @@ static int testCompareXMLToXMLFiles(const char *xml) { ret = 0; fail: + free(xmlData); free(actual); virNodeDeviceDefFree(dev); return ret; } -static int testCompareXMLToXMLHelper(const void *data) { - char xml[PATH_MAX]; - snprintf(xml, PATH_MAX, "%s/nodedevschemadata/%s.xml", - abs_srcdir, (const char*)data); - return testCompareXMLToXMLFiles(xml); +static int +testCompareXMLToXMLHelper(const void *data) +{ + int result = -1; + char *xml = NULL; + + if (virAsprintf(&xml, "%s/nodedevschemadata/%s.xml", + abs_srcdir, (const char*)data) < 0) + return -1; + + result = testCompareXMLToXMLFiles(xml); + + free(xml); + return result; } @@ -60,7 +69,6 @@ static int mymain(int argc, char **argv) { int ret = 0; - char cwd[PATH_MAX]; progname = argv[0]; diff --git a/tests/nodeinfotest.c b/tests/nodeinfotest.c index c690403..0d8c13b 100644 --- a/tests/nodeinfotest.c +++ b/tests/nodeinfotest.c @@ -22,26 +22,27 @@ mymain(int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED) #else static char *progname; +static char cwd[PATH_MAX]; static char *abs_srcdir; -# define MAX_FILE 4096 - extern int linuxNodeInfoCPUPopulate(FILE *cpuinfo, virNodeInfoPtr nodeinfo, bool need_hyperthreads); -static int linuxTestCompareFiles(const char *cpuinfofile, const char *outputfile) { - char actualData[MAX_FILE]; - char expectData[MAX_FILE]; - char *expect = &expectData[0]; +static int +linuxTestCompareFiles(const char *cpuinfofile, const char *outputfile) +{ + int ret = -1; + char *actualData = NULL; + char *expectData = NULL; virNodeInfo nodeinfo; FILE *cpuinfo; - if (virtTestLoadFile(outputfile, &expect, MAX_FILE) < 0) - return -1; + if (virtTestLoadFile(outputfile, &expectData) < 0) + goto fail; cpuinfo = fopen(cpuinfofile, "r"); if (!cpuinfo) - return -1; + goto fail; memset(&nodeinfo, 0, sizeof(nodeinfo)); if (linuxNodeInfoCPUPopulate(cpuinfo, &nodeinfo, false) < 0) { @@ -52,7 +53,7 @@ static int linuxTestCompareFiles(const char *cpuinfofile, const char *outputfile virFreeError(error); } VIR_FORCE_FCLOSE(cpuinfo); - return -1; + goto fail; } VIR_FORCE_FCLOSE(cpuinfo); @@ -61,30 +62,49 @@ static int linuxTestCompareFiles(const char *cpuinfofile, const char *outputfile * so blank it to a predictable value */ nodeinfo.nodes = 1; - snprintf(actualData, MAX_FILE, - "CPUs: %u, MHz: %u, Nodes: %u, Cores: %u\n", - nodeinfo.cpus, nodeinfo.mhz, nodeinfo.nodes, nodeinfo.cores); + if (virAsprintf(&actualData, "CPUs: %u, MHz: %u, Nodes: %u, Cores: %u\n", + nodeinfo.cpus, nodeinfo.mhz, nodeinfo.nodes, + nodeinfo.cores) < 0) + goto fail; if (STRNEQ(actualData, expectData)) { if (getenv("DEBUG_TESTS")) { printf("Expect %d '%s'\n", (int)strlen(expectData), expectData); printf("Actual %d '%s'\n", (int)strlen(actualData), actualData); } - return -1; + goto fail; } - return 0; + ret = 0; + +fail: + free(expectData); + free(actualData); + return ret; } -static int linuxTestNodeInfo(const void *data) { - char cpuinfo[PATH_MAX]; - char output[PATH_MAX]; - snprintf(cpuinfo, PATH_MAX, "%s/nodeinfodata/linux-%s.cpuinfo", - abs_srcdir, (const char*)data); - snprintf(output, PATH_MAX, "%s/nodeinfodata/linux-%s.txt", - abs_srcdir, (const char*)data); - return linuxTestCompareFiles(cpuinfo, output); +static int +linuxTestNodeInfo(const void *data) +{ + int result = -1; + char *cpuinfo = NULL; + char *output = NULL; + + if (virAsprintf(&cpuinfo, "%s/nodeinfodata/linux-%s.cpuinfo", + abs_srcdir, (const char*)data) < 0 || + virAsprintf(&output, "%s/nodeinfodata/linux-%s.txt", + abs_srcdir, (const char*)data) < 0) { + goto cleanup; + } + + result = linuxTestCompareFiles(cpuinfo, output); + +cleanup: + free(cpuinfo); + free(output); + + return result; } @@ -101,7 +121,6 @@ mymain(int argc, char **argv) "nodeinfo-5", "nodeinfo-6", }; - char cwd[PATH_MAX]; abs_srcdir = getenv("abs_srcdir"); if (!abs_srcdir) diff --git a/tests/nwfilterxml2xmltest.c b/tests/nwfilterxml2xmltest.c index 575177a..0463a79 100644 --- a/tests/nwfilterxml2xmltest.c +++ b/tests/nwfilterxml2xmltest.c @@ -17,25 +17,22 @@ #include "testutilsqemu.h" static char *progname; +static char cwd[PATH_MAX]; static char *abs_srcdir; -#define MAX_FILE 4096 - - -static int testCompareXMLToXMLFiles(const char *inxml, - const char *outxml, - bool expect_error) { - char inXmlData[MAX_FILE]; - char *inXmlPtr = &(inXmlData[0]); - char outXmlData[MAX_FILE]; - char *outXmlPtr = &(outXmlData[0]); +static int +testCompareXMLToXMLFiles(const char *inxml, const char *outxml, + bool expect_error) +{ + char *inXmlData = NULL; + char *outXmlData = NULL; char *actual = NULL; int ret = -1; virNWFilterDefPtr dev = NULL; - if (virtTestLoadFile(inxml, &inXmlPtr, MAX_FILE) < 0) + if (virtTestLoadFile(inxml, &inXmlData) < 0) goto fail; - if (virtTestLoadFile(outxml, &outXmlPtr, MAX_FILE) < 0) + if (virtTestLoadFile(outxml, &outXmlData) < 0) goto fail; virResetLastError(); @@ -62,6 +59,8 @@ static int testCompareXMLToXMLFiles(const char *inxml, ret = 0; fail: + free(inXmlData); + free(outXmlData); free(actual); virNWFilterDefFree(dev); return ret; @@ -72,23 +71,34 @@ typedef struct test_parms { bool expect_warning; } test_parms; -static int testCompareXMLToXMLHelper(const void *data) { +static int +testCompareXMLToXMLHelper(const void *data) +{ + int result = -1; const test_parms *tp = data; - char inxml[PATH_MAX]; - char outxml[PATH_MAX]; - snprintf(inxml, PATH_MAX, "%s/nwfilterxml2xmlin/%s.xml", - abs_srcdir, tp->name); - snprintf(outxml, PATH_MAX, "%s/nwfilterxml2xmlout/%s.xml", - abs_srcdir, tp->name); - return testCompareXMLToXMLFiles(inxml, outxml, tp->expect_warning); -} + char *inxml = NULL; + char *outxml = NULL; + + if (virAsprintf(&inxml, "%s/nwfilterxml2xmlin/%s.xml", + abs_srcdir, tp->name) < 0 || + virAsprintf(&outxml, "%s/nwfilterxml2xmlout/%s.xml", + abs_srcdir, tp->name) < 0) { + goto cleanup; + } + + result = testCompareXMLToXMLFiles(inxml, outxml, tp->expect_warning); +cleanup: + free(inxml); + free(outxml); + + return result; +} static int mymain(int argc, char **argv) { int ret = 0; - char cwd[PATH_MAX]; progname = argv[0]; diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 0214db0..47e452e 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -17,11 +17,10 @@ # include "testutilsqemu.h" static char *progname; +static char cwd[PATH_MAX]; static char *abs_srcdir; static struct qemud_driver driver; -# define MAX_FILE 4096 - static int blankProblemElements(char *data) { if (virtTestClearLineRegex("<name>[[:alnum:]]+</name>", data) < 0 || @@ -37,18 +36,16 @@ static int blankProblemElements(char *data) static int testCompareXMLToArgvFiles(const char *xml, const char *cmdfile, bool expect_warning) { - char xmlData[MAX_FILE]; - char cmdData[MAX_FILE]; - char *expectxml = &(xmlData[0]); + char *expectxml = NULL; char *actualxml = NULL; - char *cmd = &(cmdData[0]); + char *cmd = NULL; int ret = -1; virDomainDefPtr vmdef = NULL; char *log; - if (virtTestLoadFile(cmdfile, &cmd, MAX_FILE) < 0) + if (virtTestLoadFile(cmdfile, &cmd) < 0) goto fail; - if (virtTestLoadFile(xml, &expectxml, MAX_FILE) < 0) + if (virtTestLoadFile(xml, &expectxml) < 0) goto fail; if (!(vmdef = qemuParseCommandLineString(driver.caps, cmd))) @@ -77,7 +74,9 @@ static int testCompareXMLToArgvFiles(const char *xml, ret = 0; fail: + free(expectxml); free(actualxml); + free(cmd); virDomainDefFree(vmdef); return ret; } @@ -89,15 +88,26 @@ struct testInfo { const char *migrateFrom; }; -static int testCompareXMLToArgvHelper(const void *data) { +static int +testCompareXMLToArgvHelper(const void *data) +{ + int result = -1; const struct testInfo *info = data; - char xml[PATH_MAX]; - char args[PATH_MAX]; - snprintf(xml, PATH_MAX, "%s/qemuxml2argvdata/qemuxml2argv-%s.xml", - abs_srcdir, info->name); - snprintf(args, PATH_MAX, "%s/qemuxml2argvdata/qemuxml2argv-%s.args", - abs_srcdir, info->name); - return testCompareXMLToArgvFiles(xml, args, !!info->extraFlags); + char *xml = NULL; + char *args = NULL; + + if (virAsprintf(&xml, "%s/qemuxml2argvdata/qemuxml2argv-%s.xml", + abs_srcdir, info->name) < 0 || + virAsprintf(&args, "%s/qemuxml2argvdata/qemuxml2argv-%s.args", + abs_srcdir, info->name) < 0) + goto cleanup; + + result = testCompareXMLToArgvFiles(xml, args, !!info->extraFlags); + +cleanup: + free(xml); + free(args); + return result; } @@ -106,7 +116,6 @@ static int mymain(int argc, char **argv) { int ret = 0; - char cwd[PATH_MAX]; progname = argv[0]; diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index c86c578..4838e9a 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -9,8 +9,6 @@ # include "qemu/qemu_capabilities.h" # include "memory.h" -# define MAX_HELP_OUTPUT_SIZE 1024*64 - struct testInfo { const char *name; virBitmapPtr flags; @@ -20,6 +18,7 @@ struct testInfo { }; static char *progname; +static char cwd[PATH_MAX]; static char *abs_srcdir; static void printMismatchedFlags(virBitmapPtr got, @@ -41,8 +40,7 @@ static int testHelpStrParsing(const void *data) { const struct testInfo *info = data; char *path = NULL; - char helpStr[MAX_HELP_OUTPUT_SIZE]; - char *help = &(helpStr[0]); + char *help = NULL; unsigned int version, is_kvm, kvm_version; virBitmapPtr flags = NULL; int ret = -1; @@ -52,7 +50,7 @@ static int testHelpStrParsing(const void *data) if (virAsprintf(&path, "%s/qemuhelpdata/%s", abs_srcdir, info->name) < 0) return -1; - if (virtTestLoadFile(path, &help, MAX_HELP_OUTPUT_SIZE) < 0) + if (virtTestLoadFile(path, &help) < 0) goto cleanup; if (!(flags = qemuCapsNew())) @@ -64,11 +62,12 @@ static int testHelpStrParsing(const void *data) if (qemuCapsGet(info->flags, QEMU_CAPS_DEVICE)) { VIR_FREE(path); + VIR_FREE(help); if (virAsprintf(&path, "%s/qemuhelpdata/%s-device", abs_srcdir, info->name) < 0) goto cleanup; - if (virtTestLoadFile(path, &help, MAX_HELP_OUTPUT_SIZE) < 0) + if (virtTestLoadFile(path, &help) < 0) goto cleanup; if (qemuCapsParseDeviceStr(help, flags) < 0) @@ -114,6 +113,7 @@ static int testHelpStrParsing(const void *data) ret = 0; cleanup: VIR_FREE(path); + VIR_FREE(help); qemuCapsFree(flags); VIR_FREE(got); VIR_FREE(expected); @@ -124,7 +124,6 @@ static int mymain(int argc, char **argv) { int ret = 0; - char cwd[PATH_MAX]; progname = argv[0]; diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c06bdf6..023f5df 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -20,20 +20,19 @@ # include "testutilsqemu.h" static char *progname; +static char cwd[PATH_MAX]; static char *abs_srcdir; static const char *abs_top_srcdir; static struct qemud_driver driver; -# define MAX_FILE 4096 - static int testCompareXMLToArgvFiles(const char *xml, const char *cmdline, virBitmapPtr extraFlags, const char *migrateFrom, int migrateFd, - bool expectError) { - char argvData[MAX_FILE]; - char *expectargv = &(argvData[0]); + bool expectError) +{ + char *expectargv = NULL; int len; char *actualargv = NULL; int ret = -1; @@ -47,7 +46,7 @@ static int testCompareXMLToArgvFiles(const char *xml, if (!(conn = virGetConnect())) goto fail; - len = virtTestLoadFile(cmdline, &expectargv, MAX_FILE); + len = virtTestLoadFile(cmdline, &expectargv); if (len < 0) goto fail; if (len && expectargv[len - 1] == '\n') @@ -158,6 +157,7 @@ static int testCompareXMLToArgvFiles(const char *xml, fail: free(log); free(emulator); + free(expectargv); free(actualargv); virCommandFree(cmd); virDomainDefFree(vmdef); @@ -174,17 +174,28 @@ struct testInfo { bool expectError; }; -static int testCompareXMLToArgvHelper(const void *data) { +static int +testCompareXMLToArgvHelper(const void *data) +{ + int result = -1; const struct testInfo *info = data; - char xml[PATH_MAX]; - char args[PATH_MAX]; - snprintf(xml, PATH_MAX, "%s/qemuxml2argvdata/qemuxml2argv-%s.xml", - abs_srcdir, info->name); - snprintf(args, PATH_MAX, "%s/qemuxml2argvdata/qemuxml2argv-%s.args", - abs_srcdir, info->name); - return testCompareXMLToArgvFiles(xml, args, info->extraFlags, - info->migrateFrom, info->migrateFd, - info->expectError); + char *xml = NULL; + char *args = NULL; + + if (virAsprintf(&xml, "%s/qemuxml2argvdata/qemuxml2argv-%s.xml", + abs_srcdir, info->name) < 0 || + virAsprintf(&args, "%s/qemuxml2argvdata/qemuxml2argv-%s.args", + abs_srcdir, info->name) < 0) + goto cleanup; + + result = testCompareXMLToArgvFiles(xml, args, info->extraFlags, + info->migrateFrom, info->migrateFd, + info->expectError); + +cleanup: + free(xml); + free(args); + return result; } @@ -193,14 +204,13 @@ static int mymain(int argc, char **argv) { int ret = 0; - char cwd[PATH_MAX]; - char map[PATH_MAX]; + char *map = NULL; progname = argv[0]; if (argc > 1) { fprintf(stderr, "Usage: %s\n", progname); - return (EXIT_FAILURE); + return EXIT_FAILURE; } abs_srcdir = getenv("abs_srcdir"); @@ -224,10 +234,11 @@ mymain(int argc, char **argv) return EXIT_FAILURE; if (!(driver.spicePassword = strdup("123456"))) return EXIT_FAILURE; - - snprintf(map, PATH_MAX, "%s/src/cpu/cpu_map.xml", abs_top_srcdir); - if (cpuMapOverride(map) < 0) + if (virAsprintf(&map, "%s/src/cpu/cpu_map.xml", abs_top_srcdir) < 0 || + cpuMapOverride(map) < 0) { + free(map); return EXIT_FAILURE; + } # define DO_TEST_FULL(name, migrateFrom, migrateFd, expectError, ...) \ do { \ @@ -498,6 +509,7 @@ mymain(int argc, char **argv) free(driver.stateDir); virCapabilitiesFree(driver.caps); + free(map); return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); } diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 27330a9..9d8beeb 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -16,24 +16,22 @@ # include "testutilsqemu.h" static char *progname; +static char cwd[PATH_MAX]; static char *abs_srcdir; static struct qemud_driver driver; -# define MAX_FILE 4096 - - -static int testCompareXMLToXMLFiles(const char *inxml, const char *outxml) { - char inXmlData[MAX_FILE]; - char *inXmlPtr = &(inXmlData[0]); - char outXmlData[MAX_FILE]; - char *outXmlPtr = &(outXmlData[0]); +static int +testCompareXMLToXMLFiles(const char *inxml, const char *outxml) +{ + char *inXmlData = NULL; + char *outXmlData = NULL; char *actual = NULL; int ret = -1; virDomainDefPtr def = NULL; - if (virtTestLoadFile(inxml, &inXmlPtr, MAX_FILE) < 0) + if (virtTestLoadFile(inxml, &inXmlData) < 0) goto fail; - if (virtTestLoadFile(outxml, &outXmlPtr, MAX_FILE) < 0) + if (virtTestLoadFile(outxml, &outXmlData) < 0) goto fail; if (!(def = virDomainDefParseString(driver.caps, inXmlData, @@ -51,6 +49,8 @@ static int testCompareXMLToXMLFiles(const char *inxml, const char *outxml) { ret = 0; fail: + free(inXmlData); + free(outXmlData); free(actual); virDomainDefFree(def); return ret; @@ -61,16 +61,19 @@ struct testInfo { int different; }; -static int testCompareXMLToXMLHelper(const void *data) { +static int +testCompareXMLToXMLHelper(const void *data) +{ const struct testInfo *info = data; - char xml_in[PATH_MAX]; - char xml_out[PATH_MAX]; - int ret; + char *xml_in = NULL; + char *xml_out = NULL; + int ret = -1; - snprintf(xml_in, PATH_MAX, "%s/qemuxml2argvdata/qemuxml2argv-%s.xml", - abs_srcdir, info->name); - snprintf(xml_out, PATH_MAX, "%s/qemuxml2xmloutdata/qemuxml2xmlout-%s.xml", - abs_srcdir, info->name); + if (virAsprintf(&xml_in, "%s/qemuxml2argvdata/qemuxml2argv-%s.xml", + abs_srcdir, info->name) < 0 || + virAsprintf(&xml_out, "%s/qemuxml2xmloutdata/qemuxml2xmlout-%s.xml", + abs_srcdir, info->name) < 0) + goto cleanup; if (info->different) { ret = testCompareXMLToXMLFiles(xml_in, xml_out); @@ -78,6 +81,9 @@ static int testCompareXMLToXMLHelper(const void *data) { ret = testCompareXMLToXMLFiles(xml_in, xml_in); } +cleanup: + free(xml_in); + free(xml_out); return ret; } @@ -86,7 +92,6 @@ static int mymain(int argc, char **argv) { int ret = 0; - char cwd[PATH_MAX]; progname = argv[0]; diff --git a/tests/sexpr2xmltest.c b/tests/sexpr2xmltest.c index 9f74ece..4df0fd5 100644 --- a/tests/sexpr2xmltest.c +++ b/tests/sexpr2xmltest.c @@ -14,18 +14,16 @@ #include "testutilsxen.h" static char *progname; +static char cwd[PATH_MAX]; static char *abs_srcdir; static virCapsPtr caps; -#define MAX_FILE 4096 - -static int testCompareFiles(const char *xml, const char *sexpr, - int xendConfigVersion) { - char xmlData[MAX_FILE]; - char sexprData[MAX_FILE]; +static int +testCompareFiles(const char *xml, const char *sexpr, int xendConfigVersion) +{ + char *xmlData = NULL; + char *sexprData = NULL; char *gotxml = NULL; - char *xmlPtr = &(xmlData[0]); - char *sexprPtr = &(sexprData[0]); int id; char * tty; int vncport; @@ -38,10 +36,10 @@ static int testCompareFiles(const char *xml, const char *sexpr, conn = virGetConnect(); if (!conn) goto fail; - if (virtTestLoadFile(xml, &xmlPtr, MAX_FILE) < 0) + if (virtTestLoadFile(xml, &xmlData) < 0) goto fail; - if (virtTestLoadFile(sexpr, &sexprPtr, MAX_FILE) < 0) + if (virtTestLoadFile(sexpr, &sexprData) < 0) goto fail; memset(&priv, 0, sizeof priv); @@ -72,6 +70,8 @@ static int testCompareFiles(const char *xml, const char *sexpr, ret = 0; fail: + free(xmlData); + free(sexprData); free(gotxml); virDomainDefFree(def); if (conn) @@ -86,23 +86,34 @@ struct testInfo { int version; }; -static int testCompareHelper(const void *data) { +static int +testCompareHelper(const void *data) +{ + int result = -1; const struct testInfo *info = data; - char xml[PATH_MAX]; - char args[PATH_MAX]; - snprintf(xml, PATH_MAX, "%s/sexpr2xmldata/sexpr2xml-%s.xml", - abs_srcdir, info->input); - snprintf(args, PATH_MAX, "%s/sexpr2xmldata/sexpr2xml-%s.sexpr", - abs_srcdir, info->output); - return testCompareFiles(xml, args, info->version); -} + char *xml = NULL; + char *args = NULL; + + if (virAsprintf(&xml, "%s/sexpr2xmldata/sexpr2xml-%s.xml", + abs_srcdir, info->input) < 0 || + virAsprintf(&args, "%s/sexpr2xmldata/sexpr2xml-%s.sexpr", + abs_srcdir, info->output) < 0) { + goto cleanup; + } + + result = testCompareFiles(xml, args, info->version); +cleanup: + free(xml); + free(args); + + return result; +} static int mymain(int argc, char **argv) { int ret = 0; - char cwd[PATH_MAX]; progname = argv[0]; diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c index 2ec29e3..e947100 100644 --- a/tests/storagepoolxml2xmltest.c +++ b/tests/storagepoolxml2xmltest.c @@ -14,23 +14,21 @@ #include "testutilsqemu.h" static char *progname; +static char cwd[PATH_MAX]; static char *abs_srcdir; -#define MAX_FILE 4096 - - -static int testCompareXMLToXMLFiles(const char *inxml, const char *outxml) { - char inXmlData[MAX_FILE]; - char *inXmlPtr = &(inXmlData[0]); - char outXmlData[MAX_FILE]; - char *outXmlPtr = &(outXmlData[0]); +static int +testCompareXMLToXMLFiles(const char *inxml, const char *outxml) +{ + char *inXmlData = NULL; + char *outXmlData = NULL; char *actual = NULL; int ret = -1; virStoragePoolDefPtr dev = NULL; - if (virtTestLoadFile(inxml, &inXmlPtr, MAX_FILE) < 0) + if (virtTestLoadFile(inxml, &inXmlData) < 0) goto fail; - if (virtTestLoadFile(outxml, &outXmlPtr, MAX_FILE) < 0) + if (virtTestLoadFile(outxml, &outXmlData) < 0) goto fail; if (!(dev = virStoragePoolDefParseString(inXmlData))) @@ -47,27 +45,40 @@ static int testCompareXMLToXMLFiles(const char *inxml, const char *outxml) { ret = 0; fail: + free(inXmlData); + free(outXmlData); free(actual); virStoragePoolDefFree(dev); return ret; } -static int testCompareXMLToXMLHelper(const void *data) { - char inxml[PATH_MAX]; - char outxml[PATH_MAX]; - snprintf(inxml, PATH_MAX, "%s/storagepoolxml2xmlin/%s.xml", - abs_srcdir, (const char*)data); - snprintf(outxml, PATH_MAX, "%s/storagepoolxml2xmlout/%s.xml", - abs_srcdir, (const char*)data); - return testCompareXMLToXMLFiles(inxml, outxml); -} +static int +testCompareXMLToXMLHelper(const void *data) +{ + int result = -1; + char *inxml = NULL; + char *outxml = NULL; + + if (virAsprintf(&inxml, "%s/storagepoolxml2xmlin/%s.xml", + abs_srcdir, (const char*)data) < 0 || + virAsprintf(&outxml, "%s/storagepoolxml2xmlout/%s.xml", + abs_srcdir, (const char*)data) < 0) { + goto cleanup; + } + + result = testCompareXMLToXMLFiles(inxml, outxml); +cleanup: + free(inxml); + free(outxml); + + return result; +} static int mymain(int argc, char **argv) { int ret = 0; - char cwd[PATH_MAX]; progname = argv[0]; diff --git a/tests/storagevolxml2xmltest.c b/tests/storagevolxml2xmltest.c index ffa5dfe..c57b7a8 100644 --- a/tests/storagevolxml2xmltest.c +++ b/tests/storagevolxml2xmltest.c @@ -14,30 +14,26 @@ #include "testutilsqemu.h" static char *progname; +static char cwd[PATH_MAX]; static char *abs_srcdir; -#define MAX_FILE 4096 - - -static int testCompareXMLToXMLFiles(const char *poolxml, - const char *inxml, - const char *outxml) { - char poolXmlData[MAX_FILE]; - char *poolXmlPtr = &(poolXmlData[0]); - char inXmlData[MAX_FILE]; - char *inXmlPtr = &(inXmlData[0]); - char outXmlData[MAX_FILE]; - char *outXmlPtr = &(outXmlData[0]); +static int +testCompareXMLToXMLFiles(const char *poolxml, const char *inxml, + const char *outxml) +{ + char *poolXmlData = NULL; + char *inXmlData = NULL; + char *outXmlData = NULL; char *actual = NULL; int ret = -1; virStoragePoolDefPtr pool = NULL; virStorageVolDefPtr dev = NULL; - if (virtTestLoadFile(poolxml, &poolXmlPtr, MAX_FILE) < 0) + if (virtTestLoadFile(poolxml, &poolXmlData) < 0) goto fail; - if (virtTestLoadFile(inxml, &inXmlPtr, MAX_FILE) < 0) + if (virtTestLoadFile(inxml, &inXmlData) < 0) goto fail; - if (virtTestLoadFile(outxml, &outXmlPtr, MAX_FILE) < 0) + if (virtTestLoadFile(outxml, &outXmlData) < 0) goto fail; if (!(pool = virStoragePoolDefParseString(poolXmlData))) @@ -57,6 +53,9 @@ static int testCompareXMLToXMLFiles(const char *poolxml, ret = 0; fail: + free(poolXmlData); + free(inXmlData); + free(outXmlData); free(actual); virStoragePoolDefFree(pool); virStorageVolDefFree(dev); @@ -68,19 +67,32 @@ struct testInfo { const char *name; }; -static int testCompareXMLToXMLHelper(const void *data) { - char poolxml[PATH_MAX]; - char inxml[PATH_MAX]; - char outxml[PATH_MAX]; +static int +testCompareXMLToXMLHelper(const void *data) +{ + int result = -1; const struct testInfo *info = data; + char *poolxml = NULL; + char *inxml = NULL; + char *outxml = NULL; + + if (virAsprintf(&poolxml, "%s/storagepoolxml2xmlin/%s.xml", + abs_srcdir, info->pool) < 0 || + virAsprintf(&inxml, "%s/storagevolxml2xmlin/%s.xml", + abs_srcdir, info->name) < 0 || + virAsprintf(&outxml, "%s/storagevolxml2xmlout/%s.xml", + abs_srcdir, info->name) < 0) { + goto cleanup; + } + + result = testCompareXMLToXMLFiles(poolxml, inxml, outxml); + +cleanup: + free(poolxml); + free(inxml); + free(outxml); - snprintf(poolxml, PATH_MAX, "%s/storagepoolxml2xmlin/%s.xml", - abs_srcdir, (const char*)info->pool); - snprintf(inxml, PATH_MAX, "%s/storagevolxml2xmlin/%s.xml", - abs_srcdir, (const char*)info->name); - snprintf(outxml, PATH_MAX, "%s/storagevolxml2xmlout/%s.xml", - abs_srcdir, (const char*)info->name); - return testCompareXMLToXMLFiles(poolxml, inxml, outxml); + return result; } @@ -88,7 +100,6 @@ static int mymain(int argc, char **argv) { int ret = 0; - char cwd[PATH_MAX]; progname = argv[0]; diff --git a/tests/testutils.c b/tests/testutils.c index 1f3b569..aaee337 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -170,17 +170,16 @@ virtTestRun(const char *title, int nloops, int (*body)(const void *data), const return ret; } -/* Read FILE into buffer BUF of length BUFLEN. - Upon any failure, or if FILE appears to contain more than BUFLEN bytes, - diagnose it and return -1, but don't bother trying to preserve errno. - Otherwise, return the number of bytes copied into BUF. */ -int virtTestLoadFile(const char *file, - char **buf, - int buflen) { +/* Allocate BUF to the size of FILE. Read FILE into buffer BUF. + Upon any failure, diagnose it and return -1, but don't bother trying + to preserve errno. Otherwise, return the number of bytes copied into BUF. */ +int +virtTestLoadFile(const char *file, char **buf) +{ FILE *fp = fopen(file, "r"); struct stat st; - char *tmp = *buf; - int len, tmplen = buflen; + char *tmp; + int len, tmplen, buflen; if (!fp) { fprintf (stderr, "%s: failed to open: %s\n", file, strerror(errno)); @@ -193,17 +192,23 @@ int virtTestLoadFile(const char *file, return -1; } - if (st.st_size > (buflen-1)) { - fprintf (stderr, "%s: larger than buffer (> %d)\n", file, buflen-1); + tmplen = buflen = st.st_size + 1; + + if (VIR_ALLOC_N(*buf, buflen) < 0) { + fprintf (stderr, "%s: larger than available memory (> %d)\n", file, buflen); VIR_FORCE_FCLOSE(fp); return -1; } + tmp = *buf; (*buf)[0] = '\0'; if (st.st_size) { /* read the file line by line */ while (fgets(tmp, tmplen, fp) != NULL) { len = strlen(tmp); + /* stop on an empty line */ + if (len == 0) + break; /* remove trailing backslash-newline pair */ if (len >= 2 && tmp[len-2] == '\\' && tmp[len-1] == '\n') { len -= 2; @@ -216,6 +221,7 @@ int virtTestLoadFile(const char *file, if (ferror(fp)) { fprintf (stderr, "%s: read failed: %s\n", file, strerror(errno)); VIR_FORCE_FCLOSE(fp); + free(*buf); return -1; } } @@ -265,10 +271,11 @@ void virtTestCaptureProgramExecChild(const char *const argv[], VIR_FORCE_CLOSE(stdinfd); } -int virtTestCaptureProgramOutput(const char *const argv[], - char **buf, - int buflen) { +int +virtTestCaptureProgramOutput(const char *const argv[], char **buf, int maxlen) +{ int pipefd[2]; + int len; if (pipe(pipefd) < 0) return -1; @@ -286,34 +293,20 @@ int virtTestCaptureProgramOutput(const char *const argv[], return -1; default: - { - int got = 0; - int ret = -1; - int want = buflen-1; - - VIR_FORCE_CLOSE(pipefd[1]); - - while (want) { - if ((ret = read(pipefd[0], (*buf)+got, want)) <= 0) - break; - got += ret; - want -= ret; - } - VIR_FORCE_CLOSE(pipefd[0]); - - if (!ret) - (*buf)[got] = '\0'; - - waitpid(pid, NULL, 0); + VIR_FORCE_CLOSE(pipefd[1]); + len = virFileReadLimFD(pipefd[0], maxlen, buf); + VIR_FORCE_CLOSE(pipefd[0]); + waitpid(pid, NULL, 0); - return ret; - } + return len; } } #else /* !WIN32 */ -int virtTestCaptureProgramOutput(const char *const argv[] ATTRIBUTE_UNUSED, - char **buf ATTRIBUTE_UNUSED, - int buflen ATTRIBUTE_UNUSED) { +int +virtTestCaptureProgramOutput(const char *const argv[] ATTRIBUTE_UNUSED, + char **buf ATTRIBUTE_UNUSED, + int maxlen ATTRIBUTE_UNUSED) +{ return -1; } #endif /* !WIN32 */ diff --git a/tests/testutils.h b/tests/testutils.h index 88603a1..6adc0a0 100644 --- a/tests/testutils.h +++ b/tests/testutils.h @@ -23,12 +23,8 @@ int virtTestRun(const char *title, int nloops, int (*body)(const void *data), const void *data); -int virtTestLoadFile(const char *name, - char **buf, - int buflen); -int virtTestCaptureProgramOutput(const char *const argv[], - char **buf, - int buflen); +int virtTestLoadFile(const char *file, char **buf); +int virtTestCaptureProgramOutput(const char *const argv[], char **buf, int maxlen); int virtTestClearLineRegex(const char *pattern, char *string); diff --git a/tests/virshtest.c b/tests/virshtest.c index 8ec97a8..e6743d7 100644 --- a/tests/virshtest.c +++ b/tests/virshtest.c @@ -6,11 +6,12 @@ #include "internal.h" #include "xml.h" +#include "util.h" #include "testutils.h" static char *progname; +static char cwd[PATH_MAX]; static char *abs_srcdir; -#define MAX_FILE 4096 #define DOM_UUID "ef861801-45b9-11cb-88e3-afbfe5370493" @@ -47,41 +48,31 @@ static int testFilterLine(char *buffer, return 0; } -static int testCompareOutputLit(const char *expectData, - const char *filter, const char *const argv[]) { - char actualData[MAX_FILE]; - char *actualPtr = &(actualData[0]); - - if (virtTestCaptureProgramOutput(argv, &actualPtr, MAX_FILE) < 0) - return -1; - - if (filter) - if (testFilterLine(actualData, filter) < 0) - return -1; +static int +testCompareOutputLit(const char *expectData, + const char *filter, const char *const argv[]) +{ + int result = -1; + char *actualData = NULL; - if (STRNEQ(expectData, actualData)) { - virtTestDifference(stderr, expectData, actualData); - return -1; - } + if (virtTestCaptureProgramOutput(argv, &actualData, 4096) < 0) + goto cleanup; - return 0; -} + if (filter && testFilterLine(actualData, filter) < 0) + goto cleanup; -#if unused -static int testCompareOutput(const char *expect_rel, const char *filter, - const char *const argv[]) { - char expectData[MAX_FILE]; - char *expectPtr = &(expectData[0]); - char expect[PATH_MAX]; + if (STRNEQ(expectData, actualData)) { + virtTestDifference(stderr, expectData, actualData); + goto cleanup; + } - snprintf(expect, sizeof expect - 1, "%s/%s", abs_srcdir, expect_rel); + result = 0; - if (virtTestLoadFile(expect, &expectPtr, MAX_FILE) < 0) - return -1; +cleanup: + free(actualData); - return testCompareOutputLit(expectData, filter, argv); + return result; } -#endif #define VIRSH_DEFAULT "../tools/virsh", \ "--connect", \ @@ -235,8 +226,6 @@ static int mymain(int argc, char **argv) { int ret = 0; - char buffer[PATH_MAX]; - char cwd[PATH_MAX]; abs_srcdir = getenv("abs_srcdir"); if (!abs_srcdir) @@ -246,16 +235,17 @@ mymain(int argc, char **argv) exit (EXIT_AM_SKIP); #endif - snprintf(buffer, PATH_MAX-1, "test://%s/../examples/xml/test/testnode.xml", abs_srcdir); - buffer[PATH_MAX-1] = '\0'; - progname = argv[0]; - custom_uri = buffer; - if (argc > 1) { fprintf(stderr, "Usage: %s\n", progname); - return(EXIT_FAILURE); + return EXIT_FAILURE; } + if (virAsprintf(&custom_uri, "test://%s/../examples/xml/test/testnode.xml", + abs_srcdir) < 0) + return EXIT_FAILURE; + + progname = argv[0]; + if (virtTestRun("virsh list (default)", 1, testCompareListDefault, NULL) != 0) ret = -1; @@ -406,6 +396,7 @@ mymain(int argc, char **argv) #undef DO_TEST + free(custom_uri); return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); } diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c index 5fed1c4..53d66c5 100644 --- a/tests/vmx2xmltest.c +++ b/tests/vmx2xmltest.c @@ -11,13 +11,12 @@ # include "testutils.h" # include "vmx/vmx.h" -static char *progname = NULL; -static char *abs_srcdir = NULL; -static virCapsPtr caps = NULL; +static char *progname; +static char cwd[PATH_MAX]; +static char *abs_srcdir; +static virCapsPtr caps; static virVMXContext ctx; -# define MAX_FILE 4096 - static void testCapsInit(void) { @@ -71,19 +70,17 @@ static int testCompareFiles(const char *vmx, const char *xml) { int result = -1; - char vmxData[MAX_FILE]; - char xmlData[MAX_FILE]; + char *vmxData = NULL; + char *xmlData = NULL; char *formatted = NULL; - char *vmxPtr = &(vmxData[0]); - char *xmlPtr = &(xmlData[0]); virDomainDefPtr def = NULL; virErrorPtr err = NULL; - if (virtTestLoadFile(vmx, &vmxPtr, MAX_FILE) < 0) { + if (virtTestLoadFile(vmx, &vmxData) < 0) { goto failure; } - if (virtTestLoadFile(xml, &xmlPtr, MAX_FILE) < 0) { + if (virtTestLoadFile(xml, &xmlData) < 0) { goto failure; } @@ -111,6 +108,8 @@ testCompareFiles(const char *vmx, const char *xml) result = 0; failure: + VIR_FREE(vmxData); + VIR_FREE(xmlData); VIR_FREE(formatted); virDomainDefFree(def); @@ -125,16 +124,25 @@ struct testInfo { static int testCompareHelper(const void *data) { + int result = -1; const struct testInfo *info = data; - char vmx[PATH_MAX]; - char xml[PATH_MAX]; + char *vmx = NULL; + char *xml = NULL; + + if (virAsprintf(&vmx, "%s/vmx2xmldata/vmx2xml-%s.vmx", abs_srcdir, + info->input) < 0 || + virAsprintf(&xml, "%s/vmx2xmldata/vmx2xml-%s.xml", abs_srcdir, + info->output) < 0) { + goto cleanup; + } - snprintf(vmx, PATH_MAX, "%s/vmx2xmldata/vmx2xml-%s.vmx", abs_srcdir, - info->input); - snprintf(xml, PATH_MAX, "%s/vmx2xmldata/vmx2xml-%s.xml", abs_srcdir, - info->output); + result = testCompareFiles(vmx, xml); - return testCompareFiles(vmx, xml); + cleanup: + VIR_FREE(vmx); + VIR_FREE(xml); + + return result; } static char * @@ -184,7 +192,6 @@ static int mymain(int argc, char **argv) { int result = 0; - char cwd[PATH_MAX]; progname = argv[0]; diff --git a/tests/xencapstest.c b/tests/xencapstest.c index 7b48812..a00b30b 100644 --- a/tests/xencapstest.c +++ b/tests/xencapstest.c @@ -12,34 +12,30 @@ #include "files.h" static char *progname; +static char cwd[PATH_MAX]; static char *abs_srcdir; -#define MAX_FILE 4096 - -static int testCompareFiles(const char *hostmachine, - const char *xml_rel, - const char *cpuinfo_rel, - const char *capabilities_rel) { - char xmlData[MAX_FILE]; - char *expectxml = &(xmlData[0]); +static int +testCompareFiles(const char *hostmachine, const char *xml_rel, + const char *cpuinfo_rel, const char *capabilities_rel) +{ + char *expectxml = NULL; char *actualxml = NULL; FILE *fp1 = NULL, *fp2 = NULL; virCapsPtr caps = NULL; int ret = -1; - char xml[PATH_MAX]; - char cpuinfo[PATH_MAX]; - char capabilities[PATH_MAX]; + char *xml = NULL; + char *cpuinfo = NULL; + char *capabilities = NULL; - snprintf(xml, sizeof xml - 1, "%s/%s", - abs_srcdir, xml_rel); - snprintf(cpuinfo, sizeof cpuinfo - 1, "%s/%s", - abs_srcdir, cpuinfo_rel); - snprintf(capabilities, sizeof capabilities - 1, "%s/%s", - abs_srcdir, capabilities_rel); + if (virAsprintf(&xml, "%s/%s", abs_srcdir, xml_rel) < 0 || + virAsprintf(&cpuinfo, "%s/%s", abs_srcdir, cpuinfo_rel) < 0 || + virAsprintf(&capabilities, "%s/%s", abs_srcdir, capabilities_rel) < 0) + goto fail; - if (virtTestLoadFile(xml, &expectxml, MAX_FILE) < 0) + if (virtTestLoadFile(xml, &expectxml) < 0) goto fail; if (!(fp1 = fopen(cpuinfo, "r"))) @@ -62,8 +58,11 @@ static int testCompareFiles(const char *hostmachine, ret = 0; fail: - + free(expectxml); free(actualxml); + free(xml); + free(cpuinfo); + free(capabilities); VIR_FORCE_FCLOSE(fp1); VIR_FORCE_FCLOSE(fp2); @@ -154,7 +153,6 @@ static int mymain(int argc, char **argv) { int ret = 0; - char cwd[PATH_MAX]; progname = argv[0]; diff --git a/tests/xmconfigtest.c b/tests/xmconfigtest.c index 7d418a5..41ea0c8 100644 --- a/tests/xmconfigtest.c +++ b/tests/xmconfigtest.c @@ -37,33 +37,33 @@ #include "memory.h" static char *progname; +static char cwd[PATH_MAX]; static char *abs_srcdir; static virCapsPtr caps; -#define MAX_FILE 4096 - -static int testCompareParseXML(const char *xmcfg, const char *xml, - int xendConfigVersion) { - char xmlData[MAX_FILE]; - char xmcfgData[MAX_FILE]; - char gotxmcfgData[MAX_FILE]; - char *xmlPtr = &(xmlData[0]); - char *xmcfgPtr = &(xmcfgData[0]); - char *gotxmcfgPtr = &(gotxmcfgData[0]); +static int +testCompareParseXML(const char *xmcfg, const char *xml, int xendConfigVersion) +{ + char *xmlData = NULL; + char *xmcfgData = NULL; + char *gotxmcfgData = NULL; virConfPtr conf = NULL; int ret = -1; virConnectPtr conn; - int wrote = MAX_FILE; + int wrote = 4096; struct _xenUnifiedPrivate priv; virDomainDefPtr def = NULL; + if (VIR_ALLOC_N(gotxmcfgData, wrote) < 0) + goto fail; + conn = virGetConnect(); if (!conn) goto fail; - if (virtTestLoadFile(xml, &xmlPtr, MAX_FILE) < 0) + if (virtTestLoadFile(xml, &xmlData) < 0) goto fail; - if (virtTestLoadFile(xmcfg, &xmcfgPtr, MAX_FILE) < 0) + if (virtTestLoadFile(xmcfg, &xmcfgData) < 0) goto fail; /* Many puppies died to bring you this code. */ @@ -71,16 +71,16 @@ static int testCompareParseXML(const char *xmcfg, const char *xml, priv.caps = caps; conn->privateData = &priv; - if (!(def = virDomainDefParseString(caps, xmlPtr, + if (!(def = virDomainDefParseString(caps, xmlData, VIR_DOMAIN_XML_INACTIVE))) goto fail; if (!(conf = xenFormatXM(conn, def, xendConfigVersion))) goto fail; - if (virConfWriteMem(gotxmcfgPtr, &wrote, conf) < 0) + if (virConfWriteMem(gotxmcfgData, &wrote, conf) < 0) goto fail; - gotxmcfgPtr[wrote] = '\0'; + gotxmcfgData[wrote] = '\0'; if (STRNEQ(xmcfgData, gotxmcfgData)) { virtTestDifference(stderr, xmcfgData, gotxmcfgData); @@ -90,6 +90,9 @@ static int testCompareParseXML(const char *xmcfg, const char *xml, ret = 0; fail: + free(xmlData); + free(xmcfgData); + free(gotxmcfgData); if (conf) virConfFree(conf); virDomainDefFree(def); @@ -98,12 +101,11 @@ static int testCompareParseXML(const char *xmcfg, const char *xml, return ret; } -static int testCompareFormatXML(const char *xmcfg, const char *xml, - int xendConfigVersion) { - char xmlData[MAX_FILE]; - char xmcfgData[MAX_FILE]; - char *xmlPtr = &(xmlData[0]); - char *xmcfgPtr = &(xmcfgData[0]); +static int +testCompareFormatXML(const char *xmcfg, const char *xml, int xendConfigVersion) +{ + char *xmlData = NULL; + char *xmcfgData = NULL; char *gotxml = NULL; virConfPtr conf = NULL; int ret = -1; @@ -114,10 +116,10 @@ static int testCompareFormatXML(const char *xmcfg, const char *xml, conn = virGetConnect(); if (!conn) goto fail; - if (virtTestLoadFile(xml, &xmlPtr, MAX_FILE) < 0) + if (virtTestLoadFile(xml, &xmlData) < 0) goto fail; - if (virtTestLoadFile(xmcfg, &xmcfgPtr, MAX_FILE) < 0) + if (virtTestLoadFile(xmcfg, &xmcfgData) < 0) goto fail; /* Many puppies died to bring you this code. */ @@ -125,7 +127,7 @@ static int testCompareFormatXML(const char *xmcfg, const char *xml, priv.caps = caps; conn->privateData = &priv; - if (!(conf = virConfReadMem(xmcfgPtr, strlen(xmcfgPtr), 0))) + if (!(conf = virConfReadMem(xmcfgData, strlen(xmcfgData), 0))) goto fail; if (!(def = xenParseXM(conf, priv.xendConfigVersion, priv.caps))) @@ -144,6 +146,8 @@ static int testCompareFormatXML(const char *xmcfg, const char *xml, fail: if (conf) virConfFree(conf); + VIR_FREE(xmlData); + VIR_FREE(xmcfgData); VIR_FREE(gotxml); virDomainDefFree(def); virUnrefConnect(conn); @@ -158,18 +162,30 @@ struct testInfo { int mode; }; -static int testCompareHelper(const void *data) { +static int +testCompareHelper(const void *data) +{ + int result = -1; const struct testInfo *info = data; - char xml[PATH_MAX]; - char cfg[PATH_MAX]; - snprintf(xml, PATH_MAX, "%s/xmconfigdata/test-%s.xml", - abs_srcdir, info->name); - snprintf(cfg, PATH_MAX, "%s/xmconfigdata/test-%s.cfg", - abs_srcdir, info->name); + char *xml = NULL; + char *cfg = NULL; + + if (virAsprintf(&xml, "%s/xmconfigdata/test-%s.xml", + abs_srcdir, info->name) < 0 || + virAsprintf(&cfg, "%s/xmconfigdata/test-%s.cfg", + abs_srcdir, info->name) < 0) + goto cleanup; + if (info->mode == 0) - return testCompareParseXML(cfg, xml, info->version); + result = testCompareParseXML(cfg, xml, info->version); else - return testCompareFormatXML(cfg, xml, info->version); + result = testCompareFormatXML(cfg, xml, info->version); + +cleanup: + free(xml); + free(cfg); + + return result; } @@ -177,7 +193,6 @@ static int mymain(int argc, char **argv) { int ret = 0; - char cwd[PATH_MAX]; progname = argv[0]; diff --git a/tests/xml2sexprtest.c b/tests/xml2sexprtest.c index cf530b6..b6d8c00 100644 --- a/tests/xml2sexprtest.c +++ b/tests/xml2sexprtest.c @@ -15,25 +15,23 @@ #include "testutilsxen.h" static char *progname; +static char cwd[PATH_MAX]; static char *abs_srcdir; static virCapsPtr caps; -#define MAX_FILE 4096 - -static int testCompareFiles(const char *xml, const char *sexpr, - int xendConfigVersion) { - char xmlData[MAX_FILE]; - char sexprData[MAX_FILE]; +static int +testCompareFiles(const char *xml, const char *sexpr, int xendConfigVersion) +{ + char *xmlData = NULL; + char *sexprData = NULL; char *gotsexpr = NULL; - char *xmlPtr = &(xmlData[0]); - char *sexprPtr = &(sexprData[0]); int ret = -1; virDomainDefPtr def = NULL; - if (virtTestLoadFile(xml, &xmlPtr, MAX_FILE) < 0) + if (virtTestLoadFile(xml, &xmlData) < 0) goto fail; - if (virtTestLoadFile(sexpr, &sexprPtr, MAX_FILE) < 0) + if (virtTestLoadFile(sexpr, &sexprData) < 0) goto fail; if (!(def = virDomainDefParseString(caps, xmlData, @@ -51,8 +49,10 @@ static int testCompareFiles(const char *xml, const char *sexpr, ret = 0; fail: - virDomainDefFree(def); + free(xmlData); + free(sexprData); free(gotsexpr); + virDomainDefFree(def); return ret; } @@ -64,23 +64,34 @@ struct testInfo { int version; }; -static int testCompareHelper(const void *data) { +static int +testCompareHelper(const void *data) +{ + int result = -1; const struct testInfo *info = data; - char xml[PATH_MAX]; - char args[PATH_MAX]; - snprintf(xml, PATH_MAX, "%s/xml2sexprdata/xml2sexpr-%s.xml", - abs_srcdir, info->input); - snprintf(args, PATH_MAX, "%s/xml2sexprdata/xml2sexpr-%s.sexpr", - abs_srcdir, info->output); - return testCompareFiles(xml, args, info->version); -} + char *xml = NULL; + char *args = NULL; + + if (virAsprintf(&xml, "%s/xml2sexprdata/xml2sexpr-%s.xml", + abs_srcdir, info->input) < 0 || + virAsprintf(&args, "%s/xml2sexprdata/xml2sexpr-%s.sexpr", + abs_srcdir, info->output) < 0) { + goto cleanup; + } + result = testCompareFiles(xml, args, info->version); + +cleanup: + free(xml); + free(args); + + return result; +} static int mymain(int argc, char **argv) { int ret = 0; - char cwd[PATH_MAX]; progname = argv[0]; diff --git a/tests/xml2vmxtest.c b/tests/xml2vmxtest.c index a5fb4f7..1b58db8 100644 --- a/tests/xml2vmxtest.c +++ b/tests/xml2vmxtest.c @@ -11,13 +11,12 @@ # include "testutils.h" # include "vmx/vmx.h" -static char *progname = NULL; -static char *abs_srcdir = NULL; -static virCapsPtr caps = NULL; +static char *progname; +static char cwd[PATH_MAX]; +static char *abs_srcdir; +static virCapsPtr caps; static virVMXContext ctx; -# define MAX_FILE 4096 - static void testCapsInit(void) { @@ -71,18 +70,16 @@ static int testCompareFiles(const char *xml, const char *vmx, int virtualHW_version) { int result = -1; - char xmlData[MAX_FILE]; - char vmxData[MAX_FILE]; + char *xmlData = NULL; + char *vmxData = NULL; char *formatted = NULL; - char *xmlPtr = &(xmlData[0]); - char *vmxPtr = &(vmxData[0]); virDomainDefPtr def = NULL; - if (virtTestLoadFile(xml, &xmlPtr, MAX_FILE) < 0) { + if (virtTestLoadFile(xml, &xmlData) < 0) { goto failure; } - if (virtTestLoadFile(vmx, &vmxPtr, MAX_FILE) < 0) { + if (virtTestLoadFile(vmx, &vmxData) < 0) { goto failure; } @@ -106,6 +103,8 @@ testCompareFiles(const char *xml, const char *vmx, int virtualHW_version) result = 0; failure: + VIR_FREE(xmlData); + VIR_FREE(vmxData); VIR_FREE(formatted); virDomainDefFree(def); @@ -121,16 +120,25 @@ struct testInfo { static int testCompareHelper(const void *data) { + int result = -1; const struct testInfo *info = data; - char xml[PATH_MAX]; - char vmx[PATH_MAX]; + char *xml = NULL; + char *vmx = NULL; + + if (virAsprintf(&xml, "%s/xml2vmxdata/xml2vmx-%s.xml", abs_srcdir, + info->input) < 0 || + virAsprintf(&vmx, "%s/xml2vmxdata/xml2vmx-%s.vmx", abs_srcdir, + info->output) < 0) { + goto cleanup; + } + + result = testCompareFiles(xml, vmx, info->virtualHW_version); - snprintf(xml, PATH_MAX, "%s/xml2vmxdata/xml2vmx-%s.xml", abs_srcdir, - info->input); - snprintf(vmx, PATH_MAX, "%s/xml2vmxdata/xml2vmx-%s.vmx", abs_srcdir, - info->output); + cleanup: + VIR_FREE(xml); + VIR_FREE(vmx); - return testCompareFiles(xml, vmx, info->virtualHW_version); + return result; } static int @@ -201,7 +209,6 @@ static int mymain(int argc, char **argv) { int result = 0; - char cwd[PATH_MAX]; progname = argv[0]; -- 1.7.0.4
On 04/24/2011 04:26 PM, Matthias Bolte wrote:
Make virtTestLoadFile allocate the buffer to read the file into.
Fix logic error in virtTestLoadFile, stop reading on the an empty line.
Use virFileReadLimFD in virtTestCaptureProgramOutput. --- +++ b/tests/commandhelper.c @@ -99,8 +99,8 @@ int main(int argc, char **argv) { }
fprintf(log, "DAEMON:%s\n", getpgrp() == getsid(0) ? "yes" : "no"); - char cwd[1024]; - if (!getcwd(cwd, sizeof(cwd))) + char *cwd = NULL; + if (!(cwd = getcwd(NULL, 0)))
Ouch. This is not portable to POSIX, and while gnulib can guarantee that it works, the current gnulib getcwd module is GPL (and relies on openat, which is a rather heavy-weight replacement!). I'm going to work on a gnulib module getcwd-lgpl which doesn't fix all the known bugs in getcwd, but at least guarantees that getcwd(NULL,0) will malloc insofar as the underlying getcwd is not buggy; we'll need to import that into libvirt before applying the rest of this patch. I haven't closely reviewed the rest of this patch yet, but like the general idea once we have getcwd sorted out. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2011/4/25 Eric Blake <eblake@redhat.com>:
On 04/24/2011 04:26 PM, Matthias Bolte wrote:
Make virtTestLoadFile allocate the buffer to read the file into.
Fix logic error in virtTestLoadFile, stop reading on the an empty line.
Use virFileReadLimFD in virtTestCaptureProgramOutput. --- +++ b/tests/commandhelper.c @@ -99,8 +99,8 @@ int main(int argc, char **argv) { }
fprintf(log, "DAEMON:%s\n", getpgrp() == getsid(0) ? "yes" : "no"); - char cwd[1024]; - if (!getcwd(cwd, sizeof(cwd))) + char *cwd = NULL; + if (!(cwd = getcwd(NULL, 0)))
Ouch. This is not portable to POSIX, and while gnulib can guarantee that it works, the current gnulib getcwd module is GPL (and relies on openat, which is a rather heavy-weight replacement!).
I'm going to work on a gnulib module getcwd-lgpl which doesn't fix all the known bugs in getcwd, but at least guarantees that getcwd(NULL,0) will malloc insofar as the underlying getcwd is not buggy; we'll need to import that into libvirt before applying the rest of this patch.
I haven't closely reviewed the rest of this patch yet, but like the general idea once we have getcwd sorted out.
Oops. At first I used getcwd(NULL, 0) to replace the getcwd(cwd, sizeof(cwd)) calls in the main functions, but then decided to just move the cwd buffer to global scope instead. I just missed this one in the second rewrite round. Matthias
2011/4/25 Matthias Bolte <matthias.bolte@googlemail.com>:
2011/4/25 Eric Blake <eblake@redhat.com>:
On 04/24/2011 04:26 PM, Matthias Bolte wrote:
Make virtTestLoadFile allocate the buffer to read the file into.
Fix logic error in virtTestLoadFile, stop reading on the an empty line.
Use virFileReadLimFD in virtTestCaptureProgramOutput. --- +++ b/tests/commandhelper.c @@ -99,8 +99,8 @@ int main(int argc, char **argv) { }
fprintf(log, "DAEMON:%s\n", getpgrp() == getsid(0) ? "yes" : "no"); - char cwd[1024]; - if (!getcwd(cwd, sizeof(cwd))) + char *cwd = NULL; + if (!(cwd = getcwd(NULL, 0)))
Ouch. This is not portable to POSIX, and while gnulib can guarantee that it works, the current gnulib getcwd module is GPL (and relies on openat, which is a rather heavy-weight replacement!).
I'm going to work on a gnulib module getcwd-lgpl which doesn't fix all the known bugs in getcwd, but at least guarantees that getcwd(NULL,0) will malloc insofar as the underlying getcwd is not buggy; we'll need to import that into libvirt before applying the rest of this patch.
I haven't closely reviewed the rest of this patch yet, but like the general idea once we have getcwd sorted out.
Oops. At first I used getcwd(NULL, 0) to replace the getcwd(cwd, sizeof(cwd)) calls in the main functions, but then decided to just move the cwd buffer to global scope instead. I just missed this one in the second rewrite round.
Matthias
Grep'ing the rest of the codebase shows a getcwd(NULL, 0) call in virFileAbsPath that probably needs care too. Matthias
On 04/25/2011 10:57 AM, Eric Blake wrote:
On 04/24/2011 04:26 PM, Matthias Bolte wrote:
Make virtTestLoadFile allocate the buffer to read the file into.
Fix logic error in virtTestLoadFile, stop reading on the an empty line.
Use virFileReadLimFD in virtTestCaptureProgramOutput. --- +++ b/tests/commandhelper.c @@ -99,8 +99,8 @@ int main(int argc, char **argv) { }
fprintf(log, "DAEMON:%s\n", getpgrp() == getsid(0) ? "yes" : "no"); - char cwd[1024]; - if (!getcwd(cwd, sizeof(cwd))) + char *cwd = NULL; + if (!(cwd = getcwd(NULL, 0)))
Ouch. This is not portable to POSIX, and while gnulib can guarantee that it works, the current gnulib getcwd module is GPL (and relies on openat, which is a rather heavy-weight replacement!).
I realize my statement may have come across as rather cryptic, so here's some more details: POSIX states that getcwd(NULL, n) has unspecified behavior. On most modern systems, it malloc's n bytes, or if n is 0, the perfect size for the answer; but older Solaris would fail with EINVAL, and it is possible that there were other old OS that failed with a core dump. POSIX also states that getcwd(buf, n) must fail with ERANGE if buf was too small, so that you can manage the malloc()s yourself and iteratively try larger buffers until you succeed (or, less likely, fail for some other reason like EACCES); at least tools/virsh.c was already using this idiom. Meanwhile, modern Linux mishandles getcwd() for large directories. The kernel refuses to return the current working directory if it is larger than a page (only possible for super-deep hierarchies, but Jim Meyering is fond of creating those as stress-tests for coreutils and gnulib). /proc/self/cwd might fare better, but is probably another instance of the kernel being likely to give up if the absolute name gets too long. glibc has fallbacks in place for when the syscall fails, which involve readdir() over progressively longer ../../ chains to learn the name of each parent directory, and by using openat() it is possible to still do this in linear instead of O(n^2) time without having to use chdir(). However, these fallbacks can fail if any directory in the middle has permissions issues, such as no search permissions. The end result: portable code must _always_ be prepared for getcwd() to fail (and not just due to ENOMEM). And while it is always possible to manage malloc() yourself, that gets to be tedious, so it would be nice to make gnulib guarantee that getcwd(NULL,0) manages malloc(). The current gnulib module for getcwd is the least likely to fail - it takes the best of all worlds (syscall, /proc/self/cwd, and openat() ../../ traversal) into some pretty complex code that has very few failure cases (it can succeed in some places where glibc does not); however, that complexity comes with some pretty heavy weight (the gnulib openat() module can cause a call to exit(), so it is not library-safe, and is therefore only GPL code and can't be used in LGPL libvirt). So I'm working on adding a new gnulib module getcwd-lgpl, which guarantees the allocation for getcwd(NULL,0), but doesn't address the possible problems with super-deep hierarchies. That is, the version is more likely to fail than the robust version used in GNU coreutils' pwd, but those failures are in corner cases unlikely to happen (no one reinstalls libvirtd into a super-deep directory) or where we can safely pass failure back to the caller (it's now up to the user to bypass their super-deep hierarchy in some other manner, since libvirt won't do it), which seems like a reasonable trade-off. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 04/25/2011 10:57 AM, Eric Blake wrote:
On 04/24/2011 04:26 PM, Matthias Bolte wrote:
Make virtTestLoadFile allocate the buffer to read the file into.
Fix logic error in virtTestLoadFile, stop reading on the an empty line.
Use virFileReadLimFD in virtTestCaptureProgramOutput. --- +++ b/tests/commandhelper.c @@ -99,8 +99,8 @@ int main(int argc, char **argv) { }
fprintf(log, "DAEMON:%s\n", getpgrp() == getsid(0) ? "yes" : "no"); - char cwd[1024]; - if (!getcwd(cwd, sizeof(cwd))) + char *cwd = NULL; + if (!(cwd = getcwd(NULL, 0)))
Ouch. This is not portable to POSIX, and while gnulib can guarantee that it works, the current gnulib getcwd module is GPL (and relies on openat, which is a rather heavy-weight replacement!).
I'm going to work on a gnulib module getcwd-lgpl which doesn't fix all the known bugs in getcwd, but at least guarantees that getcwd(NULL,0) will malloc insofar as the underlying getcwd is not buggy; we'll need to import that into libvirt before applying the rest of this patch.
I've now done that. Should we review my patches first, then rebase this on top of my cleanups? If nothing else, it makes the cleanup in the tests/ directory much smaller. https://www.redhat.com/archives/libvir-list/2011-April/msg01255.html -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Matthias Bolte