[PATCH 0/9] configurator: Support for Windows and MSVC

[PATCH 0/9] configurator: Support for Windows and MSVC

[Kevin Locke]

Hi all,

This patch series adds support for building configurator with Microsoft
Visual C++ and running it on Windows.  The motivation is to add
sufficient support for Windows to allow using header-only modules
with minimal hassle.  For me, this patch series is sufficient, so I
don't have any further plans for changes to other files or the build
system at the moment.

The two patches that are particularly likely to raise objections are
patch 3 and 8:

Patch 3 adds a copy of the err.h functions used by configurator to work
around the lack of err.h on Windows.  If this is too much code to lump
into configurator.c, I can split it out (and copy err.{c,h} verbatim
from musl) or call fprintf+exit directly.

Patch 8 adds the output cflag (-o) to DEFAULT_CFLAGS and the expected
configurator cflags argument.  This changes both the configurator API
and the config.h API and will break existing code which depend on these.
If that is unacceptable, I'd be open to implementing a
backwards-compatible alternative.  One idea would be to add an
--output-cflag option to configurator, although it will require quite a
bit of argument parsing code.  I'm open to other alternatives as well.

Thanks for considering,
Kevin

Kevin Locke (9):
  configurator: Replace unlink with remove
  configurator: Reimplement run using popen
  configurator: Inline err.h functions from musl libc
  configurator: Use native directory separator
  configurator: Mark non-Windows tests MAY_NOT_COMPILE
  configurator: Print test source without cat
  configurator: Fix compiler warning with compare
  configurator: Pass output cflag to configurator
  configurator: DEFAULT_{COMPILER,FLAGS} for MSVC

 Makefile                          |   2 +-
 tools/configurator/configurator.c | 197 +++++++++++++++++++++++++-------------
 2 files changed, 131 insertions(+), 68 deletions(-)

-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH 1/9] configurator: Replace unlink with remove

[Kevin Locke]

Although Windows provides unlink, using it adds complication due to the
lack of unistd.h which must be included to define the function on POSIX
systems.  Instead, use remove, which C89 requires to be in stdio.h.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/configurator/configurator.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index 67dadc6..e322c76 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -639,8 +639,8 @@ int main(int argc, const char *argv[])
 		run_test(cmd, &tests[i]);
 	free(cmd);
 
-	unlink(OUTPUT_FILE);
-	unlink(INPUT_FILE);
+	remove(OUTPUT_FILE);
+	remove(INPUT_FILE);
 
 	printf("/* Generated by CCAN configurator */\n"
 	       "#ifndef CCAN_CONFIG_H\n"
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH 2/9] configurator: Reimplement run using popen

[Kevin Locke]

Rather than using fork+pipe+system+waitpid, most of which are only
available on POSIX-like systems, use popen which is also available on
Windows (under the name _popen).

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/configurator/configurator.c | 77 +++++++++++++++------------------------
 1 file changed, 29 insertions(+), 48 deletions(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index e322c76..9817fcd 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -24,14 +24,14 @@
 #include <stdio.h>
 #include <stdbool.h>
 #include <stdlib.h>
-#include <unistd.h>
 #include <err.h>
-#include <sys/types.h>
-#include <sys/wait.h>
-#include <sys/stat.h>
-#include <fcntl.h>
 #include <string.h>
 
+#ifdef _MSC_VER
+#define popen _popen
+#define pclose _pclose
+#endif
+
 #define DEFAULT_COMPILER "cc"
 #define DEFAULT_FLAGS "-g3 -ggdb -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition"
 
@@ -367,20 +367,19 @@ static struct test tests[] = {
 	},
 };
 
-static char *grab_fd(int fd)
+static char *grab_stream(FILE *file)
 {
-	int ret;
-	size_t max, size = 0;
+	size_t max, ret, size = 0;
 	char *buffer;
 
-	max = 16384;
-	buffer = malloc(max+1);
-	while ((ret = read(fd, buffer + size, max - size)) > 0) {
+	max = BUFSIZ;
+	buffer = malloc(max);
+	while ((ret = fread(buffer+size, 1, max - size, file)) == max - size) {
 		size += ret;
-		if (size == max)
-			buffer = realloc(buffer, max *= 2);
+		buffer = realloc(buffer, max *= 2);
 	}
-	if (ret < 0)
+	size += ret;
+	if (ferror(file))
 		err(1, "reading from command");
 	buffer[size] = '\0';
 	return buffer;
@@ -388,43 +387,25 @@ static char *grab_fd(int fd)
 
 static char *run(const char *cmd, int *exitstatus)
 {
-	pid_t pid;
-	int p[2];
+	static const char redir[] = " 2>&1";
+	size_t cmdlen;
+	char *cmdredir;
+	FILE *cmdout;
 	char *ret;
-	int status;
 
-	if (pipe(p) != 0)
-		err(1, "creating pipe");
-
-	pid = fork();
-	if (pid == -1)
-		err(1, "forking");
-
-	if (pid == 0) {
-		if (dup2(p[1], STDOUT_FILENO) != STDOUT_FILENO
-		    || dup2(p[1], STDERR_FILENO) != STDERR_FILENO
-		    || close(p[0]) != 0
-		    || close(STDIN_FILENO) != 0
-		    || open("/dev/null", O_RDONLY) != STDIN_FILENO)
-			exit(128);
-
-		status = system(cmd);
-		if (WIFEXITED(status))
-			exit(WEXITSTATUS(status));
-		/* Here's a hint... */
-		exit(128 + WTERMSIG(status));
-	}
+	cmdlen = strlen(cmd);
+	cmdredir = malloc(cmdlen + sizeof(redir));
+	memcpy(cmdredir, cmd, cmdlen);
+	memcpy(cmdredir + cmdlen, redir, sizeof(redir));
+
+	cmdout = popen(cmdredir, "r");
+	if (!cmdout)
+		err(1, "popen \"%s\"", cmdredir);
+
+	free(cmdredir);
 
-	close(p[1]);
-	ret = grab_fd(p[0]);
-	/* This shouldn't fail... */
-	if (waitpid(pid, &status, 0) != pid)
-		err(1, "Failed to wait for child");
-	close(p[0]);
-	if (WIFEXITED(status))
-		*exitstatus = WEXITSTATUS(status);
-	else
-		*exitstatus = -WTERMSIG(status);
+	ret = grab_stream(cmdout);
+	*exitstatus = pclose(cmdout);
 	return ret;
 }
 
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH 3/9] configurator: Inline err.h functions from musl libc

[Kevin Locke]

Since we can't use HAVE_ERR_H in configurator, provide a definition for
the err.h functions used.  The version provided is the one from musl
libc, since it is concise and shares the MIT License with configurator.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/configurator/configurator.c | 54 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index 9817fcd..9dcf307 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -3,6 +3,9 @@
  *
  * Copyright 2011 Rusty Russell <rusty@rustcorp.com.au>.  MIT license.
  *
+ * err, errx, verr, verrx, vwarn, vwarnx functions from musl libc
+ * Copyright 2005-2013 Rich Felker.  MIT license.
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
  * in the Software without restriction, including without limitation the rights
@@ -22,9 +25,9 @@
  * THE SOFTWARE.
  */
 #include <stdio.h>
+#include <stdarg.h>
 #include <stdbool.h>
 #include <stdlib.h>
-#include <err.h>
 #include <string.h>
 
 #ifdef _MSC_VER
@@ -38,6 +41,7 @@
 #define OUTPUT_FILE "configurator.out"
 #define INPUT_FILE "configuratortest.c"
 
+static const char *progname = "";
 static int verbose;
 
 enum test_style {
@@ -367,6 +371,51 @@ static struct test tests[] = {
 	},
 };
 
+static void vwarn(const char *fmt, va_list ap)
+{
+	fprintf (stderr, "%s: ", progname);
+	if (fmt) {
+		vfprintf(stderr, fmt, ap);
+		fputs (": ", stderr);
+	}
+	perror(0);
+}
+
+static void vwarnx(const char *fmt, va_list ap)
+{
+	fprintf (stderr, "%s: ", progname);
+	if (fmt) vfprintf(stderr, fmt, ap);
+	putc('\n', stderr);
+}
+
+static void verr(int status, const char *fmt, va_list ap)
+{
+	vwarn(fmt, ap);
+	exit(status);
+}
+
+static void verrx(int status, const char *fmt, va_list ap)
+{
+	vwarnx(fmt, ap);
+	exit(status);
+}
+
+static void err(int status, const char *fmt, ...)
+{
+	va_list ap;
+	va_start(ap, fmt);
+	verr(status, fmt, ap);
+	va_end(ap);
+}
+
+static void errx(int status, const char *fmt, ...)
+{
+	va_list ap;
+	va_start(ap, fmt);
+	verrx(status, fmt, ap);
+	va_end(ap);
+}
+
 static char *grab_stream(FILE *file)
 {
 	size_t max, ret, size = 0;
@@ -593,6 +642,9 @@ int main(int argc, const char *argv[])
 	const char *default_args[]
 		= { "", DEFAULT_COMPILER, DEFAULT_FLAGS, NULL };
 
+	if (argc > 0)
+		progname = argv[0];
+
 	if (argc > 1) {
 		if (strcmp(argv[1], "--help") == 0) {
 			printf("Usage: configurator [-v] [<compiler> <flags>...]\n"
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH 4/9] configurator: Use native directory separator

[Kevin Locke]

Although Windows APIs generally permit "/" or "\\" for directory
separators in paths, cmd.exe does not recognize "./" when invoking
executables using a relative path and prints the following error:

    '.' is not recognized as an internal or external command,
    operable program or batch file.

Therefore, use "\\" when invoking tests on Windows.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/configurator/configurator.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index 9dcf307..e95fd70 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -41,6 +41,12 @@
 #define OUTPUT_FILE "configurator.out"
 #define INPUT_FILE "configuratortest.c"
 
+#ifdef _WIN32
+#define DIR_SEP   "\\"
+#else
+#define DIR_SEP   "/"
+#endif
+
 static const char *progname = "";
 static int verbose;
 
@@ -615,7 +621,7 @@ static bool run_test(const char *cmd, struct test *test)
 		free(output);
 		/* We run INSIDE_MAIN tests for sanity checking. */
 		if ((test->style & EXECUTE) || (test->style & INSIDE_MAIN)) {
-			output = run("./" OUTPUT_FILE, &status);
+			output = run("." DIR_SEP OUTPUT_FILE, &status);
 			if (!(test->style & EXECUTE) && status != 0)
 				errx(1, "Test for %s failed with %i:\n%s",
 				     test->name, status, output);
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH 5/9] configurator: Mark non-Windows tests MAY_NOT_COMPILE

[Kevin Locke]

Several of the EXECUTABLE tests depend on headers not available on
Windows.  Mark these tests MAY_NOT_COMPILE to handle this.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/configurator/configurator.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index e95fd70..0307635 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -72,7 +72,7 @@ struct test {
 };
 
 static struct test tests[] = {
-	{ "HAVE_32BIT_OFF_T", DEFINES_EVERYTHING|EXECUTE, NULL, NULL,
+	{ "HAVE_32BIT_OFF_T", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE, NULL, NULL,
 	  "#include <sys/types.h>\n"
 	  "int main(void) {\n"
 	  "	return sizeof(off_t) == 4 ? 0 : 1;\n"
@@ -177,7 +177,7 @@ static struct test tests[] = {
 	{ "HAVE_COMPOUND_LITERALS", INSIDE_MAIN, NULL, NULL,
 	  "int *foo = (int[]) { 1, 2, 3, 4 };\n"
 	  "return foo[0] ? 0 : 1;" },
-	{ "HAVE_FCHDIR", DEFINES_EVERYTHING|EXECUTE, NULL, NULL,
+	{ "HAVE_FCHDIR", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE, NULL, NULL,
 	  "#include <sys/types.h>\n"
 	  "#include <sys/stat.h>\n"
 	  "#include <fcntl.h>\n"
@@ -198,7 +198,7 @@ static struct test tests[] = {
 	  "	if (arg == 4)\n"
 	  "		warnx(\"warn %u\", arg);\n"
 	  "}\n" },
-	{ "HAVE_FILE_OFFSET_BITS", DEFINES_EVERYTHING|EXECUTE,
+	{ "HAVE_FILE_OFFSET_BITS", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE,
 	  "HAVE_32BIT_OFF_T", NULL,
 	  "#define _FILE_OFFSET_BITS 64\n"
 	  "#include <sys/types.h>\n"
@@ -244,7 +244,7 @@ static struct test tests[] = {
 	  "static void *func(int fd) {\n"
 	  "	return mmap(0, 65536, PROT_READ, MAP_SHARED, fd, 0);\n"
 	  "}" },
-	{ "HAVE_PROC_SELF_MAPS", DEFINES_EVERYTHING|EXECUTE, NULL, NULL,
+	{ "HAVE_PROC_SELF_MAPS", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE, NULL, NULL,
 	  "#include <sys/types.h>\n"
 	  "#include <sys/stat.h>\n"
 	  "#include <fcntl.h>\n"
@@ -329,7 +329,7 @@ static struct test tests[] = {
 	  "-Werror -fopenmp" },
 	{ "HAVE_VALGRIND_MEMCHECK_H", OUTSIDE_MAIN, NULL, NULL,
 	  "#include <valgrind/memcheck.h>\n" },
-	{ "HAVE_UCONTEXT", DEFINES_EVERYTHING|EXECUTE,
+	{ "HAVE_UCONTEXT", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE,
 	  NULL, NULL,
 	  "#include <ucontext.h>\n"
 	  "static int x = 0;\n"
@@ -350,7 +350,7 @@ static struct test tests[] = {
 	  "	return (x == 3) ? 0 : 1;\n"
 	  "}\n"
 	},
-	{ "HAVE_POINTER_SAFE_MAKECONTEXT", DEFINES_EVERYTHING|EXECUTE,
+	{ "HAVE_POINTER_SAFE_MAKECONTEXT", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE,
 	  "HAVE_UCONTEXT", NULL,
 	  "#include <stddef.h>\n"
 	  "#include <ucontext.h>\n"
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH 6/9] configurator: Print test source without cat

[Kevin Locke]

Windows does not provide cat.  Instead, copy the test source to stdout
using the file stream to which it was written.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/configurator/configurator.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index 0307635..0386930 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -422,6 +422,22 @@ static void errx(int status, const char *fmt, ...)
 	va_end(ap);
 }
 
+static size_t fcopy(FILE *fsrc, FILE *fdst)
+{
+	char buffer[BUFSIZ];
+	size_t rsize, wsize;
+	size_t copied = 0;
+
+	while ((rsize = fread(buffer, 1, BUFSIZ, fsrc)) > 0) {
+		wsize = fwrite(buffer, 1, rsize, fdst);
+		copied += wsize;
+		if (wsize != rsize)
+			break;
+	}
+
+	return copied;
+}
+
 static char *grab_stream(FILE *file)
 {
 	size_t max, ret, size = 0;
@@ -545,7 +561,7 @@ static bool run_test(const char *cmd, struct test *test)
 		}
 	}
 
-	outf = fopen(INPUT_FILE, "w");
+	outf = fopen(INPUT_FILE, verbose > 1 ? "w+" : "w");
 	if (!outf)
 		err(1, "creating %s", INPUT_FILE);
 
@@ -576,11 +592,13 @@ static bool run_test(const char *cmd, struct test *test)
 		abort();
 
 	}
-	fclose(outf);
 
-	if (verbose > 1)
-		if (system("cat " INPUT_FILE) == -1)
-			;
+	if (verbose > 1) {
+		fseek(outf, 0, SEEK_SET);
+		fcopy(outf, stdout);
+	}
+
+	fclose(outf);
 
 	newcmd = strdup(cmd);
 
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH 7/9] configurator: Fix compiler warning with compare

[Kevin Locke]

Visual Studio prints warning C4706 "assignment within conditional
expression" when there is an assignment without a comparison in a
conditional expression.  Therefore, to silence the warning, add an
explicit comparison.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/configurator/configurator.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index 0386930..0c30aff 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -535,7 +535,7 @@ static bool run_test(const char *cmd, struct test *test)
 		char *dep;
 
 		/* Space-separated dependencies, could be ! for inverse. */
-		while ((len = strcspn(deps, " "))) {
+		while ((len = strcspn(deps, " ")) != 0) {
 			bool positive = true;
 			if (deps[len]) {
 				dep = strdup(deps);
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH 8/9] configurator: Pass output cflag to configurator

[Kevin Locke]

Unfortunately, not all compilers support -o as a command-line option for
specifying the output file.  Visual Studio cl.exe issues warning D9035
when -o is given, which is detected as a compile warning by the
configurator.

To support such compilers, pass the output flag as the last flag to the
configurator.

This is a breaking change.  Existing scripts which pass flags to the
compiler must be modified to add "-o" as the last flag.  As noted in the
cover letter for this patch series, I'm open to considering alternatives
if this is unacceptable.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 Makefile                          | 2 +-
 tools/configurator/configurator.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 127b875..f36ccb5 100644
--- a/Makefile
+++ b/Makefile
@@ -89,7 +89,7 @@ FORCE:
 
 # Ensure we don't end up with empty file if configurator fails!
 config.h: tools/configurator/configurator Makefile Makefile-ccan
-	tools/configurator/configurator $(CC) $(CCAN_CFLAGS) > $@.tmp && mv $@.tmp $@
+	tools/configurator/configurator $(CC) $(CCAN_CFLAGS) -o > $@.tmp && mv $@.tmp $@
 
 include tools/Makefile
 -include inter-depends
diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index 0c30aff..820ccf7 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -36,7 +36,7 @@
 #endif
 
 #define DEFAULT_COMPILER "cc"
-#define DEFAULT_FLAGS "-g3 -ggdb -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition"
+#define DEFAULT_FLAGS "-g3 -ggdb -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition -o"
 
 #define OUTPUT_FILE "configurator.out"
 #define INPUT_FILE "configuratortest.c"
@@ -672,7 +672,7 @@ int main(int argc, const char *argv[])
 	if (argc > 1) {
 		if (strcmp(argv[1], "--help") == 0) {
 			printf("Usage: configurator [-v] [<compiler> <flags>...]\n"
-			       "  <compiler> <flags> will have \"-o <outfile> <infile.c>\" appended\n"
+			       "  <compiler> <flags> will have \"<outfile> <infile.c>\" appended\n"
 			       "Default: %s %s\n",
 			       DEFAULT_COMPILER, DEFAULT_FLAGS);
 			exit(0);
@@ -691,7 +691,7 @@ int main(int argc, const char *argv[])
 	if (argc == 1)
 		argv = default_args;
 
-	cmd = connect_args(argv, " -o " OUTPUT_FILE " " INPUT_FILE);
+	cmd = connect_args(argv, OUTPUT_FILE " " INPUT_FILE);
 	for (i = 0; i < sizeof(tests)/sizeof(tests[0]); i++)
 		run_test(cmd, &tests[i]);
 	free(cmd);
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH 9/9] configurator: DEFAULT_{COMPILER, FLAGS} for MSVC

[Kevin Locke]

When compiling with Visual Studio, use default compiler name and flags
which are likely to work with the known-available compiler.

This is also a convenience for users who may not know what arguments
cl.exe may need to compile the tests.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/configurator/configurator.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index 820ccf7..4f08701 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -35,8 +35,14 @@
 #define pclose _pclose
 #endif
 
+#ifdef _MSC_VER
+#define DEFAULT_COMPILER "cl"
+#define DEFAULT_FLAGS "/nologo /Zi /W4 " \
+	"/D_CRT_NONSTDC_NO_WARNINGS /D_CRT_SECURE_NO_WARNINGS /Fe:"
+#else
 #define DEFAULT_COMPILER "cc"
 #define DEFAULT_FLAGS "-g3 -ggdb -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition -o"
+#endif
 
 #define OUTPUT_FILE "configurator.out"
 #define INPUT_FILE "configuratortest.c"
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH 0/9] configurator: Support for Windows and MSVC

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 2960 bytes --]

On Sun, Sep 18, 2016 at 06:51:57PM -0600, Kevin Locke wrote:
> Hi all,
> 
> This patch series adds support for building configurator with Microsoft
> Visual C++ and running it on Windows.  The motivation is to add
> sufficient support for Windows to allow using header-only modules
> with minimal hassle.  For me, this patch series is sufficient, so I
> don't have any further plans for changes to other files or the build
> system at the moment.

First, thanks for this.  ccan isn't supposed to be platform specific,
but it kind of is at the moment, simply because most of the
contributions have been from Linux developers.  I'm really happy to
see some work to make it useful on different platforms.

Second, because I'm not and have never been a Windows dev, my ability
to review these is limited - so I'm inclined to apply anything that
doesn't break builds on Linux / POSIX.

Third, because most of the main ccan contributors are not Windows
devs, I'm a bit worried about bitrot.  If you can think of any way of
making some kind of automated smoke test form builds on Windows, that
would be terrific.  Otherwise, I guess we'll just hope for the best.

> The two patches that are particularly likely to raise objections are
> patch 3 and 8:
> 
> Patch 3 adds a copy of the err.h functions used by configurator to work
> around the lack of err.h on Windows.  If this is too much code to lump
> into configurator.c, I can split it out (and copy err.{c,h} verbatim
> from musl) or call fprintf+exit directly.
> 
> Patch 8 adds the output cflag (-o) to DEFAULT_CFLAGS and the expected
> configurator cflags argument.  This changes both the configurator API
> and the config.h API and will break existing code which depend on these.
> If that is unacceptable, I'd be open to implementing a
> backwards-compatible alternative.  One idea would be to add an
> --output-cflag option to configurator, although it will require quite a
> bit of argument parsing code.  I'm open to other alternatives as well.
> 
> Thanks for considering,
> Kevin
> 
> Kevin Locke (9):
>   configurator: Replace unlink with remove
>   configurator: Reimplement run using popen
>   configurator: Inline err.h functions from musl libc
>   configurator: Use native directory separator
>   configurator: Mark non-Windows tests MAY_NOT_COMPILE
>   configurator: Print test source without cat
>   configurator: Fix compiler warning with compare
>   configurator: Pass output cflag to configurator
>   configurator: DEFAULT_{COMPILER,FLAGS} for MSVC
> 
>  Makefile                          |   2 +-
>  tools/configurator/configurator.c | 197 +++++++++++++++++++++++++-------------
>  2 files changed, 131 insertions(+), 68 deletions(-)
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH 1/9] configurator: Replace unlink with remove

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 1226 bytes --]

On Sun, Sep 18, 2016 at 06:51:58PM -0600, Kevin Locke wrote:
> Although Windows provides unlink, using it adds complication due to the
> lack of unistd.h which must be included to define the function on POSIX
> systems.  Instead, use remove, which C89 requires to be in stdio.h.
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  tools/configurator/configurator.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
> index 67dadc6..e322c76 100644
> --- a/tools/configurator/configurator.c
> +++ b/tools/configurator/configurator.c
> @@ -639,8 +639,8 @@ int main(int argc, const char *argv[])
>  		run_test(cmd, &tests[i]);
>  	free(cmd);
>  
> -	unlink(OUTPUT_FILE);
> -	unlink(INPUT_FILE);
> +	remove(OUTPUT_FILE);
> +	remove(INPUT_FILE);
>  
>  	printf("/* Generated by CCAN configurator */\n"
>  	       "#ifndef CCAN_CONFIG_H\n"

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH 2/9] configurator: Reimplement run using popen

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 3938 bytes --]

On Sun, Sep 18, 2016 at 06:51:59PM -0600, Kevin Locke wrote:
> Rather than using fork+pipe+system+waitpid, most of which are only
> available on POSIX-like systems, use popen which is also available on
> Windows (under the name _popen).

Concept looks good, one little wart.

> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
> ---
>  tools/configurator/configurator.c | 77 +++++++++++++++------------------------
>  1 file changed, 29 insertions(+), 48 deletions(-)
> 
> diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
> index e322c76..9817fcd 100644
> --- a/tools/configurator/configurator.c
> +++ b/tools/configurator/configurator.c
> @@ -24,14 +24,14 @@
>  #include <stdio.h>
>  #include <stdbool.h>
>  #include <stdlib.h>
> -#include <unistd.h>
>  #include <err.h>
> -#include <sys/types.h>
> -#include <sys/wait.h>
> -#include <sys/stat.h>
> -#include <fcntl.h>
>  #include <string.h>
>  
> +#ifdef _MSC_VER
> +#define popen _popen
> +#define pclose _pclose
> +#endif
> +
>  #define DEFAULT_COMPILER "cc"
>  #define DEFAULT_FLAGS "-g3 -ggdb -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition"
>  
> @@ -367,20 +367,19 @@ static struct test tests[] = {
>  	},
>  };
>  
> -static char *grab_fd(int fd)
> +static char *grab_stream(FILE *file)
>  {
> -	int ret;
> -	size_t max, size = 0;
> +	size_t max, ret, size = 0;
>  	char *buffer;
>  
> -	max = 16384;
> -	buffer = malloc(max+1);
> -	while ((ret = read(fd, buffer + size, max - size)) > 0) {
> +	max = BUFSIZ;
> +	buffer = malloc(max);
> +	while ((ret = fread(buffer+size, 1, max - size, file)) == max - size) {

This assumes that fread() will never return a short read except on EOF
or error.  I expect that will be true in practice for regular files,
but I'm not sure if it's a good idea to rely on it.

>  		size += ret;
> -		if (size == max)
> -			buffer = realloc(buffer, max *= 2);
> +		buffer = realloc(buffer, max *= 2);
>  	}
> -	if (ret < 0)
> +	size += ret;
> +	if (ferror(file))
>  		err(1, "reading from command");
>  	buffer[size] = '\0';
>  	return buffer;
> @@ -388,43 +387,25 @@ static char *grab_fd(int fd)
>  
>  static char *run(const char *cmd, int *exitstatus)
>  {
> -	pid_t pid;
> -	int p[2];
> +	static const char redir[] = " 2>&1";
> +	size_t cmdlen;
> +	char *cmdredir;
> +	FILE *cmdout;
>  	char *ret;
> -	int status;
>  
> -	if (pipe(p) != 0)
> -		err(1, "creating pipe");
> -
> -	pid = fork();
> -	if (pid == -1)
> -		err(1, "forking");
> -
> -	if (pid == 0) {
> -		if (dup2(p[1], STDOUT_FILENO) != STDOUT_FILENO
> -		    || dup2(p[1], STDERR_FILENO) != STDERR_FILENO
> -		    || close(p[0]) != 0
> -		    || close(STDIN_FILENO) != 0
> -		    || open("/dev/null", O_RDONLY) != STDIN_FILENO)
> -			exit(128);
> -
> -		status = system(cmd);
> -		if (WIFEXITED(status))
> -			exit(WEXITSTATUS(status));
> -		/* Here's a hint... */
> -		exit(128 + WTERMSIG(status));
> -	}
> +	cmdlen = strlen(cmd);
> +	cmdredir = malloc(cmdlen + sizeof(redir));
> +	memcpy(cmdredir, cmd, cmdlen);
> +	memcpy(cmdredir + cmdlen, redir, sizeof(redir));
> +
> +	cmdout = popen(cmdredir, "r");
> +	if (!cmdout)
> +		err(1, "popen \"%s\"", cmdredir);
> +
> +	free(cmdredir);
>  
> -	close(p[1]);
> -	ret = grab_fd(p[0]);
> -	/* This shouldn't fail... */
> -	if (waitpid(pid, &status, 0) != pid)
> -		err(1, "Failed to wait for child");
> -	close(p[0]);
> -	if (WIFEXITED(status))
> -		*exitstatus = WEXITSTATUS(status);
> -	else
> -		*exitstatus = -WTERMSIG(status);
> +	ret = grab_stream(cmdout);
> +	*exitstatus = pclose(cmdout);
>  	return ret;
>  }
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH 3/9] configurator: Inline err.h functions from musl libc

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 3489 bytes --]

On Sun, Sep 18, 2016 at 06:52:00PM -0600, Kevin Locke wrote:
> Since we can't use HAVE_ERR_H in configurator, provide a definition for
> the err.h functions used.  The version provided is the one from musl
> libc, since it is concise and shares the MIT License with configurator.
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>

Right, so this is a general problem of how do we make the configurator
itself sufficiently portable, since it hasn't yet worked out the
portbility macros.

Personally, I think duplicating these here is an acceptable
workaround.  Rusty, any opinion?

> ---
>  tools/configurator/configurator.c | 54 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
> index 9817fcd..9dcf307 100644
> --- a/tools/configurator/configurator.c
> +++ b/tools/configurator/configurator.c
> @@ -3,6 +3,9 @@
>   *
>   * Copyright 2011 Rusty Russell <rusty@rustcorp.com.au>.  MIT license.
>   *
> + * err, errx, verr, verrx, vwarn, vwarnx functions from musl libc
> + * Copyright 2005-2013 Rich Felker.  MIT license.
> + *
>   * Permission is hereby granted, free of charge, to any person obtaining a copy
>   * of this software and associated documentation files (the "Software"), to deal
>   * in the Software without restriction, including without limitation the rights
> @@ -22,9 +25,9 @@
>   * THE SOFTWARE.
>   */
>  #include <stdio.h>
> +#include <stdarg.h>
>  #include <stdbool.h>
>  #include <stdlib.h>
> -#include <err.h>
>  #include <string.h>
>  
>  #ifdef _MSC_VER
> @@ -38,6 +41,7 @@
>  #define OUTPUT_FILE "configurator.out"
>  #define INPUT_FILE "configuratortest.c"
>  
> +static const char *progname = "";
>  static int verbose;
>  
>  enum test_style {
> @@ -367,6 +371,51 @@ static struct test tests[] = {
>  	},
>  };
>  
> +static void vwarn(const char *fmt, va_list ap)
> +{
> +	fprintf (stderr, "%s: ", progname);
> +	if (fmt) {
> +		vfprintf(stderr, fmt, ap);
> +		fputs (": ", stderr);
> +	}
> +	perror(0);
> +}
> +
> +static void vwarnx(const char *fmt, va_list ap)
> +{
> +	fprintf (stderr, "%s: ", progname);
> +	if (fmt) vfprintf(stderr, fmt, ap);
> +	putc('\n', stderr);
> +}
> +
> +static void verr(int status, const char *fmt, va_list ap)
> +{
> +	vwarn(fmt, ap);
> +	exit(status);
> +}
> +
> +static void verrx(int status, const char *fmt, va_list ap)
> +{
> +	vwarnx(fmt, ap);
> +	exit(status);
> +}
> +
> +static void err(int status, const char *fmt, ...)
> +{
> +	va_list ap;
> +	va_start(ap, fmt);
> +	verr(status, fmt, ap);
> +	va_end(ap);
> +}
> +
> +static void errx(int status, const char *fmt, ...)
> +{
> +	va_list ap;
> +	va_start(ap, fmt);
> +	verrx(status, fmt, ap);
> +	va_end(ap);
> +}
> +
>  static char *grab_stream(FILE *file)
>  {
>  	size_t max, ret, size = 0;
> @@ -593,6 +642,9 @@ int main(int argc, const char *argv[])
>  	const char *default_args[]
>  		= { "", DEFAULT_COMPILER, DEFAULT_FLAGS, NULL };
>  
> +	if (argc > 0)
> +		progname = argv[0];
> +
>  	if (argc > 1) {
>  		if (strcmp(argv[1], "--help") == 0) {
>  			printf("Usage: configurator [-v] [<compiler> <flags>...]\n"

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH 4/9] configurator: Use native directory separator

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 1887 bytes --]

On Sun, Sep 18, 2016 at 06:52:01PM -0600, Kevin Locke wrote:
> Although Windows APIs generally permit "/" or "\\" for directory
> separators in paths, cmd.exe does not recognize "./" when invoking
> executables using a relative path and prints the following error:
> 
>     '.' is not recognized as an internal or external command,
>     operable program or batch file.
> 
> Therefore, use "\\" when invoking tests on Windows.
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>

A bit ugly, but I think necessarily so to accomplish the goal.

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  tools/configurator/configurator.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
> index 9dcf307..e95fd70 100644
> --- a/tools/configurator/configurator.c
> +++ b/tools/configurator/configurator.c
> @@ -41,6 +41,12 @@
>  #define OUTPUT_FILE "configurator.out"
>  #define INPUT_FILE "configuratortest.c"
>  
> +#ifdef _WIN32
> +#define DIR_SEP   "\\"
> +#else
> +#define DIR_SEP   "/"
> +#endif
> +
>  static const char *progname = "";
>  static int verbose;
>  
> @@ -615,7 +621,7 @@ static bool run_test(const char *cmd, struct test *test)
>  		free(output);
>  		/* We run INSIDE_MAIN tests for sanity checking. */
>  		if ((test->style & EXECUTE) || (test->style & INSIDE_MAIN)) {
> -			output = run("./" OUTPUT_FILE, &status);
> +			output = run("." DIR_SEP OUTPUT_FILE, &status);
>  			if (!(test->style & EXECUTE) && status != 0)
>  				errx(1, "Test for %s failed with %i:\n%s",
>  				     test->name, status, output);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH 5/9] configurator: Mark non-Windows tests MAY_NOT_COMPILE

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 3156 bytes --]


On Sun, Sep 18, 2016 at 06:52:02PM -0600, Kevin Locke wrote:
> Several of the EXECUTABLE tests depend on headers not available on
> Windows.  Mark these tests MAY_NOT_COMPILE to handle this.
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  tools/configurator/configurator.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
> index e95fd70..0307635 100644
> --- a/tools/configurator/configurator.c
> +++ b/tools/configurator/configurator.c
> @@ -72,7 +72,7 @@ struct test {
>  };
>  
>  static struct test tests[] = {
> -	{ "HAVE_32BIT_OFF_T", DEFINES_EVERYTHING|EXECUTE, NULL, NULL,
> +	{ "HAVE_32BIT_OFF_T", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE, NULL, NULL,
>  	  "#include <sys/types.h>\n"
>  	  "int main(void) {\n"
>  	  "	return sizeof(off_t) == 4 ? 0 : 1;\n"
> @@ -177,7 +177,7 @@ static struct test tests[] = {
>  	{ "HAVE_COMPOUND_LITERALS", INSIDE_MAIN, NULL, NULL,
>  	  "int *foo = (int[]) { 1, 2, 3, 4 };\n"
>  	  "return foo[0] ? 0 : 1;" },
> -	{ "HAVE_FCHDIR", DEFINES_EVERYTHING|EXECUTE, NULL, NULL,
> +	{ "HAVE_FCHDIR", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE, NULL, NULL,
>  	  "#include <sys/types.h>\n"
>  	  "#include <sys/stat.h>\n"
>  	  "#include <fcntl.h>\n"
> @@ -198,7 +198,7 @@ static struct test tests[] = {
>  	  "	if (arg == 4)\n"
>  	  "		warnx(\"warn %u\", arg);\n"
>  	  "}\n" },
> -	{ "HAVE_FILE_OFFSET_BITS", DEFINES_EVERYTHING|EXECUTE,
> +	{ "HAVE_FILE_OFFSET_BITS", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE,
>  	  "HAVE_32BIT_OFF_T", NULL,
>  	  "#define _FILE_OFFSET_BITS 64\n"
>  	  "#include <sys/types.h>\n"
> @@ -244,7 +244,7 @@ static struct test tests[] = {
>  	  "static void *func(int fd) {\n"
>  	  "	return mmap(0, 65536, PROT_READ, MAP_SHARED, fd, 0);\n"
>  	  "}" },
> -	{ "HAVE_PROC_SELF_MAPS", DEFINES_EVERYTHING|EXECUTE, NULL, NULL,
> +	{ "HAVE_PROC_SELF_MAPS", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE, NULL, NULL,
>  	  "#include <sys/types.h>\n"
>  	  "#include <sys/stat.h>\n"
>  	  "#include <fcntl.h>\n"
> @@ -329,7 +329,7 @@ static struct test tests[] = {
>  	  "-Werror -fopenmp" },
>  	{ "HAVE_VALGRIND_MEMCHECK_H", OUTSIDE_MAIN, NULL, NULL,
>  	  "#include <valgrind/memcheck.h>\n" },
> -	{ "HAVE_UCONTEXT", DEFINES_EVERYTHING|EXECUTE,
> +	{ "HAVE_UCONTEXT", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE,
>  	  NULL, NULL,
>  	  "#include <ucontext.h>\n"
>  	  "static int x = 0;\n"
> @@ -350,7 +350,7 @@ static struct test tests[] = {
>  	  "	return (x == 3) ? 0 : 1;\n"
>  	  "}\n"
>  	},
> -	{ "HAVE_POINTER_SAFE_MAKECONTEXT", DEFINES_EVERYTHING|EXECUTE,
> +	{ "HAVE_POINTER_SAFE_MAKECONTEXT", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE,
>  	  "HAVE_UCONTEXT", NULL,
>  	  "#include <stddef.h>\n"
>  	  "#include <ucontext.h>\n"

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH 6/9] configurator: Print test source without cat

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 2221 bytes --]

On Sun, Sep 18, 2016 at 06:52:03PM -0600, Kevin Locke wrote:
> Windows does not provide cat.  Instead, copy the test source to stdout
> using the file stream to which it was written.
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
> ---
>  tools/configurator/configurator.c | 28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
> index 0307635..0386930 100644
> --- a/tools/configurator/configurator.c
> +++ b/tools/configurator/configurator.c
> @@ -422,6 +422,22 @@ static void errx(int status, const char *fmt, ...)
>  	va_end(ap);
>  }
>  
> +static size_t fcopy(FILE *fsrc, FILE *fdst)
> +{
> +	char buffer[BUFSIZ];
> +	size_t rsize, wsize;
> +	size_t copied = 0;
> +
> +	while ((rsize = fread(buffer, 1, BUFSIZ, fsrc)) > 0) {
> +		wsize = fwrite(buffer, 1, rsize, fdst);
> +		copied += wsize;
> +		if (wsize != rsize)
> +			break;

Similar to 2/9 you're assuming you'll never get a short fwrite() here.
Since the output stream is very likely to be a terminal in this case,
that seems a lot more likely to break than just assuming no short
reads on fread().  I think this one really does need to handle short
writes here.

> +	}
> +
> +	return copied;
> +}
> +
>  static char *grab_stream(FILE *file)
>  {
>  	size_t max, ret, size = 0;
> @@ -545,7 +561,7 @@ static bool run_test(const char *cmd, struct test *test)
>  		}
>  	}
>  
> -	outf = fopen(INPUT_FILE, "w");
> +	outf = fopen(INPUT_FILE, verbose > 1 ? "w+" : "w");
>  	if (!outf)
>  		err(1, "creating %s", INPUT_FILE);
>  
> @@ -576,11 +592,13 @@ static bool run_test(const char *cmd, struct test *test)
>  		abort();
>  
>  	}
> -	fclose(outf);
>  
> -	if (verbose > 1)
> -		if (system("cat " INPUT_FILE) == -1)
> -			;
> +	if (verbose > 1) {
> +		fseek(outf, 0, SEEK_SET);
> +		fcopy(outf, stdout);
> +	}
> +
> +	fclose(outf);
>  
>  	newcmd = strdup(cmd);
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH 7/9] configurator: Fix compiler warning with compare

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 1262 bytes --]

On Sun, Sep 18, 2016 at 06:52:04PM -0600, Kevin Locke wrote:
> Visual Studio prints warning C4706 "assignment within conditional
> expression" when there is an assignment without a comparison in a
> conditional expression.  Therefore, to silence the warning, add an
> explicit comparison.
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  tools/configurator/configurator.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
> index 0386930..0c30aff 100644
> --- a/tools/configurator/configurator.c
> +++ b/tools/configurator/configurator.c
> @@ -535,7 +535,7 @@ static bool run_test(const char *cmd, struct test *test)
>  		char *dep;
>  
>  		/* Space-separated dependencies, could be ! for inverse. */
> -		while ((len = strcspn(deps, " "))) {
> +		while ((len = strcspn(deps, " ")) != 0) {
>  			bool positive = true;
>  			if (deps[len]) {
>  				dep = strdup(deps);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH 8/9] configurator: Pass output cflag to configurator

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 3699 bytes --]

On Sun, Sep 18, 2016 at 06:52:05PM -0600, Kevin Locke wrote:
> Unfortunately, not all compilers support -o as a command-line option for
> specifying the output file.  Visual Studio cl.exe issues warning D9035
> when -o is given, which is detected as a compile warning by the
> configurator.
> 
> To support such compilers, pass the output flag as the last flag to the
> configurator.
> 
> This is a breaking change.  Existing scripts which pass flags to the
> compiler must be modified to add "-o" as the last flag.  As noted in the
> cover letter for this patch series, I'm open to considering alternatives
> if this is unacceptable.
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>

So, as it stands, this change completely breaks ccanlint on POSIX,
which is not ok.  Specifically it looks like the problem is that the
DEFAULT_FLAGS from the configurator make it into CCAN_CFLAGS in
config.h.  ccanlint then tries to add further options and its own -o
to the end of that, which isn't going to work.

In short, I think the problem is that ccanlint needs fixes to work
with MSVC (and possibly anything much else apart from gcc).  I think
we need to look at how to fix that and the configurator together with
each other.

Not having working ccanlint on Windows seems like a pretty big, since
its the main tool for testing ccan, amongst other things.

> ---
>  Makefile                          | 2 +-
>  tools/configurator/configurator.c | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 127b875..f36ccb5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -89,7 +89,7 @@ FORCE:
>  
>  # Ensure we don't end up with empty file if configurator fails!
>  config.h: tools/configurator/configurator Makefile Makefile-ccan
> -	tools/configurator/configurator $(CC) $(CCAN_CFLAGS) > $@.tmp && mv $@.tmp $@
> +	tools/configurator/configurator $(CC) $(CCAN_CFLAGS) -o > $@.tmp && mv $@.tmp $@
>  
>  include tools/Makefile
>  -include inter-depends
> diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
> index 0c30aff..820ccf7 100644
> --- a/tools/configurator/configurator.c
> +++ b/tools/configurator/configurator.c
> @@ -36,7 +36,7 @@
>  #endif
>  
>  #define DEFAULT_COMPILER "cc"
> -#define DEFAULT_FLAGS "-g3 -ggdb -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition"
> +#define DEFAULT_FLAGS "-g3 -ggdb -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition -o"
>  
>  #define OUTPUT_FILE "configurator.out"
>  #define INPUT_FILE "configuratortest.c"
> @@ -672,7 +672,7 @@ int main(int argc, const char *argv[])
>  	if (argc > 1) {
>  		if (strcmp(argv[1], "--help") == 0) {
>  			printf("Usage: configurator [-v] [<compiler> <flags>...]\n"
> -			       "  <compiler> <flags> will have \"-o <outfile> <infile.c>\" appended\n"
> +			       "  <compiler> <flags> will have \"<outfile> <infile.c>\" appended\n"
>  			       "Default: %s %s\n",
>  			       DEFAULT_COMPILER, DEFAULT_FLAGS);
>  			exit(0);
> @@ -691,7 +691,7 @@ int main(int argc, const char *argv[])
>  	if (argc == 1)
>  		argv = default_args;
>  
> -	cmd = connect_args(argv, " -o " OUTPUT_FILE " " INPUT_FILE);
> +	cmd = connect_args(argv, OUTPUT_FILE " " INPUT_FILE);
>  	for (i = 0; i < sizeof(tests)/sizeof(tests[0]); i++)
>  		run_test(cmd, &tests[i]);
>  	free(cmd);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH 0/9] configurator: Support for Windows and MSVC

[Kevin Locke]

Hi David,

Thank you for taking the time to review these patches, especially for a 
platform that you don't have a strong stake in.  I really appreciate it.

On 09/19/2016 10:48 PM, David Gibson wrote:
> On Sun, Sep 18, 2016 at 06:51:57PM -0600, Kevin Locke wrote:
>> This patch series adds support for building configurator with Microsoft
>> Visual C++ and running it on Windows.  The motivation is to add
>> sufficient support for Windows to allow using header-only modules
>> with minimal hassle.  For me, this patch series is sufficient, so I
>> don't have any further plans for changes to other files or the build
>> system at the moment.
>
> First, thanks for this.  ccan isn't supposed to be platform specific,
> but it kind of is at the moment, simply because most of the
> contributions have been from Linux developers.  I'm really happy to
> see some work to make it useful on different platforms.
>
> Second, because I'm not and have never been a Windows dev, my ability
> to review these is limited - so I'm inclined to apply anything that
> doesn't break builds on Linux / POSIX.
>
> Third, because most of the main ccan contributors are not Windows
> devs, I'm a bit worried about bitrot.  If you can think of any way of
> making some kind of automated smoke test form builds on Windows, that
> would be terrific.  Otherwise, I guess we'll just hope for the best.

I think that's a very legitimate concern.  I'm not a frequent Windows 
developer myself, but had a need for a current project.  I've tried to 
minimize the amount of Windows-specific code, with good results I think, 
but that's obviously not much of a guarantee that it won't rot or subtly 
break on Windows going forward.

I've seen appveyor.com used quite often for Windows CI, and it is free 
for FOSS projects.  I'd be happy to work up some basic smoke tests if it 
seems like that might be useful and not too much of a maintenance 
burden.  Let me know what you think.

Thanks again,
Kevin
_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH 2/9] configurator: Reimplement run using popen

[Kevin Locke]

On 09/19/2016 11:00 PM, David Gibson wrote:
 > On Sun, Sep 18, 2016 at 06:51:59PM -0600, Kevin Locke wrote:
 >> Rather than using fork+pipe+system+waitpid, most of which are only
 >> available on POSIX-like systems, use popen which is also available on
 >> Windows (under the name _popen).
 >
 > Concept looks good, one little wart.
 >
 >> [...]
 >>
 >> -static char *grab_fd(int fd)
 >> +static char *grab_stream(FILE *file)
 >>  {
 >> -	int ret;
 >> -	size_t max, size = 0;
 >> +	size_t max, ret, size = 0;
 >>  	char *buffer;
 >>
 >> -	max = 16384;
 >> -	buffer = malloc(max+1);
 >> -	while ((ret = read(fd, buffer + size, max - size)) > 0) {
 >> +	max = BUFSIZ;
 >> +	buffer = malloc(max);
 >> +	while ((ret = fread(buffer+size, 1, max - size, file)) == max - 
size) {
 >
 > This assumes that fread() will never return a short read except on EOF
 > or error.  I expect that will be true in practice for regular files,
 > but I'm not sure if it's a good idea to rely on it.

Interesting.  I was under the impression that the fread couldn't short 
read.  POSIX describes the return value as "the number of elements 
successfully read which is less than nitems only if a read error or 
end-of-file is encountered."[1]  Now that I think about it, I suppose 
EINTR could be an issue for non-glibc systems.  Is that what you had in 
mind, or are there other issues I'm overlooking?  (Also for fwrite in 6/9?)

Kevin

1.  http://pubs.opengroup.org/onlinepubs/9699919799/functions/fread.html
_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH 8/9] configurator: Pass output cflag to configurator

[Kevin Locke]

On 09/19/2016 11:23 PM, David Gibson wrote:
> On Sun, Sep 18, 2016 at 06:52:05PM -0600, Kevin Locke wrote:
>> Unfortunately, not all compilers support -o as a command-line option for
>> specifying the output file.  Visual Studio cl.exe issues warning D9035
>> when -o is given, which is detected as a compile warning by the
>> configurator.
>>
>> To support such compilers, pass the output flag as the last flag to the
>> configurator.
>>
>> This is a breaking change.  Existing scripts which pass flags to the
>> compiler must be modified to add "-o" as the last flag.  As noted in the
>> cover letter for this patch series, I'm open to considering alternatives
>> if this is unacceptable.
>
> So, as it stands, this change completely breaks ccanlint on POSIX,
> which is not ok.  Specifically it looks like the problem is that the
> DEFAULT_FLAGS from the configurator make it into CCAN_CFLAGS in
> config.h.  ccanlint then tries to add further options and its own -o
> to the end of that, which isn't going to work.
>
> In short, I think the problem is that ccanlint needs fixes to work
> with MSVC (and possibly anything much else apart from gcc).  I think
> we need to look at how to fix that and the configurator together with
> each other.
>
> Not having working ccanlint on Windows seems like a pretty big, since
> its the main tool for testing ccan, amongst other things.

Good catch.  I hadn't noticed the ccanlint breakage.  That was a big 
oversight on my part.

The breakage likely extends to any other programs which use CCAN_CFLAGS, 
since none are expecting the output flag at the end.  Perhaps a better 
solution would be to emit the output flag as a separate macro. 
Something like CCAN_OUTPUT_CFLAG.  What do you think?

The command line API for configurator could either expect the output 
flag to be the last argument as currently in the patch (which is a 
breaking change for anything which call it).  Or a command line option 
such as --output-cflag could be added to specify the flag and preserve 
command-line syntax compatibility.

Thoughts?

Kevin
_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH 0/9] configurator: Support for Windows and MSVC

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 2348 bytes --]

On Tue, Sep 20, 2016 at 12:21:25AM -0600, Kevin Locke wrote:
> Hi David,
> 
> Thank you for taking the time to review these patches, especially for a
> platform that you don't have a strong stake in.  I really appreciate it.
> 
> On 09/19/2016 10:48 PM, David Gibson wrote:
> > On Sun, Sep 18, 2016 at 06:51:57PM -0600, Kevin Locke wrote:
> > > This patch series adds support for building configurator with Microsoft
> > > Visual C++ and running it on Windows.  The motivation is to add
> > > sufficient support for Windows to allow using header-only modules
> > > with minimal hassle.  For me, this patch series is sufficient, so I
> > > don't have any further plans for changes to other files or the build
> > > system at the moment.
> > 
> > First, thanks for this.  ccan isn't supposed to be platform specific,
> > but it kind of is at the moment, simply because most of the
> > contributions have been from Linux developers.  I'm really happy to
> > see some work to make it useful on different platforms.
> > 
> > Second, because I'm not and have never been a Windows dev, my ability
> > to review these is limited - so I'm inclined to apply anything that
> > doesn't break builds on Linux / POSIX.
> > 
> > Third, because most of the main ccan contributors are not Windows
> > devs, I'm a bit worried about bitrot.  If you can think of any way of
> > making some kind of automated smoke test form builds on Windows, that
> > would be terrific.  Otherwise, I guess we'll just hope for the best.
> 
> I think that's a very legitimate concern.  I'm not a frequent Windows
> developer myself, but had a need for a current project.  I've tried to
> minimize the amount of Windows-specific code, with good results I think, but
> that's obviously not much of a guarantee that it won't rot or subtly break
> on Windows going forward.
> 
> I've seen appveyor.com used quite often for Windows CI, and it is free for
> FOSS projects.  I'd be happy to work up some basic smoke tests if it seems
> like that might be useful and not too much of a maintenance burden.  Let me
> know what you think.

Sounds good to me.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH 2/9] configurator: Reimplement run using popen

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 2110 bytes --]

On Tue, Sep 20, 2016 at 12:22:41AM -0600, Kevin Locke wrote:
> On 09/19/2016 11:00 PM, David Gibson wrote:
> > On Sun, Sep 18, 2016 at 06:51:59PM -0600, Kevin Locke wrote:
> >> Rather than using fork+pipe+system+waitpid, most of which are only
> >> available on POSIX-like systems, use popen which is also available on
> >> Windows (under the name _popen).
> >
> > Concept looks good, one little wart.
> >
> >> [...]
> >>
> >> -static char *grab_fd(int fd)
> >> +static char *grab_stream(FILE *file)
> >>  {
> >> -	int ret;
> >> -	size_t max, size = 0;
> >> +	size_t max, ret, size = 0;
> >>  	char *buffer;
> >>
> >> -	max = 16384;
> >> -	buffer = malloc(max+1);
> >> -	while ((ret = read(fd, buffer + size, max - size)) > 0) {
> >> +	max = BUFSIZ;
> >> +	buffer = malloc(max);
> >> +	while ((ret = fread(buffer+size, 1, max - size, file)) == max - size) {
> >
> > This assumes that fread() will never return a short read except on EOF
> > or error.  I expect that will be true in practice for regular files,
> > but I'm not sure if it's a good idea to rely on it.
> 
> Interesting.  I was under the impression that the fread couldn't short read.
> POSIX describes the return value as "the number of elements successfully
> read which is less than nitems only if a read error or end-of-file is
> encountered."[1]  Now that I think about it, I suppose EINTR could be an
> issue for non-glibc systems.  Is that what you had in mind, or are there
> other issues I'm overlooking?  (Also for fwrite in 6/9?)

Ah..so it does.  I'm afraid I just looked at the Linux man page, not
the POSIX requirements, and it didn't mention this constraint.

I guess it's ok then.  Although.. I do wonder a bit whether we can
trust all implementations to actually comply with this requirement.

> 
> Kevin
> 
> 1.  http://pubs.opengroup.org/onlinepubs/9699919799/functions/fread.html
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH 8/9] configurator: Pass output cflag to configurator

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 3064 bytes --]

On Tue, Sep 20, 2016 at 12:22:47AM -0600, Kevin Locke wrote:
> On 09/19/2016 11:23 PM, David Gibson wrote:
> > On Sun, Sep 18, 2016 at 06:52:05PM -0600, Kevin Locke wrote:
> > > Unfortunately, not all compilers support -o as a command-line option for
> > > specifying the output file.  Visual Studio cl.exe issues warning D9035
> > > when -o is given, which is detected as a compile warning by the
> > > configurator.
> > > 
> > > To support such compilers, pass the output flag as the last flag to the
> > > configurator.
> > > 
> > > This is a breaking change.  Existing scripts which pass flags to the
> > > compiler must be modified to add "-o" as the last flag.  As noted in the
> > > cover letter for this patch series, I'm open to considering alternatives
> > > if this is unacceptable.
> > 
> > So, as it stands, this change completely breaks ccanlint on POSIX,
> > which is not ok.  Specifically it looks like the problem is that the
> > DEFAULT_FLAGS from the configurator make it into CCAN_CFLAGS in
> > config.h.  ccanlint then tries to add further options and its own -o
> > to the end of that, which isn't going to work.
> > 
> > In short, I think the problem is that ccanlint needs fixes to work
> > with MSVC (and possibly anything much else apart from gcc).  I think
> > we need to look at how to fix that and the configurator together with
> > each other.
> > 
> > Not having working ccanlint on Windows seems like a pretty big, since
> > its the main tool for testing ccan, amongst other things.
> 
> Good catch.  I hadn't noticed the ccanlint breakage.  That was a big
> oversight on my part.
> 
> The breakage likely extends to any other programs which use CCAN_CFLAGS,
> since none are expecting the output flag at the end.  Perhaps a better
> solution would be to emit the output flag as a separate macro. Something
> like CCAN_OUTPUT_CFLAG.  What do you think?

I think that's a better option, and maybe the way to go in the short
term.

Longer term, I'm wondering if we want some sort of mini-library for
shared use by the ccan tools - configurator, ccanlint, etc.  Amongst
other things that could have an "invoke compiler" function which
abstracts this sort of thing in a more general way.

Of course, this borders on the questions about use of err() etc. in
the configurator - within the tools themselves, what can we and can't
we assume about the system.

> The command line API for configurator could either expect the output flag to
> be the last argument as currently in the patch (which is a breaking change
> for anything which call it).  Or a command line option such as
> --output-cflag could be added to specify the flag and preserve command-line
> syntax compatibility.
> 
> Thoughts?

--output-cflag is kind of clunky, but again might be our best bet in
the short term.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH 0/9] configurator: Support for Windows and MSVC

[Daniel Burke]

[-- Attachment #1.1: Type: text/plain, Size: 3585 bytes --]

I do have a bit of experience with windows/linux C code interop, and while
Visual C++ tends to be somewhat... unpredictable (It's getting much better
than it used to be), I can tell you that...

x86_64-w64-mingw32-gcc

will output binaries that will happily run on windows, so long as you're
using a version at least as new as is provided with Debian Jessie. (the
wheezy version produced broken binaries). Wine can then be used to test
them quite reliably. That compiler comes with enough windows libraries and
headers to do most reasonable jobs, and quite a few unix headers mangled to
work in windows environments.

sudo apt-get install mingw-w64

There's a 32bit version if you're that way inclined (which is also broken
on wheezy), although I haven't used it recently enough to recall it.

regards,

dan
--
"The world is full of magical things patiently waiting for our wits to grow
sharper." - Bertrand Russell

On 20 September 2016 at 21:55, David Gibson <david@gibson.dropbear.id.au>
wrote:

> On Tue, Sep 20, 2016 at 12:21:25AM -0600, Kevin Locke wrote:
> > Hi David,
> >
> > Thank you for taking the time to review these patches, especially for a
> > platform that you don't have a strong stake in.  I really appreciate it.
> >
> > On 09/19/2016 10:48 PM, David Gibson wrote:
> > > On Sun, Sep 18, 2016 at 06:51:57PM -0600, Kevin Locke wrote:
> > > > This patch series adds support for building configurator with
> Microsoft
> > > > Visual C++ and running it on Windows.  The motivation is to add
> > > > sufficient support for Windows to allow using header-only modules
> > > > with minimal hassle.  For me, this patch series is sufficient, so I
> > > > don't have any further plans for changes to other files or the build
> > > > system at the moment.
> > >
> > > First, thanks for this.  ccan isn't supposed to be platform specific,
> > > but it kind of is at the moment, simply because most of the
> > > contributions have been from Linux developers.  I'm really happy to
> > > see some work to make it useful on different platforms.
> > >
> > > Second, because I'm not and have never been a Windows dev, my ability
> > > to review these is limited - so I'm inclined to apply anything that
> > > doesn't break builds on Linux / POSIX.
> > >
> > > Third, because most of the main ccan contributors are not Windows
> > > devs, I'm a bit worried about bitrot.  If you can think of any way of
> > > making some kind of automated smoke test form builds on Windows, that
> > > would be terrific.  Otherwise, I guess we'll just hope for the best.
> >
> > I think that's a very legitimate concern.  I'm not a frequent Windows
> > developer myself, but had a need for a current project.  I've tried to
> > minimize the amount of Windows-specific code, with good results I think,
> but
> > that's obviously not much of a guarantee that it won't rot or subtly
> break
> > on Windows going forward.
> >
> > I've seen appveyor.com used quite often for Windows CI, and it is free
> for
> > FOSS projects.  I'd be happy to work up some basic smoke tests if it
> seems
> > like that might be useful and not too much of a maintenance burden.  Let
> me
> > know what you think.
>
> Sounds good to me.
>
> --
> David Gibson                    | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_
> _other_
>                                 | _way_ _around_!
> http://www.ozlabs.org/~dgibson
>
> _______________________________________________
> ccan mailing list
> ccan@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/ccan
>
>

[-- Attachment #1.2: Type: text/html, Size: 5042 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH 0/9] configurator: Support for Windows and MSVC

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 1421 bytes --]

On Tue, Sep 20, 2016 at 11:46:23PM +0930, Daniel Burke wrote:
> I do have a bit of experience with windows/linux C code interop, and while
> Visual C++ tends to be somewhat... unpredictable (It's getting much better
> than it used to be), I can tell you that...
> 
> x86_64-w64-mingw32-gcc
> 
> will output binaries that will happily run on windows, so long as you're
> using a version at least as new as is provided with Debian Jessie. (the
> wheezy version produced broken binaries). Wine can then be used to test
> them quite reliably. That compiler comes with enough windows libraries and
> headers to do most reasonable jobs, and quite a few unix headers mangled to
> work in windows environments.
> 
> sudo apt-get install mingw-w64
> 
> There's a 32bit version if you're that way inclined (which is also broken
> on wheezy), although I haven't used it recently enough to recall it.

Thanks for the info.  We should try to get some mingw builds and wine
tests into Travis (although at the moment the Travis builds are
already broken due to a problem on clang I haven't had a chance to
debug :).

However, as noted elsewhere, mingw shouldn't be our only or primary
way of building for windows.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v2 00/13] configurator: Support for Windows and MSVC

[Kevin Locke]

Hi all,

This is the second revision of my patch series which adds support for
building configurator with Microsoft Visual C++ and running it on
Windows.  The motivation is to add sufficient support for Windows to
allow using header-only modules with minimal hassle.

The major change in this revision is the handling of the output cflag,
which can now be specified by a command-line option to configurator and
is set in a separate config.h macro.  This both fixes the ccanlint
breakage in the previous series and preserves backwards-compatibility
for any code calling configurator or using CCAN_CFLAGS.

Patches which need special attention:

As before, patch 3 adds a copy of the err.h functions used by
configurator to work around the lack of err.h on Windows.  If this is
too much code to lump into configurator.c, I can split it out (and copy
err.{c,h} verbatim from musl) or call fprintf+exit directly or consider
modifying the ccan/err module for use by configurator.

Patch 8 changes the configurator API by adding a command-line option and
config.h macro.  The names are different than I had previously
suggested, as noted in the commit message.

Patch 13 adds appveyor.yml for AppVeyor CI.  This will require (free)
account setup on ci.appveyor.com once merged to enable automatic
building and GitHub integration (if desired).  You may also wish to
consider whether to add a README badge or email notification or other
mechanism to be notified of build status (beyond the default pull
request hooks).

Thanks for considering once again,
Kevin


Kevin Locke (13):
  configurator: Replace unlink with remove
  configurator: Reimplement run using popen
  configurator: Inline err.h functions from musl libc
  configurator: Use native directory separator
  configurator: Mark non-Windows tests MAY_NOT_COMPILE
  configurator: Print test source without cat
  configurator: Fix compiler warning with compare
  configurator: Add output cflag option and macro
  configurator: DEFAULT_{COMPILER,FLAGS} for MSVC
  configurator: Fix warning in HAVE_FOR_LOOP_DECLARATION
  configurator: Return pointer difference as ptrdiff_t
  Makefile: Define CFLAGS_FORCE_C_SOURCE macro
  Add appveyor.yml

 Makefile                          |   2 +-
 Makefile-ccan                     |   1 +
 appveyor.yml                      |  25 ++++
 tools/compile.c                   |  15 +-
 tools/configurator/configurator.c | 288 +++++++++++++++++++++++++++-----------
 tools/tools.h                     |   6 +-
 6 files changed, 249 insertions(+), 88 deletions(-)
 create mode 100644 appveyor.yml

--
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v2 01/13] configurator: Replace unlink with remove

[Kevin Locke]

Although Windows provides unlink, using it adds complication due to the
lack of unistd.h which must be included to define the function on POSIX
systems.  Instead, use remove, which C89 requires to be in stdio.h.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 tools/configurator/configurator.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index 67dadc6..e322c76 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -639,8 +639,8 @@ int main(int argc, const char *argv[])
 		run_test(cmd, &tests[i]);
 	free(cmd);
 
-	unlink(OUTPUT_FILE);
-	unlink(INPUT_FILE);
+	remove(OUTPUT_FILE);
+	remove(INPUT_FILE);
 
 	printf("/* Generated by CCAN configurator */\n"
 	       "#ifndef CCAN_CONFIG_H\n"
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v2 02/13] configurator: Reimplement run using popen

[Kevin Locke]

Rather than using fork+pipe+system+waitpid, most of which are only
available on POSIX-like systems, use popen which is also available on
Windows (under the name _popen).

Changes since v1:
- Create fread_noeintr to avoid EINTR in fread without reading any data.
- Handle short reads from fread.  This can happen with non-conformant
  libc or if EINTR occurs after reading some data.
- Define _POSIX_C_SOURCE for popen/pclose with strict implementations
  which require it (e.g. gcc with -std=c11).

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/configurator/configurator.c | 90 +++++++++++++++++++--------------------
 1 file changed, 44 insertions(+), 46 deletions(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index e322c76..4eed188 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -21,17 +21,20 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#define _POSIX_C_SOURCE 200809L		/* For pclose, popen, strdup */
+
+#include <errno.h>
 #include <stdio.h>
 #include <stdbool.h>
 #include <stdlib.h>
-#include <unistd.h>
 #include <err.h>
-#include <sys/types.h>
-#include <sys/wait.h>
-#include <sys/stat.h>
-#include <fcntl.h>
 #include <string.h>
 
+#ifdef _MSC_VER
+#define popen _popen
+#define pclose _pclose
+#endif
+
 #define DEFAULT_COMPILER "cc"
 #define DEFAULT_FLAGS "-g3 -ggdb -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition"
 
@@ -367,20 +370,33 @@ static struct test tests[] = {
 	},
 };
 
-static char *grab_fd(int fd)
+static size_t fread_noeintr(void *ptr, size_t size, size_t nitems,
+		FILE *stream)
+{
+	size_t ret;
+
+	do {
+		errno = 0;
+		ret = fread(ptr, size, nitems, stream);
+	} while (ret == 0 && errno == EINTR);
+
+	return ret;
+}
+
+static char *grab_stream(FILE *file)
 {
-	int ret;
-	size_t max, size = 0;
+	size_t max, ret, size = 0;
 	char *buffer;
 
-	max = 16384;
-	buffer = malloc(max+1);
-	while ((ret = read(fd, buffer + size, max - size)) > 0) {
+	max = BUFSIZ;
+	buffer = malloc(max);
+	while ((ret = fread_noeintr(buffer + size, 1, max - size, file)) > 0) {
 		size += ret;
 		if (size == max)
 			buffer = realloc(buffer, max *= 2);
 	}
-	if (ret < 0)
+	size += ret;
+	if (ferror(file))
 		err(1, "reading from command");
 	buffer[size] = '\0';
 	return buffer;
@@ -388,43 +404,25 @@ static char *grab_fd(int fd)
 
 static char *run(const char *cmd, int *exitstatus)
 {
-	pid_t pid;
-	int p[2];
+	static const char redir[] = " 2>&1";
+	size_t cmdlen;
+	char *cmdredir;
+	FILE *cmdout;
 	char *ret;
-	int status;
 
-	if (pipe(p) != 0)
-		err(1, "creating pipe");
-
-	pid = fork();
-	if (pid == -1)
-		err(1, "forking");
-
-	if (pid == 0) {
-		if (dup2(p[1], STDOUT_FILENO) != STDOUT_FILENO
-		    || dup2(p[1], STDERR_FILENO) != STDERR_FILENO
-		    || close(p[0]) != 0
-		    || close(STDIN_FILENO) != 0
-		    || open("/dev/null", O_RDONLY) != STDIN_FILENO)
-			exit(128);
-
-		status = system(cmd);
-		if (WIFEXITED(status))
-			exit(WEXITSTATUS(status));
-		/* Here's a hint... */
-		exit(128 + WTERMSIG(status));
-	}
+	cmdlen = strlen(cmd);
+	cmdredir = malloc(cmdlen + sizeof(redir));
+	memcpy(cmdredir, cmd, cmdlen);
+	memcpy(cmdredir + cmdlen, redir, sizeof(redir));
+
+	cmdout = popen(cmdredir, "r");
+	if (!cmdout)
+		err(1, "popen \"%s\"", cmdredir);
+
+	free(cmdredir);
 
-	close(p[1]);
-	ret = grab_fd(p[0]);
-	/* This shouldn't fail... */
-	if (waitpid(pid, &status, 0) != pid)
-		err(1, "Failed to wait for child");
-	close(p[0]);
-	if (WIFEXITED(status))
-		*exitstatus = WEXITSTATUS(status);
-	else
-		*exitstatus = -WTERMSIG(status);
+	ret = grab_stream(cmdout);
+	*exitstatus = pclose(cmdout);
 	return ret;
 }
 
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v2 03/13] configurator: Inline err.h functions from musl libc

[Kevin Locke]

Since we can't use HAVE_ERR_H in configurator, provide a definition for
the err.h functions used.  The version provided is the one from musl
libc, since it is concise and shares the MIT License with configurator.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/configurator/configurator.c | 54 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index 4eed188..5929355 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -3,6 +3,9 @@
  *
  * Copyright 2011 Rusty Russell <rusty@rustcorp.com.au>.  MIT license.
  *
+ * err, errx, verr, verrx, vwarn, vwarnx functions from musl libc
+ * Copyright 2005-2013 Rich Felker.  MIT license.
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
  * in the Software without restriction, including without limitation the rights
@@ -25,9 +28,9 @@
 
 #include <errno.h>
 #include <stdio.h>
+#include <stdarg.h>
 #include <stdbool.h>
 #include <stdlib.h>
-#include <err.h>
 #include <string.h>
 
 #ifdef _MSC_VER
@@ -41,6 +44,7 @@
 #define OUTPUT_FILE "configurator.out"
 #define INPUT_FILE "configuratortest.c"
 
+static const char *progname = "";
 static int verbose;
 
 enum test_style {
@@ -370,6 +374,51 @@ static struct test tests[] = {
 	},
 };
 
+static void vwarn(const char *fmt, va_list ap)
+{
+	fprintf (stderr, "%s: ", progname);
+	if (fmt) {
+		vfprintf(stderr, fmt, ap);
+		fputs (": ", stderr);
+	}
+	perror(0);
+}
+
+static void vwarnx(const char *fmt, va_list ap)
+{
+	fprintf (stderr, "%s: ", progname);
+	if (fmt) vfprintf(stderr, fmt, ap);
+	putc('\n', stderr);
+}
+
+static void verr(int status, const char *fmt, va_list ap)
+{
+	vwarn(fmt, ap);
+	exit(status);
+}
+
+static void verrx(int status, const char *fmt, va_list ap)
+{
+	vwarnx(fmt, ap);
+	exit(status);
+}
+
+static void err(int status, const char *fmt, ...)
+{
+	va_list ap;
+	va_start(ap, fmt);
+	verr(status, fmt, ap);
+	va_end(ap);
+}
+
+static void errx(int status, const char *fmt, ...)
+{
+	va_list ap;
+	va_start(ap, fmt);
+	verrx(status, fmt, ap);
+	va_end(ap);
+}
+
 static size_t fread_noeintr(void *ptr, size_t size, size_t nitems,
 		FILE *stream)
 {
@@ -610,6 +659,9 @@ int main(int argc, const char *argv[])
 	const char *default_args[]
 		= { "", DEFAULT_COMPILER, DEFAULT_FLAGS, NULL };
 
+	if (argc > 0)
+		progname = argv[0];
+
 	if (argc > 1) {
 		if (strcmp(argv[1], "--help") == 0) {
 			printf("Usage: configurator [-v] [<compiler> <flags>...]\n"
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v2 04/13] configurator: Use native directory separator

[Kevin Locke]

Although Windows APIs generally permit "/" or "\\" for directory
separators in paths, cmd.exe does not recognize "./" when invoking
executables using a relative path and prints the following error:

    '.' is not recognized as an internal or external command,
    operable program or batch file.

Therefore, use "\\" when invoking tests on Windows.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 tools/configurator/configurator.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index 5929355..7c4dd4a 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -44,6 +44,12 @@
 #define OUTPUT_FILE "configurator.out"
 #define INPUT_FILE "configuratortest.c"
 
+#ifdef _WIN32
+#define DIR_SEP   "\\"
+#else
+#define DIR_SEP   "/"
+#endif
+
 static const char *progname = "";
 static int verbose;
 
@@ -632,7 +638,7 @@ static bool run_test(const char *cmd, struct test *test)
 		free(output);
 		/* We run INSIDE_MAIN tests for sanity checking. */
 		if ((test->style & EXECUTE) || (test->style & INSIDE_MAIN)) {
-			output = run("./" OUTPUT_FILE, &status);
+			output = run("." DIR_SEP OUTPUT_FILE, &status);
 			if (!(test->style & EXECUTE) && status != 0)
 				errx(1, "Test for %s failed with %i:\n%s",
 				     test->name, status, output);
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v2 05/13] configurator: Mark non-Windows tests MAY_NOT_COMPILE

[Kevin Locke]

Several of the EXECUTABLE tests depend on headers not available on
Windows.  Mark these tests MAY_NOT_COMPILE to handle this.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 tools/configurator/configurator.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index 7c4dd4a..31e3d11 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -75,7 +75,7 @@ struct test {
 };
 
 static struct test tests[] = {
-	{ "HAVE_32BIT_OFF_T", DEFINES_EVERYTHING|EXECUTE, NULL, NULL,
+	{ "HAVE_32BIT_OFF_T", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE, NULL, NULL,
 	  "#include <sys/types.h>\n"
 	  "int main(void) {\n"
 	  "	return sizeof(off_t) == 4 ? 0 : 1;\n"
@@ -180,7 +180,7 @@ static struct test tests[] = {
 	{ "HAVE_COMPOUND_LITERALS", INSIDE_MAIN, NULL, NULL,
 	  "int *foo = (int[]) { 1, 2, 3, 4 };\n"
 	  "return foo[0] ? 0 : 1;" },
-	{ "HAVE_FCHDIR", DEFINES_EVERYTHING|EXECUTE, NULL, NULL,
+	{ "HAVE_FCHDIR", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE, NULL, NULL,
 	  "#include <sys/types.h>\n"
 	  "#include <sys/stat.h>\n"
 	  "#include <fcntl.h>\n"
@@ -201,7 +201,7 @@ static struct test tests[] = {
 	  "	if (arg == 4)\n"
 	  "		warnx(\"warn %u\", arg);\n"
 	  "}\n" },
-	{ "HAVE_FILE_OFFSET_BITS", DEFINES_EVERYTHING|EXECUTE,
+	{ "HAVE_FILE_OFFSET_BITS", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE,
 	  "HAVE_32BIT_OFF_T", NULL,
 	  "#define _FILE_OFFSET_BITS 64\n"
 	  "#include <sys/types.h>\n"
@@ -247,7 +247,7 @@ static struct test tests[] = {
 	  "static void *func(int fd) {\n"
 	  "	return mmap(0, 65536, PROT_READ, MAP_SHARED, fd, 0);\n"
 	  "}" },
-	{ "HAVE_PROC_SELF_MAPS", DEFINES_EVERYTHING|EXECUTE, NULL, NULL,
+	{ "HAVE_PROC_SELF_MAPS", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE, NULL, NULL,
 	  "#include <sys/types.h>\n"
 	  "#include <sys/stat.h>\n"
 	  "#include <fcntl.h>\n"
@@ -332,7 +332,7 @@ static struct test tests[] = {
 	  "-Werror -fopenmp" },
 	{ "HAVE_VALGRIND_MEMCHECK_H", OUTSIDE_MAIN, NULL, NULL,
 	  "#include <valgrind/memcheck.h>\n" },
-	{ "HAVE_UCONTEXT", DEFINES_EVERYTHING|EXECUTE,
+	{ "HAVE_UCONTEXT", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE,
 	  NULL, NULL,
 	  "#include <ucontext.h>\n"
 	  "static int x = 0;\n"
@@ -353,7 +353,7 @@ static struct test tests[] = {
 	  "	return (x == 3) ? 0 : 1;\n"
 	  "}\n"
 	},
-	{ "HAVE_POINTER_SAFE_MAKECONTEXT", DEFINES_EVERYTHING|EXECUTE,
+	{ "HAVE_POINTER_SAFE_MAKECONTEXT", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE,
 	  "HAVE_UCONTEXT", NULL,
 	  "#include <stddef.h>\n"
 	  "#include <ucontext.h>\n"
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v2 06/13] configurator: Print test source without cat

[Kevin Locke]

Windows does not provide cat.  Instead, copy the test source to stdout
using the file stream to which it was written.

Changes since v1:
- Create fwrite_noeintr to avoid EINTR in fwrite without writing any
  data.
- Handle short reads from fread.  This can happen with non-conformant
  libc or if EINTR occurs after reading some data.
- Handle short writes from fwrite.  This can happen with non-conformant
  libc or if EINTR occurs after writing some data.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/configurator/configurator.c | 48 +++++++++++++++++++++++++++++++++++----
 1 file changed, 43 insertions(+), 5 deletions(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index 31e3d11..51d7ac8 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -438,6 +438,42 @@ static size_t fread_noeintr(void *ptr, size_t size, size_t nitems,
 	return ret;
 }
 
+static size_t fwrite_noeintr(const void *ptr, size_t size, size_t nitems,
+		FILE *stream)
+{
+	size_t ret;
+
+	do {
+		errno = 0;
+		ret = fwrite(ptr, size, nitems, stream);
+	} while (ret == 0 && errno == EINTR);
+
+	return ret;
+}
+
+static size_t fcopy(FILE *fsrc, FILE *fdst)
+{
+	char buffer[BUFSIZ];
+	size_t copied = 0, rsize;
+
+	while ((rsize = fread_noeintr(buffer, 1, BUFSIZ, fsrc)) > 0) {
+		size_t wsize, wtotal = 0;
+
+		while (wtotal < rsize &&
+		       ((wsize = fwrite_noeintr(buffer + wtotal, 1,
+						rsize - wtotal, fdst)) > 0)) {
+			wtotal += wsize;
+		}
+
+		if (wtotal < rsize)
+			break;
+
+		copied += wtotal;
+	}
+
+	return copied;
+}
+
 static char *grab_stream(FILE *file)
 {
 	size_t max, ret, size = 0;
@@ -562,7 +598,7 @@ static bool run_test(const char *cmd, struct test *test)
 		}
 	}
 
-	outf = fopen(INPUT_FILE, "w");
+	outf = fopen(INPUT_FILE, verbose > 1 ? "w+" : "w");
 	if (!outf)
 		err(1, "creating %s", INPUT_FILE);
 
@@ -593,11 +629,13 @@ static bool run_test(const char *cmd, struct test *test)
 		abort();
 
 	}
-	fclose(outf);
 
-	if (verbose > 1)
-		if (system("cat " INPUT_FILE) == -1)
-			;
+	if (verbose > 1) {
+		fseek(outf, 0, SEEK_SET);
+		fcopy(outf, stdout);
+	}
+
+	fclose(outf);
 
 	newcmd = strdup(cmd);
 
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v2 07/13] configurator: Fix compiler warning with compare

[Kevin Locke]

Visual Studio prints warning C4706 "assignment within conditional
expression" when there is an assignment without a comparison in a
conditional expression.  Therefore, to silence the warning, add an
explicit comparison.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 tools/configurator/configurator.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index 51d7ac8..acf6cc0 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -572,7 +572,7 @@ static bool run_test(const char *cmd, struct test *test)
 		char *dep;
 
 		/* Space-separated dependencies, could be ! for inverse. */
-		while ((len = strcspn(deps, " "))) {
+		while ((len = strcspn(deps, " ")) != 0) {
 			bool positive = true;
 			if (deps[len]) {
 				dep = strdup(deps);
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v2 08/13] configurator: Add output cflag option and macro

[Kevin Locke]

Unfortunately, not all compilers support -o as a command-line option for
specifying the output file.  Visual Studio cl.exe issues warning D9035
when -o is given, which is detected as a compile warning by the
configurator.

To support such compilers, add the command-line option -O to
configurator which can be used to specify the cflag for setting the
output executable file name.  Additionally define the macro
CCAN_OUTPUT_EXE_CFLAG in config.h and use it when invoking the compiler
(e.g.  from ccanlint).

For reference, the name CCAN_OUTPUT_EXE_CFLAG was chosen to avoid
potential name conflicts in the future due to cl.exe requiring different
flags for different types of output[1] (e.g. object files are /Fo:).

1.  https://msdn.microsoft.com/en-us/library/f1cb223a.aspx

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/compile.c                   | 15 ++++++++----
 tools/configurator/configurator.c | 50 +++++++++++++++++++++++++++------------
 tools/tools.h                     |  6 +++--
 3 files changed, 49 insertions(+), 22 deletions(-)

diff --git a/tools/compile.c b/tools/compile.c
index b88c1d8..e796e3f 100644
--- a/tools/compile.c
+++ b/tools/compile.c
@@ -7,8 +7,12 @@
 #ifndef CCAN_CFLAGS
 #define CCAN_CFLAGS DEFAULT_CCAN_CFLAGS
 #endif
+#ifndef CCAN_OUTPUT_EXE_CFLAG
+#define CCAN_OUTPUT_EXE_CFLAG DEFAULT_CCAN_OUTPUT_EXE_CFLAG
+#endif
 const char *compiler = CCAN_COMPILER;
 const char *cflags = CCAN_CFLAGS;
+const char *outexecflag = CCAN_OUTPUT_EXE_CFLAG;
 
 bool compile_verbose = false;
 
@@ -37,8 +41,9 @@ bool compile_object(const void *ctx, const char *cfile, const char *ccandir,
 	if (compile_verbose)
 		printf("Compiling %s\n", outfile);
 	return run_command(ctx, NULL, output,
-			   "%s %s -I%s -c -o %s %s",
-			   compiler, cflags, ccandir, outfile, cfile);
+			   "%s %s -I%s -c %s%s %s",
+			   compiler, cflags, ccandir,
+			   outexecflag, outfile, cfile);
 }
 
 /* Compile and link single C file, with object files.
@@ -51,7 +56,7 @@ bool compile_and_link(const void *ctx, const char *cfile, const char *ccandir,
 	if (compile_verbose)
 		printf("Compiling and linking %s\n", outfile);
 	return run_command(ctx, NULL, output,
-			   "%s %s -I%s -o %s %s %s %s",
-			   compiler, cflags,
-			   ccandir, outfile, cfile, objs, libs);
+			   "%s %s -I%s %s%s %s %s %s",
+			   compiler, cflags, ccandir,
+			   outexecflag, outfile, cfile, objs, libs);
 }
diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index acf6cc0..d81f878 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -40,6 +40,7 @@
 
 #define DEFAULT_COMPILER "cc"
 #define DEFAULT_FLAGS "-g3 -ggdb -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition"
+#define DEFAULT_OUTPUT_EXE_FLAG "-o"
 
 #define OUTPUT_FILE "configurator.out"
 #define INPUT_FILE "configuratortest.c"
@@ -517,10 +518,12 @@ static char *run(const char *cmd, int *exitstatus)
 	return ret;
 }
 
-static char *connect_args(const char *argv[], const char *extra)
+static char *connect_args(const char *argv[], const char *outflag,
+		const char *files)
 {
-	unsigned int i, len = strlen(extra) + 1;
+	unsigned int i;
 	char *ret;
+	size_t len = strlen(outflag) + strlen(files) + 1;
 
 	for (i = 1; argv[i]; i++)
 		len += 1 + strlen(argv[i]);
@@ -530,10 +533,12 @@ static char *connect_args(const char *argv[], const char *extra)
 	for (i = 1; argv[i]; i++) {
 		strcpy(ret + len, argv[i]);
 		len += strlen(argv[i]);
-		if (argv[i+1])
+		if (argv[i+1] || *outflag)
 			ret[len++] = ' ';
 	}
-	strcpy(ret + len, extra);
+	strcpy(ret + len, outflag);
+	len += strlen(outflag);
+	strcpy(ret + len, files);
 	return ret;
 }
 
@@ -702,33 +707,47 @@ int main(int argc, const char *argv[])
 	unsigned int i;
 	const char *default_args[]
 		= { "", DEFAULT_COMPILER, DEFAULT_FLAGS, NULL };
+	const char *outflag = DEFAULT_OUTPUT_EXE_FLAG;
 
 	if (argc > 0)
 		progname = argv[0];
 
-	if (argc > 1) {
+	while (argc > 1) {
 		if (strcmp(argv[1], "--help") == 0) {
-			printf("Usage: configurator [-v] [<compiler> <flags>...]\n"
-			       "  <compiler> <flags> will have \"-o <outfile> <infile.c>\" appended\n"
-			       "Default: %s %s\n",
-			       DEFAULT_COMPILER, DEFAULT_FLAGS);
+			printf("Usage: configurator [-v] [-O<outflag>] [<compiler> <flags>...]\n"
+			       "  <compiler> <flags> will have \"<outflag> <outfile> <infile.c>\" appended\n"
+			       "Default: %s %s %s\n",
+			       DEFAULT_COMPILER, DEFAULT_FLAGS,
+			       DEFAULT_OUTPUT_EXE_FLAG);
 			exit(0);
 		}
-		if (strcmp(argv[1], "-v") == 0) {
+		if (strncmp(argv[1], "-O", 2) == 0) {
 			argc--;
 			argv++;
-			verbose = 1;
+			outflag = argv[1] + 2;
+			if (!*outflag) {
+				fprintf(stderr,
+					"%s: option requires an argument -- O\n",
+					argv[0]);
+				exit(1);
+			}
+		} else if (strcmp(argv[1], "-v") == 0) {
+			argc--;
+			argv++;
+			verbose++;
 		} else if (strcmp(argv[1], "-vv") == 0) {
 			argc--;
 			argv++;
-			verbose = 2;
+			verbose += 2;
+		} else {
+			break;
 		}
 	}
 
 	if (argc == 1)
 		argv = default_args;
 
-	cmd = connect_args(argv, " -o " OUTPUT_FILE " " INPUT_FILE);
+	cmd = connect_args(argv, outflag, OUTPUT_FILE " " INPUT_FILE);
 	for (i = 0; i < sizeof(tests)/sizeof(tests[0]); i++)
 		run_test(cmd, &tests[i]);
 	free(cmd);
@@ -743,9 +762,10 @@ int main(int argc, const char *argv[])
 	printf("#define _GNU_SOURCE /* Always use GNU extensions. */\n");
 	printf("#endif\n");
 	printf("#define CCAN_COMPILER \"%s\"\n", argv[1]);
-	cmd = connect_args(argv+1, "");
-	printf("#define CCAN_CFLAGS \"%s\"\n\n", cmd);
+	cmd = connect_args(argv + 1, "", "");
+	printf("#define CCAN_CFLAGS \"%s\"\n", cmd);
 	free(cmd);
+	printf("#define CCAN_OUTPUT_EXE_CFLAG \"%s\"\n\n", outflag);
 	/* This one implies "#include <ccan/..." works, eg. for tdb2.h */
 	printf("#define HAVE_CCAN 1\n");
 	for (i = 0; i < sizeof(tests)/sizeof(tests[0]); i++)
diff --git a/tools/tools.h b/tools/tools.h
index 3b5c4be..668c91a 100644
--- a/tools/tools.h
+++ b/tools/tools.h
@@ -11,6 +11,7 @@
 /* These are the defaults. */
 #define DEFAULT_CCAN_COMPILER "cc"
 #define DEFAULT_CCAN_CFLAGS "-g"
+#define DEFAULT_CCAN_OUTPUT_EXE_CFLAG "-o"
 
 #define IDENT_CHARS	"ABCDEFGHIJKLMNOPQRSTUVWXYZ" \
 			"abcdefghijklmnopqrstuvwxyz" \
@@ -20,8 +21,9 @@
 
 #define COVERAGE_CFLAGS "-fprofile-arcs -ftest-coverage"
 
-/* Actual compiler and cflags (defaults to CCAN_COMPILER and CCAN_CFLAGS). */
-extern const char *compiler, *cflags;
+/* Actual compiler and cflags
+ * (defaults to CCAN_COMPILER, CCAN_CFLAGS, CCAN_OUTPUT_EXE_CFLAG). */
+extern const char *compiler, *cflags, *outexecflag;
 
 /* This compiles up the _info file into a temporary. */
 char *compile_info(const void *ctx, const char *dir);
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v2 09/13] configurator: DEFAULT_{COMPILER, FLAGS} for MSVC

[Kevin Locke]

When compiling with Visual Studio, use default compiler name and flags
which are likely to work with the known-available compiler.

This is also a convenience for users who may not know what arguments
cl.exe may need to compile the tests.

Changes since v1:
- Use "-option" instead of "/option" to avoid issues running under msys.
- Disable C4200 warning for use of flexible array members, which MSVC
  considers an extension (since it does not fully support C99).

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/configurator/configurator.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index d81f878..d3e01cd 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -38,9 +38,21 @@
 #define pclose _pclose
 #endif
 
+#ifdef _MSC_VER
+#define DEFAULT_COMPILER "cl"
+/* Note:  Dash options avoid POSIX path conversion when used under msys bash
+ *        and are therefore preferred to slash (e.g. -nologo over /nologo)
+ * Note:  Disable Warning 4200 "nonstandard extension used : zero-sized array
+ *        in struct/union" for flexible array members.
+ */
+#define DEFAULT_FLAGS "-nologo -Zi -W4 -wd4200 " \
+	"-D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS"
+#define DEFAULT_OUTPUT_EXE_FLAG "-Fe:"
+#else
 #define DEFAULT_COMPILER "cc"
 #define DEFAULT_FLAGS "-g3 -ggdb -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition"
 #define DEFAULT_OUTPUT_EXE_FLAG "-o"
+#endif
 
 #define OUTPUT_FILE "configurator.out"
 #define INPUT_FILE "configuratortest.c"
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v2 10/13] configurator: Fix warning in HAVE_FOR_LOOP_DECLARATION

[Kevin Locke]

Visual C++ prints "warning C4702: unreachable code" due to the return
statement after the for loop which is never reached.  Fix this by
setting a variable returned by a single return statement at the end.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/configurator/configurator.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index d3e01cd..a43061b 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -222,8 +222,9 @@ static struct test tests[] = {
 	  "	return sizeof(off_t) == 8 ? 0 : 1;\n"
 	  "}\n" },
 	{ "HAVE_FOR_LOOP_DECLARATION", INSIDE_MAIN, NULL, NULL,
-	  "for (int i = 0; i < argc; i++) { return 0; };\n"
-	  "return 1;" },
+	  "int ret = 1;\n"
+	  "for (int i = 0; i < argc; i++) { ret = 0; };\n"
+	  "return ret;" },
 	{ "HAVE_FLEXIBLE_ARRAY_MEMBER", OUTSIDE_MAIN, NULL, NULL,
 	  "struct foo { unsigned int x; int arr[]; };" },
 	{ "HAVE_GETPAGESIZE", DEFINES_FUNC, NULL, NULL,
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v2 11/13] configurator: Return pointer difference as ptrdiff_t

[Kevin Locke]

On LLP64 systems (like 64-bit Windows) long is 32 bits while pointers
are 64 bits, which results in a warning similar to the following:

warning C4244: 'return': conversion from '__int64' to 'long', possible loss of data

for HAVE_STACK_GROWS_UPWARDS.  Fix this by using the ptrdiff_t type
introduced by C99 for this purpose.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/configurator/configurator.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index a43061b..2acb0b2 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -298,7 +298,8 @@ static struct test tests[] = {
 	  "	return __stop_mysec - __start_mysec;\n"
 	  "}\n" },
 	{ "HAVE_STACK_GROWS_UPWARDS", DEFINES_EVERYTHING|EXECUTE, NULL, NULL,
-	  "static long nest(const void *base, unsigned int i)\n"
+	  "#include <stddef.h>\n"
+	  "static ptrdiff_t nest(const void *base, unsigned int i)\n"
 	  "{\n"
 	  "	if (i == 0)\n"
 	  "		return (const char *)&i - (const char *)base;\n"
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v2 12/13] Makefile: Define CFLAGS_FORCE_C_SOURCE macro

[Kevin Locke]

This macro holds the C compiler flag(s) to force input files to be
recognized as C sources regardless of extension.  It is defined to allow
overriding on the make command line.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 Makefile      | 2 +-
 Makefile-ccan | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 127b875..37ff17d 100644
--- a/Makefile
+++ b/Makefile
@@ -64,7 +64,7 @@ summary-fastcheck/%: tools/ccanlint/ccanlint $(OBJFILES)
 	$(CCANLINT_FAST) -s ccan/$*
 
 ccan/%/info: ccan/%/_info config.h
-	$(CC) $(CCAN_CFLAGS) -I. -o $@ -x c $<
+	$(CC) $(CCAN_CFLAGS) -I. -o $@ $(CFLAGS_FORCE_C_SOURCE) $<
 
 all_info: $(MODS:%=ccan/%/info)
 
diff --git a/Makefile-ccan b/Makefile-ccan
index 5c8dea2..d4a5528 100644
--- a/Makefile-ccan
+++ b/Makefile-ccan
@@ -5,6 +5,7 @@
 #CCAN_CFLAGS=-g -O3 -Wall -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-declarations -Wpointer-arith -Wwrite-strings -Wundef -DCCAN_STR_DEBUG=1
 CCAN_CFLAGS=-g3 -ggdb -Wall -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-declarations -Wpointer-arith -Wwrite-strings -Wundef -DCCAN_STR_DEBUG=1
 CFLAGS = $(CCAN_CFLAGS) -I. $(DEPGEN)
+CFLAGS_FORCE_C_SOURCE = -x c
 
 MODS := a_star \
 	aga \
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v2 13/13] Add appveyor.yml

[Kevin Locke]

This file defines the AppVeyor CI (appveyor.com) settings.

It builds using make+bash under MSYS2 so that the current build system
can be used with minimal changes.  It currently only builds configurator
and generates config.h.

The build and test commands for more thorough testing are left as
comments in appveyor.yml so interested parties can use them as a
starting point for future work.

Note that several compiler errors not related to configurator are
printed due to make attempting to generate and include test-depends.
Although Windows-specific code could be added to Makefile to avoid
these, it seemed unwarranted if the compile errors may be fixed soon.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 appveyor.yml | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 appveyor.yml

diff --git a/appveyor.yml b/appveyor.yml
new file mode 100644
index 0000000..822c4bd
--- /dev/null
+++ b/appveyor.yml
@@ -0,0 +1,25 @@
+version: 0.0.{build}
+
+platform:
+- x86
+- amd64
+
+build_script:
+# Reset %PATH% to avoid msys/cygwin DLL conflicts which cause
+# *** fatal error - cygheap base mismatch detected
+- set PATH=C:\Windows\System32;C:\Windows;C:\Windows\System32\Wbem;C:\msys64\usr\bin
+# Set environment variables for chosen compiler
+- "\"C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\vcvarsall.bat\" %Platform%"
+# Ensure config.h can be generated.
+# Note: Dash options avoid POSIX path conversion by msys
+- "make config.h CC=cl CCAN_CFLAGS=\"-nologo -Zi -FS -W4 -wd4200 -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS\" CFLAGS_FORCE_C_SOURCE=-TC OUTPUT_OPTION=-Fo:$@ DEPGEN= LD=link SHELL=bash"
+# FIXME:  Work in progress.  Disabled due to unfixed compile errors.
+# FIXME:  -j%NUMBER_OF_PROCESSORS% won't work without DEPGEN for config.h
+#         It may be possible to generate .d from cl using /showIncludes
+#         See https://stackoverflow.com/q/37685069
+#- "make tools CC=cl CCAN_CFLAGS=\"-nologo -Zi -FS -W4 -wd4200 -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS\" CFLAGS_FORCE_C_SOURCE=-TC OUTPUT_OPTION=-Fo:$@ DEPGEN= LD=link SHELL=bash"
+#- "make -k CC=cl CCAN_CFLAGS=\"-nologo -Zi -FS -W4 -wd4200 -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS\" CFLAGS_FORCE_C_SOURCE=-TC OUTPUT_OPTION=-Fo:$@ DEPGEN= LD=link SHELL=bash"
+
+test_script:
+# FIXME:  Work in progress.  Disabled due to unfixed compile errors.
+#- "make -k check CC=cl CCAN_CFLAGS=\"-nologo -Zi -FS -W4 -wd4200 -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS\" CFLAGS_FORCE_C_SOURCE=-TC OUTPUT_OPTION=-Fo:$@ DEPGEN= LD=link SHELL=bash"
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 01/13] configurator: Replace unlink with remove

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 1289 bytes --]

On Thu, Sep 22, 2016 at 09:33:04PM -0600, Kevin Locke wrote:
> Although Windows provides unlink, using it adds complication due to the
> lack of unistd.h which must be included to define the function on POSIX
> systems.  Instead, use remove, which C89 requires to be in stdio.h.
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

This is sane enough on its own, so I'll apply it to master.

> ---
>  tools/configurator/configurator.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
> index 67dadc6..e322c76 100644
> --- a/tools/configurator/configurator.c
> +++ b/tools/configurator/configurator.c
> @@ -639,8 +639,8 @@ int main(int argc, const char *argv[])
>  		run_test(cmd, &tests[i]);
>  	free(cmd);
>  
> -	unlink(OUTPUT_FILE);
> -	unlink(INPUT_FILE);
> +	remove(OUTPUT_FILE);
> +	remove(INPUT_FILE);
>  
>  	printf("/* Generated by CCAN configurator */\n"
>  	       "#ifndef CCAN_CONFIG_H\n"

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 02/13] configurator: Reimplement run using popen

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 4906 bytes --]

On Thu, Sep 22, 2016 at 09:33:05PM -0600, Kevin Locke wrote:
> Rather than using fork+pipe+system+waitpid, most of which are only
> available on POSIX-like systems, use popen which is also available on
> Windows (under the name _popen).
> 
> Changes since v1:
> - Create fread_noeintr to avoid EINTR in fread without reading any data.
> - Handle short reads from fread.  This can happen with non-conformant
>   libc or if EINTR occurs after reading some data.

Hrm.  Have you actually confirmed this problem with EINTR?  My
previous objections to not handling short read were based entirely on
not realizing that fread() wasn't supposed to do that, rather than
thinking of some edge case you hadn't.

Unless we have a confirmed case where fread() can give a short read
that won't have feof() or ferror() set afterwards, I'm inclined to go
back to your initial implementation.

> - Define _POSIX_C_SOURCE for popen/pclose with strict implementations
>   which require it (e.g. gcc with -std=c11).
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
> ---
>  tools/configurator/configurator.c | 90 +++++++++++++++++++--------------------
>  1 file changed, 44 insertions(+), 46 deletions(-)
> 
> diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
> index e322c76..4eed188 100644
> --- a/tools/configurator/configurator.c
> +++ b/tools/configurator/configurator.c
> @@ -21,17 +21,20 @@
>   * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>   * THE SOFTWARE.
>   */
> +#define _POSIX_C_SOURCE 200809L		/* For pclose, popen, strdup */
> +
> +#include <errno.h>
>  #include <stdio.h>
>  #include <stdbool.h>
>  #include <stdlib.h>
> -#include <unistd.h>
>  #include <err.h>
> -#include <sys/types.h>
> -#include <sys/wait.h>
> -#include <sys/stat.h>
> -#include <fcntl.h>
>  #include <string.h>
>  
> +#ifdef _MSC_VER
> +#define popen _popen
> +#define pclose _pclose
> +#endif
> +
>  #define DEFAULT_COMPILER "cc"
>  #define DEFAULT_FLAGS "-g3 -ggdb -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition"
>  
> @@ -367,20 +370,33 @@ static struct test tests[] = {
>  	},
>  };
>  
> -static char *grab_fd(int fd)
> +static size_t fread_noeintr(void *ptr, size_t size, size_t nitems,
> +		FILE *stream)
> +{
> +	size_t ret;
> +
> +	do {
> +		errno = 0;
> +		ret = fread(ptr, size, nitems, stream);
> +	} while (ret == 0 && errno == EINTR);
> +
> +	return ret;
> +}
> +
> +static char *grab_stream(FILE *file)
>  {
> -	int ret;
> -	size_t max, size = 0;
> +	size_t max, ret, size = 0;
>  	char *buffer;
>  
> -	max = 16384;
> -	buffer = malloc(max+1);
> -	while ((ret = read(fd, buffer + size, max - size)) > 0) {
> +	max = BUFSIZ;
> +	buffer = malloc(max);
> +	while ((ret = fread_noeintr(buffer + size, 1, max - size, file)) > 0) {
>  		size += ret;
>  		if (size == max)
>  			buffer = realloc(buffer, max *= 2);
>  	}
> -	if (ret < 0)
> +	size += ret;
> +	if (ferror(file))
>  		err(1, "reading from command");
>  	buffer[size] = '\0';
>  	return buffer;
> @@ -388,43 +404,25 @@ static char *grab_fd(int fd)
>  
>  static char *run(const char *cmd, int *exitstatus)
>  {
> -	pid_t pid;
> -	int p[2];
> +	static const char redir[] = " 2>&1";
> +	size_t cmdlen;
> +	char *cmdredir;
> +	FILE *cmdout;
>  	char *ret;
> -	int status;
>  
> -	if (pipe(p) != 0)
> -		err(1, "creating pipe");
> -
> -	pid = fork();
> -	if (pid == -1)
> -		err(1, "forking");
> -
> -	if (pid == 0) {
> -		if (dup2(p[1], STDOUT_FILENO) != STDOUT_FILENO
> -		    || dup2(p[1], STDERR_FILENO) != STDERR_FILENO
> -		    || close(p[0]) != 0
> -		    || close(STDIN_FILENO) != 0
> -		    || open("/dev/null", O_RDONLY) != STDIN_FILENO)
> -			exit(128);
> -
> -		status = system(cmd);
> -		if (WIFEXITED(status))
> -			exit(WEXITSTATUS(status));
> -		/* Here's a hint... */
> -		exit(128 + WTERMSIG(status));
> -	}
> +	cmdlen = strlen(cmd);
> +	cmdredir = malloc(cmdlen + sizeof(redir));
> +	memcpy(cmdredir, cmd, cmdlen);
> +	memcpy(cmdredir + cmdlen, redir, sizeof(redir));
> +
> +	cmdout = popen(cmdredir, "r");
> +	if (!cmdout)
> +		err(1, "popen \"%s\"", cmdredir);
> +
> +	free(cmdredir);
>  
> -	close(p[1]);
> -	ret = grab_fd(p[0]);
> -	/* This shouldn't fail... */
> -	if (waitpid(pid, &status, 0) != pid)
> -		err(1, "Failed to wait for child");
> -	close(p[0]);
> -	if (WIFEXITED(status))
> -		*exitstatus = WEXITSTATUS(status);
> -	else
> -		*exitstatus = -WTERMSIG(status);
> +	ret = grab_stream(cmdout);
> +	*exitstatus = pclose(cmdout);
>  	return ret;
>  }
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 03/13] configurator: Inline err.h functions from musl libc

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 3506 bytes --]

On Thu, Sep 22, 2016 at 09:33:06PM -0600, Kevin Locke wrote:
> Since we can't use HAVE_ERR_H in configurator, provide a definition for
> the err.h functions used.  The version provided is the one from musl
> libc, since it is concise and shares the MIT License with configurator.
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>

For me this breaks Linux build, because we get "static declaration
follows non-static" on these functions.

I think we instead want to stop using the err() etc. functions and use
similar functions with a different name that we always define locally.

> ---
>  tools/configurator/configurator.c | 54 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
> index 4eed188..5929355 100644
> --- a/tools/configurator/configurator.c
> +++ b/tools/configurator/configurator.c
> @@ -3,6 +3,9 @@
>   *
>   * Copyright 2011 Rusty Russell <rusty@rustcorp.com.au>.  MIT license.
>   *
> + * err, errx, verr, verrx, vwarn, vwarnx functions from musl libc
> + * Copyright 2005-2013 Rich Felker.  MIT license.
> + *
>   * Permission is hereby granted, free of charge, to any person obtaining a copy
>   * of this software and associated documentation files (the "Software"), to deal
>   * in the Software without restriction, including without limitation the rights
> @@ -25,9 +28,9 @@
>  
>  #include <errno.h>
>  #include <stdio.h>
> +#include <stdarg.h>
>  #include <stdbool.h>
>  #include <stdlib.h>
> -#include <err.h>
>  #include <string.h>
>  
>  #ifdef _MSC_VER
> @@ -41,6 +44,7 @@
>  #define OUTPUT_FILE "configurator.out"
>  #define INPUT_FILE "configuratortest.c"
>  
> +static const char *progname = "";
>  static int verbose;
>  
>  enum test_style {
> @@ -370,6 +374,51 @@ static struct test tests[] = {
>  	},
>  };
>  
> +static void vwarn(const char *fmt, va_list ap)
> +{
> +	fprintf (stderr, "%s: ", progname);
> +	if (fmt) {
> +		vfprintf(stderr, fmt, ap);
> +		fputs (": ", stderr);
> +	}
> +	perror(0);
> +}
> +
> +static void vwarnx(const char *fmt, va_list ap)
> +{
> +	fprintf (stderr, "%s: ", progname);
> +	if (fmt) vfprintf(stderr, fmt, ap);
> +	putc('\n', stderr);
> +}
> +
> +static void verr(int status, const char *fmt, va_list ap)
> +{
> +	vwarn(fmt, ap);
> +	exit(status);
> +}
> +
> +static void verrx(int status, const char *fmt, va_list ap)
> +{
> +	vwarnx(fmt, ap);
> +	exit(status);
> +}
> +
> +static void err(int status, const char *fmt, ...)
> +{
> +	va_list ap;
> +	va_start(ap, fmt);
> +	verr(status, fmt, ap);
> +	va_end(ap);
> +}
> +
> +static void errx(int status, const char *fmt, ...)
> +{
> +	va_list ap;
> +	va_start(ap, fmt);
> +	verrx(status, fmt, ap);
> +	va_end(ap);
> +}
> +
>  static size_t fread_noeintr(void *ptr, size_t size, size_t nitems,
>  		FILE *stream)
>  {
> @@ -610,6 +659,9 @@ int main(int argc, const char *argv[])
>  	const char *default_args[]
>  		= { "", DEFAULT_COMPILER, DEFAULT_FLAGS, NULL };
>  
> +	if (argc > 0)
> +		progname = argv[0];
> +
>  	if (argc > 1) {
>  		if (strcmp(argv[1], "--help") == 0) {
>  			printf("Usage: configurator [-v] [<compiler> <flags>...]\n"

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 04/13] configurator: Use native directory separator

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 1913 bytes --]

On Thu, Sep 22, 2016 at 09:33:07PM -0600, Kevin Locke wrote:
> Although Windows APIs generally permit "/" or "\\" for directory
> separators in paths, cmd.exe does not recognize "./" when invoking
> executables using a relative path and prints the following error:
> 
>     '.' is not recognized as an internal or external command,
>     operable program or batch file.
> 
> Therefore, use "\\" when invoking tests on Windows.
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

This one also looks fine standalone (apart from a trivial context
conflict).  Applying.

> ---
>  tools/configurator/configurator.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
> index 5929355..7c4dd4a 100644
> --- a/tools/configurator/configurator.c
> +++ b/tools/configurator/configurator.c
> @@ -44,6 +44,12 @@
>  #define OUTPUT_FILE "configurator.out"
>  #define INPUT_FILE "configuratortest.c"
>  
> +#ifdef _WIN32
> +#define DIR_SEP   "\\"
> +#else
> +#define DIR_SEP   "/"
> +#endif
> +
>  static const char *progname = "";
>  static int verbose;
>  
> @@ -632,7 +638,7 @@ static bool run_test(const char *cmd, struct test *test)
>  		free(output);
>  		/* We run INSIDE_MAIN tests for sanity checking. */
>  		if ((test->style & EXECUTE) || (test->style & INSIDE_MAIN)) {
> -			output = run("./" OUTPUT_FILE, &status);
> +			output = run("." DIR_SEP OUTPUT_FILE, &status);
>  			if (!(test->style & EXECUTE) && status != 0)
>  				errx(1, "Test for %s failed with %i:\n%s",
>  				     test->name, status, output);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 05/13] configurator: Mark non-Windows tests MAY_NOT_COMPILE

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 3167 bytes --]

On Thu, Sep 22, 2016 at 09:33:08PM -0600, Kevin Locke wrote:
> Several of the EXECUTABLE tests depend on headers not available on
> Windows.  Mark these tests MAY_NOT_COMPILE to handle this.
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

Applying.

> ---
>  tools/configurator/configurator.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
> index 7c4dd4a..31e3d11 100644
> --- a/tools/configurator/configurator.c
> +++ b/tools/configurator/configurator.c
> @@ -75,7 +75,7 @@ struct test {
>  };
>  
>  static struct test tests[] = {
> -	{ "HAVE_32BIT_OFF_T", DEFINES_EVERYTHING|EXECUTE, NULL, NULL,
> +	{ "HAVE_32BIT_OFF_T", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE, NULL, NULL,
>  	  "#include <sys/types.h>\n"
>  	  "int main(void) {\n"
>  	  "	return sizeof(off_t) == 4 ? 0 : 1;\n"
> @@ -180,7 +180,7 @@ static struct test tests[] = {
>  	{ "HAVE_COMPOUND_LITERALS", INSIDE_MAIN, NULL, NULL,
>  	  "int *foo = (int[]) { 1, 2, 3, 4 };\n"
>  	  "return foo[0] ? 0 : 1;" },
> -	{ "HAVE_FCHDIR", DEFINES_EVERYTHING|EXECUTE, NULL, NULL,
> +	{ "HAVE_FCHDIR", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE, NULL, NULL,
>  	  "#include <sys/types.h>\n"
>  	  "#include <sys/stat.h>\n"
>  	  "#include <fcntl.h>\n"
> @@ -201,7 +201,7 @@ static struct test tests[] = {
>  	  "	if (arg == 4)\n"
>  	  "		warnx(\"warn %u\", arg);\n"
>  	  "}\n" },
> -	{ "HAVE_FILE_OFFSET_BITS", DEFINES_EVERYTHING|EXECUTE,
> +	{ "HAVE_FILE_OFFSET_BITS", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE,
>  	  "HAVE_32BIT_OFF_T", NULL,
>  	  "#define _FILE_OFFSET_BITS 64\n"
>  	  "#include <sys/types.h>\n"
> @@ -247,7 +247,7 @@ static struct test tests[] = {
>  	  "static void *func(int fd) {\n"
>  	  "	return mmap(0, 65536, PROT_READ, MAP_SHARED, fd, 0);\n"
>  	  "}" },
> -	{ "HAVE_PROC_SELF_MAPS", DEFINES_EVERYTHING|EXECUTE, NULL, NULL,
> +	{ "HAVE_PROC_SELF_MAPS", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE, NULL, NULL,
>  	  "#include <sys/types.h>\n"
>  	  "#include <sys/stat.h>\n"
>  	  "#include <fcntl.h>\n"
> @@ -332,7 +332,7 @@ static struct test tests[] = {
>  	  "-Werror -fopenmp" },
>  	{ "HAVE_VALGRIND_MEMCHECK_H", OUTSIDE_MAIN, NULL, NULL,
>  	  "#include <valgrind/memcheck.h>\n" },
> -	{ "HAVE_UCONTEXT", DEFINES_EVERYTHING|EXECUTE,
> +	{ "HAVE_UCONTEXT", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE,
>  	  NULL, NULL,
>  	  "#include <ucontext.h>\n"
>  	  "static int x = 0;\n"
> @@ -353,7 +353,7 @@ static struct test tests[] = {
>  	  "	return (x == 3) ? 0 : 1;\n"
>  	  "}\n"
>  	},
> -	{ "HAVE_POINTER_SAFE_MAKECONTEXT", DEFINES_EVERYTHING|EXECUTE,
> +	{ "HAVE_POINTER_SAFE_MAKECONTEXT", DEFINES_EVERYTHING|EXECUTE|MAY_NOT_COMPILE,
>  	  "HAVE_UCONTEXT", NULL,
>  	  "#include <stddef.h>\n"
>  	  "#include <ucontext.h>\n"

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 06/13] configurator: Print test source without cat

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 2815 bytes --]

On Thu, Sep 22, 2016 at 09:33:09PM -0600, Kevin Locke wrote:
> Windows does not provide cat.  Instead, copy the test source to stdout
> using the file stream to which it was written.
> 
> Changes since v1:
> - Create fwrite_noeintr to avoid EINTR in fwrite without writing any
>   data.
> - Handle short reads from fread.  This can happen with non-conformant
>   libc or if EINTR occurs after reading some data.
> - Handle short writes from fwrite.  This can happen with non-conformant
>   libc or if EINTR occurs after writing some data.
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>

As with patch 2, I'm not sure about the need for fwrite_noeintr().

> ---
>  tools/configurator/configurator.c | 48 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 43 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
> index 31e3d11..51d7ac8 100644
> --- a/tools/configurator/configurator.c
> +++ b/tools/configurator/configurator.c
> @@ -438,6 +438,42 @@ static size_t fread_noeintr(void *ptr, size_t size, size_t nitems,
>  	return ret;
>  }
>  
> +static size_t fwrite_noeintr(const void *ptr, size_t size, size_t nitems,
> +		FILE *stream)
> +{
> +	size_t ret;
> +
> +	do {
> +		errno = 0;
> +		ret = fwrite(ptr, size, nitems, stream);
> +	} while (ret == 0 && errno == EINTR);
> +
> +	return ret;
> +}
> +
> +static size_t fcopy(FILE *fsrc, FILE *fdst)
> +{
> +	char buffer[BUFSIZ];
> +	size_t copied = 0, rsize;
> +
> +	while ((rsize = fread_noeintr(buffer, 1, BUFSIZ, fsrc)) > 0) {
> +		size_t wsize, wtotal = 0;
> +
> +		while (wtotal < rsize &&
> +		       ((wsize = fwrite_noeintr(buffer + wtotal, 1,
> +						rsize - wtotal, fdst)) > 0)) {
> +			wtotal += wsize;
> +		}
> +
> +		if (wtotal < rsize)
> +			break;
> +
> +		copied += wtotal;
> +	}
> +
> +	return copied;
> +}
> +
>  static char *grab_stream(FILE *file)
>  {
>  	size_t max, ret, size = 0;
> @@ -562,7 +598,7 @@ static bool run_test(const char *cmd, struct test *test)
>  		}
>  	}
>  
> -	outf = fopen(INPUT_FILE, "w");
> +	outf = fopen(INPUT_FILE, verbose > 1 ? "w+" : "w");
>  	if (!outf)
>  		err(1, "creating %s", INPUT_FILE);
>  
> @@ -593,11 +629,13 @@ static bool run_test(const char *cmd, struct test *test)
>  		abort();
>  
>  	}
> -	fclose(outf);
>  
> -	if (verbose > 1)
> -		if (system("cat " INPUT_FILE) == -1)
> -			;
> +	if (verbose > 1) {
> +		fseek(outf, 0, SEEK_SET);
> +		fcopy(outf, stdout);
> +	}
> +
> +	fclose(outf);
>  
>  	newcmd = strdup(cmd);
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 07/13] configurator: Fix compiler warning with compare

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 1273 bytes --]

On Thu, Sep 22, 2016 at 09:33:10PM -0600, Kevin Locke wrote:
> Visual Studio prints warning C4706 "assignment within conditional
> expression" when there is an assignment without a comparison in a
> conditional expression.  Therefore, to silence the warning, add an
> explicit comparison.
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

Applying.

> ---
>  tools/configurator/configurator.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
> index 51d7ac8..acf6cc0 100644
> --- a/tools/configurator/configurator.c
> +++ b/tools/configurator/configurator.c
> @@ -572,7 +572,7 @@ static bool run_test(const char *cmd, struct test *test)
>  		char *dep;
>  
>  		/* Space-separated dependencies, could be ! for inverse. */
> -		while ((len = strcspn(deps, " "))) {
> +		while ((len = strcspn(deps, " ")) != 0) {
>  			bool positive = true;
>  			if (deps[len]) {
>  				dep = strdup(deps);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 10/13] configurator: Fix warning in HAVE_FOR_LOOP_DECLARATION

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 1393 bytes --]

On Thu, Sep 22, 2016 at 09:33:13PM -0600, Kevin Locke wrote:
> Visual C++ prints "warning C4702: unreachable code" due to the return
> statement after the for loop which is never reached.  Fix this by
> setting a variable returned by a single return statement at the end.
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>

Applying.

> ---
>  tools/configurator/configurator.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
> index d3e01cd..a43061b 100644
> --- a/tools/configurator/configurator.c
> +++ b/tools/configurator/configurator.c
> @@ -222,8 +222,9 @@ static struct test tests[] = {
>  	  "	return sizeof(off_t) == 8 ? 0 : 1;\n"
>  	  "}\n" },
>  	{ "HAVE_FOR_LOOP_DECLARATION", INSIDE_MAIN, NULL, NULL,
> -	  "for (int i = 0; i < argc; i++) { return 0; };\n"
> -	  "return 1;" },
> +	  "int ret = 1;\n"
> +	  "for (int i = 0; i < argc; i++) { ret = 0; };\n"
> +	  "return ret;" },
>  	{ "HAVE_FLEXIBLE_ARRAY_MEMBER", OUTSIDE_MAIN, NULL, NULL,
>  	  "struct foo { unsigned int x; int arr[]; };" },
>  	{ "HAVE_GETPAGESIZE", DEFINES_FUNC, NULL, NULL,

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 11/13] configurator: Return pointer difference as ptrdiff_t

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 1441 bytes --]

On Thu, Sep 22, 2016 at 09:33:14PM -0600, Kevin Locke wrote:
> On LLP64 systems (like 64-bit Windows) long is 32 bits while pointers
> are 64 bits, which results in a warning similar to the following:
> 
> warning C4244: 'return': conversion from '__int64' to 'long', possible loss of data
> 
> for HAVE_STACK_GROWS_UPWARDS.  Fix this by using the ptrdiff_t type
> introduced by C99 for this purpose.
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>

Applying.

> ---
>  tools/configurator/configurator.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
> index a43061b..2acb0b2 100644
> --- a/tools/configurator/configurator.c
> +++ b/tools/configurator/configurator.c
> @@ -298,7 +298,8 @@ static struct test tests[] = {
>  	  "	return __stop_mysec - __start_mysec;\n"
>  	  "}\n" },
>  	{ "HAVE_STACK_GROWS_UPWARDS", DEFINES_EVERYTHING|EXECUTE, NULL, NULL,
> -	  "static long nest(const void *base, unsigned int i)\n"
> +	  "#include <stddef.h>\n"
> +	  "static ptrdiff_t nest(const void *base, unsigned int i)\n"
>  	  "{\n"
>  	  "	if (i == 0)\n"
>  	  "		return (const char *)&i - (const char *)base;\n"

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 08/13] configurator: Add output cflag option and macro

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 7819 bytes --]

On Thu, Sep 22, 2016 at 09:33:11PM -0600, Kevin Locke wrote:
> Unfortunately, not all compilers support -o as a command-line option for
> specifying the output file.  Visual Studio cl.exe issues warning D9035
> when -o is given, which is detected as a compile warning by the
> configurator.
> 
> To support such compilers, add the command-line option -O to
> configurator which can be used to specify the cflag for setting the
> output executable file name.  Additionally define the macro
> CCAN_OUTPUT_EXE_CFLAG in config.h and use it when invoking the compiler
> (e.g.  from ccanlint).
> 
> For reference, the name CCAN_OUTPUT_EXE_CFLAG was chosen to avoid
> potential name conflicts in the future due to cl.exe requiring different
> flags for different types of output[1] (e.g. object files are /Fo:).
> 
> 1.  https://msdn.microsoft.com/en-us/library/f1cb223a.aspx
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

This one has some more complex conflicts if applied without the
preceding ones, so I haven't applied it yet.

> ---
>  tools/compile.c                   | 15 ++++++++----
>  tools/configurator/configurator.c | 50 +++++++++++++++++++++++++++------------
>  tools/tools.h                     |  6 +++--
>  3 files changed, 49 insertions(+), 22 deletions(-)
> 
> diff --git a/tools/compile.c b/tools/compile.c
> index b88c1d8..e796e3f 100644
> --- a/tools/compile.c
> +++ b/tools/compile.c
> @@ -7,8 +7,12 @@
>  #ifndef CCAN_CFLAGS
>  #define CCAN_CFLAGS DEFAULT_CCAN_CFLAGS
>  #endif
> +#ifndef CCAN_OUTPUT_EXE_CFLAG
> +#define CCAN_OUTPUT_EXE_CFLAG DEFAULT_CCAN_OUTPUT_EXE_CFLAG
> +#endif
>  const char *compiler = CCAN_COMPILER;
>  const char *cflags = CCAN_CFLAGS;
> +const char *outexecflag = CCAN_OUTPUT_EXE_CFLAG;
>  
>  bool compile_verbose = false;
>  
> @@ -37,8 +41,9 @@ bool compile_object(const void *ctx, const char *cfile, const char *ccandir,
>  	if (compile_verbose)
>  		printf("Compiling %s\n", outfile);
>  	return run_command(ctx, NULL, output,
> -			   "%s %s -I%s -c -o %s %s",
> -			   compiler, cflags, ccandir, outfile, cfile);
> +			   "%s %s -I%s -c %s%s %s",
> +			   compiler, cflags, ccandir,
> +			   outexecflag, outfile, cfile);
>  }
>  
>  /* Compile and link single C file, with object files.
> @@ -51,7 +56,7 @@ bool compile_and_link(const void *ctx, const char *cfile, const char *ccandir,
>  	if (compile_verbose)
>  		printf("Compiling and linking %s\n", outfile);
>  	return run_command(ctx, NULL, output,
> -			   "%s %s -I%s -o %s %s %s %s",
> -			   compiler, cflags,
> -			   ccandir, outfile, cfile, objs, libs);
> +			   "%s %s -I%s %s%s %s %s %s",
> +			   compiler, cflags, ccandir,
> +			   outexecflag, outfile, cfile, objs, libs);
>  }
> diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
> index acf6cc0..d81f878 100644
> --- a/tools/configurator/configurator.c
> +++ b/tools/configurator/configurator.c
> @@ -40,6 +40,7 @@
>  
>  #define DEFAULT_COMPILER "cc"
>  #define DEFAULT_FLAGS "-g3 -ggdb -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition"
> +#define DEFAULT_OUTPUT_EXE_FLAG "-o"
>  
>  #define OUTPUT_FILE "configurator.out"
>  #define INPUT_FILE "configuratortest.c"
> @@ -517,10 +518,12 @@ static char *run(const char *cmd, int *exitstatus)
>  	return ret;
>  }
>  
> -static char *connect_args(const char *argv[], const char *extra)
> +static char *connect_args(const char *argv[], const char *outflag,
> +		const char *files)
>  {
> -	unsigned int i, len = strlen(extra) + 1;
> +	unsigned int i;
>  	char *ret;
> +	size_t len = strlen(outflag) + strlen(files) + 1;
>  
>  	for (i = 1; argv[i]; i++)
>  		len += 1 + strlen(argv[i]);
> @@ -530,10 +533,12 @@ static char *connect_args(const char *argv[], const char *extra)
>  	for (i = 1; argv[i]; i++) {
>  		strcpy(ret + len, argv[i]);
>  		len += strlen(argv[i]);
> -		if (argv[i+1])
> +		if (argv[i+1] || *outflag)
>  			ret[len++] = ' ';
>  	}
> -	strcpy(ret + len, extra);
> +	strcpy(ret + len, outflag);
> +	len += strlen(outflag);
> +	strcpy(ret + len, files);
>  	return ret;
>  }
>  
> @@ -702,33 +707,47 @@ int main(int argc, const char *argv[])
>  	unsigned int i;
>  	const char *default_args[]
>  		= { "", DEFAULT_COMPILER, DEFAULT_FLAGS, NULL };
> +	const char *outflag = DEFAULT_OUTPUT_EXE_FLAG;
>  
>  	if (argc > 0)
>  		progname = argv[0];
>  
> -	if (argc > 1) {
> +	while (argc > 1) {
>  		if (strcmp(argv[1], "--help") == 0) {
> -			printf("Usage: configurator [-v] [<compiler> <flags>...]\n"
> -			       "  <compiler> <flags> will have \"-o <outfile> <infile.c>\" appended\n"
> -			       "Default: %s %s\n",
> -			       DEFAULT_COMPILER, DEFAULT_FLAGS);
> +			printf("Usage: configurator [-v] [-O<outflag>] [<compiler> <flags>...]\n"
> +			       "  <compiler> <flags> will have \"<outflag> <outfile> <infile.c>\" appended\n"
> +			       "Default: %s %s %s\n",
> +			       DEFAULT_COMPILER, DEFAULT_FLAGS,
> +			       DEFAULT_OUTPUT_EXE_FLAG);
>  			exit(0);
>  		}
> -		if (strcmp(argv[1], "-v") == 0) {
> +		if (strncmp(argv[1], "-O", 2) == 0) {
>  			argc--;
>  			argv++;
> -			verbose = 1;
> +			outflag = argv[1] + 2;
> +			if (!*outflag) {
> +				fprintf(stderr,
> +					"%s: option requires an argument -- O\n",
> +					argv[0]);
> +				exit(1);
> +			}
> +		} else if (strcmp(argv[1], "-v") == 0) {
> +			argc--;
> +			argv++;
> +			verbose++;
>  		} else if (strcmp(argv[1], "-vv") == 0) {
>  			argc--;
>  			argv++;
> -			verbose = 2;
> +			verbose += 2;
> +		} else {
> +			break;
>  		}
>  	}
>  
>  	if (argc == 1)
>  		argv = default_args;
>  
> -	cmd = connect_args(argv, " -o " OUTPUT_FILE " " INPUT_FILE);
> +	cmd = connect_args(argv, outflag, OUTPUT_FILE " " INPUT_FILE);
>  	for (i = 0; i < sizeof(tests)/sizeof(tests[0]); i++)
>  		run_test(cmd, &tests[i]);
>  	free(cmd);
> @@ -743,9 +762,10 @@ int main(int argc, const char *argv[])
>  	printf("#define _GNU_SOURCE /* Always use GNU extensions. */\n");
>  	printf("#endif\n");
>  	printf("#define CCAN_COMPILER \"%s\"\n", argv[1]);
> -	cmd = connect_args(argv+1, "");
> -	printf("#define CCAN_CFLAGS \"%s\"\n\n", cmd);
> +	cmd = connect_args(argv + 1, "", "");
> +	printf("#define CCAN_CFLAGS \"%s\"\n", cmd);
>  	free(cmd);
> +	printf("#define CCAN_OUTPUT_EXE_CFLAG \"%s\"\n\n", outflag);
>  	/* This one implies "#include <ccan/..." works, eg. for tdb2.h */
>  	printf("#define HAVE_CCAN 1\n");
>  	for (i = 0; i < sizeof(tests)/sizeof(tests[0]); i++)
> diff --git a/tools/tools.h b/tools/tools.h
> index 3b5c4be..668c91a 100644
> --- a/tools/tools.h
> +++ b/tools/tools.h
> @@ -11,6 +11,7 @@
>  /* These are the defaults. */
>  #define DEFAULT_CCAN_COMPILER "cc"
>  #define DEFAULT_CCAN_CFLAGS "-g"
> +#define DEFAULT_CCAN_OUTPUT_EXE_CFLAG "-o"
>  
>  #define IDENT_CHARS	"ABCDEFGHIJKLMNOPQRSTUVWXYZ" \
>  			"abcdefghijklmnopqrstuvwxyz" \
> @@ -20,8 +21,9 @@
>  
>  #define COVERAGE_CFLAGS "-fprofile-arcs -ftest-coverage"
>  
> -/* Actual compiler and cflags (defaults to CCAN_COMPILER and CCAN_CFLAGS). */
> -extern const char *compiler, *cflags;
> +/* Actual compiler and cflags
> + * (defaults to CCAN_COMPILER, CCAN_CFLAGS, CCAN_OUTPUT_EXE_CFLAG). */
> +extern const char *compiler, *cflags, *outexecflag;
>  
>  /* This compiles up the _info file into a temporary. */
>  char *compile_info(const void *ctx, const char *dir);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 09/13] configurator: DEFAULT_{COMPILER, FLAGS} for MSVC

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 2178 bytes --]

On Thu, Sep 22, 2016 at 09:33:12PM -0600, Kevin Locke wrote:
> When compiling with Visual Studio, use default compiler name and flags
> which are likely to work with the known-available compiler.
> 
> This is also a convenience for users who may not know what arguments
> cl.exe may need to compile the tests.
> 
> Changes since v1:
> - Use "-option" instead of "/option" to avoid issues running under msys.
> - Disable C4200 warning for use of flexible array members, which MSVC
>   considers an extension (since it does not fully support C99).
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

But I won't apply it for now, since it makes no sense without the
previous one.

> ---
>  tools/configurator/configurator.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
> index d81f878..d3e01cd 100644
> --- a/tools/configurator/configurator.c
> +++ b/tools/configurator/configurator.c
> @@ -38,9 +38,21 @@
>  #define pclose _pclose
>  #endif
>  
> +#ifdef _MSC_VER
> +#define DEFAULT_COMPILER "cl"
> +/* Note:  Dash options avoid POSIX path conversion when used under msys bash
> + *        and are therefore preferred to slash (e.g. -nologo over /nologo)
> + * Note:  Disable Warning 4200 "nonstandard extension used : zero-sized array
> + *        in struct/union" for flexible array members.
> + */
> +#define DEFAULT_FLAGS "-nologo -Zi -W4 -wd4200 " \
> +	"-D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS"
> +#define DEFAULT_OUTPUT_EXE_FLAG "-Fe:"
> +#else
>  #define DEFAULT_COMPILER "cc"
>  #define DEFAULT_FLAGS "-g3 -ggdb -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition"
>  #define DEFAULT_OUTPUT_EXE_FLAG "-o"
> +#endif
>  
>  #define OUTPUT_FILE "configurator.out"
>  #define INPUT_FILE "configuratortest.c"

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 12/13] Makefile: Define CFLAGS_FORCE_C_SOURCE macro

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 1958 bytes --]

On Thu, Sep 22, 2016 at 09:33:15PM -0600, Kevin Locke wrote:
> This macro holds the C compiler flag(s) to force input files to be
> recognized as C sources regardless of extension.  It is defined to allow
> overriding on the make command line.
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>

So.. this approach works.

But I've been wondering for a while if the weird thing we do of having
the _info files not have a .c extension is a good idea.

I think the underscore enough is probably enough to mark those
special, so things might be less messy if we just called them _info.c.

Rusty, thoughts?

> ---
>  Makefile      | 2 +-
>  Makefile-ccan | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 127b875..37ff17d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -64,7 +64,7 @@ summary-fastcheck/%: tools/ccanlint/ccanlint $(OBJFILES)
>  	$(CCANLINT_FAST) -s ccan/$*
>  
>  ccan/%/info: ccan/%/_info config.h
> -	$(CC) $(CCAN_CFLAGS) -I. -o $@ -x c $<
> +	$(CC) $(CCAN_CFLAGS) -I. -o $@ $(CFLAGS_FORCE_C_SOURCE) $<
>  
>  all_info: $(MODS:%=ccan/%/info)
>  
> diff --git a/Makefile-ccan b/Makefile-ccan
> index 5c8dea2..d4a5528 100644
> --- a/Makefile-ccan
> +++ b/Makefile-ccan
> @@ -5,6 +5,7 @@
>  #CCAN_CFLAGS=-g -O3 -Wall -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-declarations -Wpointer-arith -Wwrite-strings -Wundef -DCCAN_STR_DEBUG=1
>  CCAN_CFLAGS=-g3 -ggdb -Wall -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-declarations -Wpointer-arith -Wwrite-strings -Wundef -DCCAN_STR_DEBUG=1
>  CFLAGS = $(CCAN_CFLAGS) -I. $(DEPGEN)
> +CFLAGS_FORCE_C_SOURCE = -x c
>  
>  MODS := a_star \
>  	aga \

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 13/13] Add appveyor.yml

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 3340 bytes --]

On Thu, Sep 22, 2016 at 09:33:16PM -0600, Kevin Locke wrote:
> This file defines the AppVeyor CI (appveyor.com) settings.
> 
> It builds using make+bash under MSYS2 so that the current build system
> can be used with minimal changes.  It currently only builds configurator
> and generates config.h.
> 
> The build and test commands for more thorough testing are left as
> comments in appveyor.yml so interested parties can use them as a
> starting point for future work.
> 
> Note that several compiler errors not related to configurator are
> printed due to make attempting to generate and include test-depends.
> Although Windows-specific code could be added to Makefile to avoid
> these, it seemed unwarranted if the compile errors may be fixed
> soon.

Nice, though I can't really test it until we've got the rest of the
Windows / MSVC fixes applied.

If possible it would be nice to have in the commit message, and maybe
as a comment in the yml file too, some instructions on how to view the
appveyor results - what page to go to, do you need a login etc.

> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
> ---
>  appveyor.yml | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 appveyor.yml
> 
> diff --git a/appveyor.yml b/appveyor.yml
> new file mode 100644
> index 0000000..822c4bd
> --- /dev/null
> +++ b/appveyor.yml
> @@ -0,0 +1,25 @@
> +version: 0.0.{build}
> +
> +platform:
> +- x86
> +- amd64
> +
> +build_script:
> +# Reset %PATH% to avoid msys/cygwin DLL conflicts which cause
> +# *** fatal error - cygheap base mismatch detected
> +- set PATH=C:\Windows\System32;C:\Windows;C:\Windows\System32\Wbem;C:\msys64\usr\bin
> +# Set environment variables for chosen compiler
> +- "\"C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\vcvarsall.bat\" %Platform%"
> +# Ensure config.h can be generated.
> +# Note: Dash options avoid POSIX path conversion by msys
> +- "make config.h CC=cl CCAN_CFLAGS=\"-nologo -Zi -FS -W4 -wd4200 -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS\" CFLAGS_FORCE_C_SOURCE=-TC OUTPUT_OPTION=-Fo:$@ DEPGEN= LD=link SHELL=bash"
> +# FIXME:  Work in progress.  Disabled due to unfixed compile errors.
> +# FIXME:  -j%NUMBER_OF_PROCESSORS% won't work without DEPGEN for config.h
> +#         It may be possible to generate .d from cl using /showIncludes
> +#         See https://stackoverflow.com/q/37685069
> +#- "make tools CC=cl CCAN_CFLAGS=\"-nologo -Zi -FS -W4 -wd4200 -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS\" CFLAGS_FORCE_C_SOURCE=-TC OUTPUT_OPTION=-Fo:$@ DEPGEN= LD=link SHELL=bash"
> +#- "make -k CC=cl CCAN_CFLAGS=\"-nologo -Zi -FS -W4 -wd4200 -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS\" CFLAGS_FORCE_C_SOURCE=-TC OUTPUT_OPTION=-Fo:$@ DEPGEN= LD=link SHELL=bash"
> +
> +test_script:
> +# FIXME:  Work in progress.  Disabled due to unfixed compile errors.
> +#- "make -k check CC=cl CCAN_CFLAGS=\"-nologo -Zi -FS -W4 -wd4200 -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS\" CFLAGS_FORCE_C_SOURCE=-TC OUTPUT_OPTION=-Fo:$@ DEPGEN= LD=link SHELL=bash"

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 00/13] configurator: Support for Windows and MSVC

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 2591 bytes --]

On Thu, Sep 22, 2016 at 09:33:03PM -0600, Kevin Locke wrote:
> Hi all,
> 
> This is the second revision of my patch series which adds support for
> building configurator with Microsoft Visual C++ and running it on
> Windows.  The motivation is to add sufficient support for Windows to
> allow using header-only modules with minimal hassle.
> 
> The major change in this revision is the handling of the output cflag,
> which can now be specified by a command-line option to configurator and
> is set in a separate config.h macro.  This both fixes the ccanlint
> breakage in the previous series and preserves backwards-compatibility
> for any code calling configurator or using CCAN_CFLAGS.
> 
> Patches which need special attention:
> 
> As before, patch 3 adds a copy of the err.h functions used by
> configurator to work around the lack of err.h on Windows.  If this is
> too much code to lump into configurator.c, I can split it out (and copy
> err.{c,h} verbatim from musl) or call fprintf+exit directly or consider
> modifying the ccan/err module for use by configurator.
> 
> Patch 8 changes the configurator API by adding a command-line option and
> config.h macro.  The names are different than I had previously
> suggested, as noted in the commit message.
> 
> Patch 13 adds appveyor.yml for AppVeyor CI.  This will require (free)
> account setup on ci.appveyor.com once merged to enable automatic
> building and GitHub integration (if desired).  You may also wish to
> consider whether to add a README badge or email notification or other
> mechanism to be notified of build status (beyond the default pull
> request hooks).
> 
> Thanks for considering once again,

Thanks for sending again.  I've applied a number of the simpler ones
already.  The remainder I still have some comments on.

Incidentally you are introducing a new concept here which isn't really
comment on.  The use of more #ifdefs on system macros into
configurator means that the way our default compiler options will be
set up is basically going to be dependent on the setup used to compile
configurator itself.

That seems like a sensible approach to me; we just need to be a bit
careful of interactions with make and/or other build systems when we
could have a system with multiple compilation options (e.g. both MSVC
and cygwin/gcc installed on a Windows machine).

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH 3/9] configurator: Inline err.h functions from musl libc

[Rusty Russell]

David Gibson <david@gibson.dropbear.id.au> writes:
> On Sun, Sep 18, 2016 at 06:52:00PM -0600, Kevin Locke wrote:
>> Since we can't use HAVE_ERR_H in configurator, provide a definition for
>> the err.h functions used.  The version provided is the one from musl
>> libc, since it is concise and shares the MIT License with configurator.
>> 
>> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
>
> Right, so this is a general problem of how do we make the configurator
> itself sufficiently portable, since it hasn't yet worked out the
> portbility macros.
>
> Personally, I think duplicating these here is an acceptable
> workaround.  Rusty, any opinion?

Sorry for the delay, I was away for a week, then laptop death...

Yes, this is the right approach.  configurator has to stand alone.

Thanks!
Rusty.
_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 12/13] Makefile: Define CFLAGS_FORCE_C_SOURCE macro

[Rusty Russell]

David Gibson <david@gibson.dropbear.id.au> writes:
> On Thu, Sep 22, 2016 at 09:33:15PM -0600, Kevin Locke wrote:
>> This macro holds the C compiler flag(s) to force input files to be
>> recognized as C sources regardless of extension.  It is defined to allow
>> overriding on the make command line.
>> 
>> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
>
> So.. this approach works.
>
> But I've been wondering for a while if the weird thing we do of having
> the _info files not have a .c extension is a good idea.
>
> I think the underscore enough is probably enough to mark those
> special, so things might be less messy if we just called them _info.c.
>
> Rusty, thoughts?

_info.c breaks the "compile *.c to get the module" rule, though.

_info is for both machines and humans, so it's a border case, but I
think it comes down on "make the machine work harder".

Cheers,
Rusty.
_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 00/13] configurator: Support for Windows and MSVC

[Rusty Russell]

David Gibson <david@gibson.dropbear.id.au> writes:
> On Thu, Sep 22, 2016 at 09:33:03PM -0600, Kevin Locke wrote:
>> Hi all,
>> 
>> This is the second revision of my patch series which adds support for
>> building configurator with Microsoft Visual C++ and running it on
>> Windows.  The motivation is to add sufficient support for Windows to
>> allow using header-only modules with minimal hassle.

Thanks for working on this! (Both of you!) I know it's a pain being a
pioneer, but you'll be owed beers by many future programmers!

Cheers,
Rusty.
_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 12/13] Makefile: Define CFLAGS_FORCE_C_SOURCE macro

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 1358 bytes --]

On Tue, Sep 27, 2016 at 03:48:33PM +0930, Paul 'Rusty' Russell wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> > On Thu, Sep 22, 2016 at 09:33:15PM -0600, Kevin Locke wrote:
> >> This macro holds the C compiler flag(s) to force input files to be
> >> recognized as C sources regardless of extension.  It is defined to allow
> >> overriding on the make command line.
> >> 
> >> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
> >
> > So.. this approach works.
> >
> > But I've been wondering for a while if the weird thing we do of having
> > the _info files not have a .c extension is a good idea.
> >
> > I think the underscore enough is probably enough to mark those
> > special, so things might be less messy if we just called them _info.c.
> >
> > Rusty, thoughts?
> 
> _info.c breaks the "compile *.c to get the module" rule, though.

Which is already broken by modules which have a non-trivial Makefile
fragment (like ntdb).

> _info is for both machines and humans, so it's a border case, but I
> think it comes down on "make the machine work harder".

Ok, let's keep this patch as is for the time being then.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v3 2/13] configurator: Reimplement run using popen

[Kevin Locke]

On Tue, 2016-09-27 at 14:52 +1000, David Gibson wrote:
> On Thu, Sep 22, 2016 at 09:33:05PM -0600, Kevin Locke wrote:
>> - Create fread_noeintr to avoid EINTR in fread without reading any data.
>> - Handle short reads from fread.  This can happen with non-conformant
>>   libc or if EINTR occurs after reading some data.
> 
> Hrm.  Have you actually confirmed this problem with EINTR?  My
> previous objections to not handling short read were based entirely on
> not realizing that fread() wasn't supposed to do that, rather than
> thinking of some edge case you hadn't.
> 
> Unless we have a confirmed case where fread() can give a short read
> that won't have feof() or ferror() set afterwards, I'm inclined to go
> back to your initial implementation.

Nope, I was just being overly cautious.  Sounds good to me.  Below is
an updated patch which is the same as v1 but with the addition of the
#define for _POSIX_C_SOURCE to get pclose, popen, and strdup.

-8<--------------------------------------------------------------------
Rather than using fork+pipe+system+waitpid, most of which are only
available on POSIX-like systems, use popen which is also available on
Windows (under the name _popen).

Changes since v1:
- Create fread_noeintr to avoid EINTR in fread without reading any data.
- Handle short reads from fread.  This can happen with non-conformant
  libc or if EINTR occurs after reading some data.
- Define _POSIX_C_SOURCE for popen/pclose with strict implementations
  which require it (e.g. gcc with -std=c11).

Changes since v2:
- Revert fread_noeintr and short read changes in v1 as unnecessary.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/configurator/configurator.c | 79 +++++++++++++++------------------------
 1 file changed, 31 insertions(+), 48 deletions(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index c7577c0..2ad3fff 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -21,17 +21,19 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#define _POSIX_C_SOURCE 200809L                /* For pclose, popen, strdup */
+
 #include <stdio.h>
 #include <stdbool.h>
 #include <stdlib.h>
-#include <unistd.h>
 #include <err.h>
-#include <sys/types.h>
-#include <sys/wait.h>
-#include <sys/stat.h>
-#include <fcntl.h>
 #include <string.h>

+#ifdef _MSC_VER
+#define popen _popen
+#define pclose _pclose
+#endif
+
 #define DEFAULT_COMPILER "cc"
 #define DEFAULT_FLAGS "-g3 -ggdb -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition"

@@ -375,20 +377,19 @@ static struct test tests[] = {
 	},
 };

-static char *grab_fd(int fd)
+static char *grab_stream(FILE *file)
 {
-	int ret;
-	size_t max, size = 0;
+	size_t max, ret, size = 0;
 	char *buffer;

-	max = 16384;
-	buffer = malloc(max+1);
-	while ((ret = read(fd, buffer + size, max - size)) > 0) {
+	max = BUFSIZ;
+	buffer = malloc(max);
+	while ((ret = fread(buffer+size, 1, max - size, file)) == max - size) {
 		size += ret;
-		if (size == max)
-			buffer = realloc(buffer, max *= 2);
+		buffer = realloc(buffer, max *= 2);
 	}
-	if (ret < 0)
+	size += ret;
+	if (ferror(file))
 		err(1, "reading from command");
 	buffer[size] = '\0';
 	return buffer;
@@ -396,43 +397,25 @@ static char *grab_fd(int fd)

 static char *run(const char *cmd, int *exitstatus)
 {
-	pid_t pid;
-	int p[2];
+	static const char redir[] = " 2>&1";
+	size_t cmdlen;
+	char *cmdredir;
+	FILE *cmdout;
 	char *ret;
-	int status;

-	if (pipe(p) != 0)
-		err(1, "creating pipe");
-
-	pid = fork();
-	if (pid == -1)
-		err(1, "forking");
-
-	if (pid == 0) {
-		if (dup2(p[1], STDOUT_FILENO) != STDOUT_FILENO
-		    || dup2(p[1], STDERR_FILENO) != STDERR_FILENO
-		    || close(p[0]) != 0
-		    || close(STDIN_FILENO) != 0
-		    || open("/dev/null", O_RDONLY) != STDIN_FILENO)
-			exit(128);
-
-		status = system(cmd);
-		if (WIFEXITED(status))
-			exit(WEXITSTATUS(status));
-		/* Here's a hint... */
-		exit(128 + WTERMSIG(status));
-	}
+	cmdlen = strlen(cmd);
+	cmdredir = malloc(cmdlen + sizeof(redir));
+	memcpy(cmdredir, cmd, cmdlen);
+	memcpy(cmdredir + cmdlen, redir, sizeof(redir));
+
+	cmdout = popen(cmdredir, "r");
+	if (!cmdout)
+		err(1, "popen \"%s\"", cmdredir);
+
+	free(cmdredir);

-	close(p[1]);
-	ret = grab_fd(p[0]);
-	/* This shouldn't fail... */
-	if (waitpid(pid, &status, 0) != pid)
-		err(1, "Failed to wait for child");
-	close(p[0]);
-	if (WIFEXITED(status))
-		*exitstatus = WEXITSTATUS(status);
-	else
-		*exitstatus = -WTERMSIG(status);
+	ret = grab_stream(cmdout);
+	*exitstatus = pclose(cmdout);
 	return ret;
 }

--
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v3 3/13] configurator: Inline err.h functions

[Kevin Locke]

On Tue, 2016-09-27 at 15:01 +1000, David Gibson wrote:
> For me this breaks Linux build, because we get "static declaration
> follows non-static" on these functions.
> 
> I think we instead want to stop using the err() etc. functions and use
> similar functions with a different name that we always define locally.

Good catch.  On my system the error only appears if I include <err.h>
in configurator.c, but it's a good one to fix and may affect many
other libcs.

The following patch addresses the issue by prefixing the err functions
with "c12r_".  It also replaced the musl libc implementation with the
one from the err module, since I only recently realized that CCAN
already had an err.h implementation and I don't see any need to add a
different one to the sources.

-8<--------------------------------------------------------------------
On systems where err.h is not provided (e.g. MSVC) configurator must
provide its own definition.  The err module can not be used by
configurator due to its dependency on config.h, so the relevant source
is copied into configurator with the minimum changes necessary.

Changes since v2:
- Use the CCAN err module sources instead of musl libc sources to avoid
  introducing another implementation of these functions.
- Prefix err.h functions with "c12r_" to avoid name conflicts and
  "static declaration follows non-static" errors.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/configurator/configurator.c | 49 ++++++++++++++++++++++++++++++++-------
 1 file changed, 41 insertions(+), 8 deletions(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index 2ad3fff..a7cbfa5 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -3,6 +3,9 @@
  *
  * Copyright 2011 Rusty Russell <rusty@rustcorp.com.au>.  MIT license.
  *
+ * c12r_err, c12r_errx functions copied from ccan/err/err.c
+ * Copyright Rusty Russell <rusty@rustcorp.com.au>. CC0 (Public domain) License.
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
  * in the Software without restriction, including without limitation the rights
@@ -23,10 +26,11 @@
  */
 #define _POSIX_C_SOURCE 200809L                /* For pclose, popen, strdup */

+#include <errno.h>
 #include <stdio.h>
+#include <stdarg.h>
 #include <stdbool.h>
 #include <stdlib.h>
-#include <err.h>
 #include <string.h>

 #ifdef _MSC_VER
@@ -46,6 +50,7 @@
 #define DIR_SEP   "/"
 #endif

+static const char *progname = "";
 static int verbose;

 enum test_style {
@@ -377,6 +382,31 @@ static struct test tests[] = {
 	},
 };

+static void c12r_err(int eval, const char *fmt, ...)
+{
+	int err_errno = errno;
+	va_list ap;
+
+	fprintf(stderr, "%s: ", progname);
+	va_start(ap, fmt);
+	vfprintf(stderr, fmt, ap);
+	va_end(ap);
+	fprintf(stderr, ": %s\n", strerror(err_errno));
+	exit(eval);
+}
+
+static void c12r_errx(int eval, const char *fmt, ...)
+{
+	va_list ap;
+
+	fprintf(stderr, "%s: ", progname);
+	va_start(ap, fmt);
+	vfprintf(stderr, fmt, ap);
+	va_end(ap);
+	fprintf(stderr, "\n");
+	exit(eval);
+}
+
 static char *grab_stream(FILE *file)
 {
 	size_t max, ret, size = 0;
@@ -390,7 +420,7 @@ static char *grab_stream(FILE *file)
 	}
 	size += ret;
 	if (ferror(file))
-		err(1, "reading from command");
+		c12r_err(1, "reading from command");
 	buffer[size] = '\0';
 	return buffer;
 }
@@ -410,7 +440,7 @@ static char *run(const char *cmd, int *exitstatus)

 	cmdout = popen(cmdredir, "r");
 	if (!cmdout)
-		err(1, "popen \"%s\"", cmdredir);
+		c12r_err(1, "popen \"%s\"", cmdredir);

 	free(cmdredir);

@@ -502,7 +532,7 @@ static bool run_test(const char *cmd, struct test *test)

 	outf = fopen(INPUT_FILE, "w");
 	if (!outf)
-		err(1, "creating %s", INPUT_FILE);
+		c12r_err(1, "creating %s", INPUT_FILE);

 	fprintf(outf, "%s", PRE_BOILERPLATE);
 	switch (test->style & ~(EXECUTE|MAY_NOT_COMPILE)) {
@@ -567,8 +597,8 @@ static bool run_test(const char *cmd, struct test *test)
 			       status ? "fail" : "warning",
 			       test->name, status, output);
 		if ((test->style & EXECUTE) && !(test->style & MAY_NOT_COMPILE))
-			errx(1, "Test for %s did not compile:\n%s",
-			     test->name, output);
+			c12r_errx(1, "Test for %s did not compile:\n%s",
+				  test->name, output);
 		test->answer = false;
 		free(output);
 	} else {
@@ -578,8 +608,8 @@ static bool run_test(const char *cmd, struct test *test)
 		if ((test->style & EXECUTE) || (test->style & INSIDE_MAIN)) {
 			output = run("." DIR_SEP OUTPUT_FILE, &status);
 			if (!(test->style & EXECUTE) && status != 0)
-				errx(1, "Test for %s failed with %i:\n%s",
-				     test->name, status, output);
+				c12r_errx(1, "Test for %s failed with %i:\n%s",
+					  test->name, status, output);
 			if (verbose && status)
 				printf("%s exited %i\n", test->name, status);
 			free(output);
@@ -603,6 +633,9 @@ int main(int argc, const char *argv[])
 	const char *default_args[]
 		= { "", DEFAULT_COMPILER, DEFAULT_FLAGS, NULL };

+	if (argc > 0)
+		progname = argv[0];
+
 	if (argc > 1) {
 		if (strcmp(argv[1], "--help") == 0) {
 			printf("Usage: configurator [-v] [<compiler> <flags>...]\n"
--
2.9.3
_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 06/13] configurator: Print test source without cat

[Kevin Locke]

On Tue, 2016-09-27 at 15:06 +1000, David Gibson wrote:
> On Thu, Sep 22, 2016 at 09:33:09PM -0600, Kevin Locke wrote:
>> Windows does not provide cat.  Instead, copy the test source to stdout
>> using the file stream to which it was written.
>> 
>> Changes since v1:
>> - Create fwrite_noeintr to avoid EINTR in fwrite without writing any
>>   data.
>> - Handle short reads from fread.  This can happen with non-conformant
>>   libc or if EINTR occurs after reading some data.
>> - Handle short writes from fwrite.  This can happen with non-conformant
>>   libc or if EINTR occurs after writing some data.
>> 
>> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
> 
> As with patch 2, I'm not sure about the need for fwrite_noeintr().

Agreed.  The v1 patch should be good as-is with just a few line
offsets.  If it gives you any problems, I can post an updated one.

Kevin
_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v3 13/13] Add appveyor.yml

[Kevin Locke]

On Tue, 2016-09-27 at 15:20 +1000, David Gibson wrote:
> On Thu, Sep 22, 2016 at 09:33:16PM -0600, Kevin Locke wrote:
>> This file defines the AppVeyor CI (appveyor.com) settings.
> 
> Nice, though I can't really test it until we've got the rest of the
> Windows / MSVC fixes applied.
> 
> If possible it would be nice to have in the commit message, and maybe
> as a comment in the yml file too, some instructions on how to view the
> appveyor results - what page to go to, do you need a login etc.

Sure, good idea.  This patch includes basic instructions for how to
view the current status and how to setup AppVeyor for a copy of the
repository.

It makes the assumption that Rusty wouldn't mind setting up AppVeyor
for his CCAN GitHub repo (which would also add build checks to pull
requests).  If you'd prefer not to, I think I can set it up to build
from git://git.ozlabs.org/~ccan/ccan on my own account (although I'm
not sure how it would handle update checking).  Let me know and I'll
give it a try to find out.

Also, for reference, you can see the current build results on my repo
at https://ci.appveyor.com/project/kevinoid/ccan

-8<--------------------------------------------------------------------
This file defines the AppVeyor CI (appveyor.com) settings.

It builds using make+bash under MSYS2 so that the current build system
can be used with minimal changes.  It currently only builds configurator
and generates config.h.

The build and test commands for more thorough testing are left as
comments in appveyor.yml so interested parties can use them as a
starting point for future work.

Note that several compiler errors not related to configurator are
printed due to make attempting to generate and include test-depends.
Although Windows-specific code could be added to Makefile to avoid
these, it seemed unwarranted if the compile errors may be fixed soon.

Changes since v2:
- Add reference to AppVeyor results for Rusty's GitHub repo and basic
  instructions to setup AppVeyor for forks.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 appveyor.yml | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 appveyor.yml

diff --git a/appveyor.yml b/appveyor.yml
new file mode 100644
index 0000000..0a930c6
--- /dev/null
+++ b/appveyor.yml
@@ -0,0 +1,37 @@
+# AppVeyor CI Configuration
+#
+# Current build status can be viewed at
+# https://ci.appveyor.com/project/rustyrussell/ccan
+#
+# To build test a fork of this repository using AppVeyor:
+# 1. Visit appveyor.com and create a free account.
+# 2. On the Projects tab, select "New Project" then the project hosting site.
+# 3. Grant OAuth permissions to your project git host.
+# 4. Select the repository for your project fork.
+# 5. Review build results and push updates until the desired result is reached.
+
+version: 0.0.{build}
+
+platform:
+- x86
+- amd64
+
+build_script:
+# Reset %PATH% to avoid msys/cygwin DLL conflicts which cause
+# *** fatal error - cygheap base mismatch detected
+- set PATH=C:\Windows\System32;C:\Windows;C:\Windows\System32\Wbem;C:\msys64\usr\bin
+# Set environment variables for chosen compiler
+- "\"C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\vcvarsall.bat\" %Platform%"
+# Ensure config.h can be generated.
+# Note: Dash options avoid POSIX path conversion by msys
+- "make config.h CC=cl CCAN_CFLAGS=\"-nologo -Zi -FS -W4 -wd4200 -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS\" CFLAGS_FORCE_C_SOURCE=-TC OUTPUT_OPTION=-Fo:$@ DEPGEN= LD=link SHELL=bash"
+# FIXME:  Work in progress.  Disabled due to unfixed compile errors.
+# FIXME:  -j%NUMBER_OF_PROCESSORS% won't work without DEPGEN for config.h
+#         It may be possible to generate .d from cl using /showIncludes
+#         See https://stackoverflow.com/q/37685069
+#- "make tools CC=cl CCAN_CFLAGS=\"-nologo -Zi -FS -W4 -wd4200 -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS\" CFLAGS_FORCE_C_SOURCE=-TC OUTPUT_OPTION=-Fo:$@ DEPGEN= LD=link SHELL=bash"
+#- "make -k CC=cl CCAN_CFLAGS=\"-nologo -Zi -FS -W4 -wd4200 -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS\" CFLAGS_FORCE_C_SOURCE=-TC OUTPUT_OPTION=-Fo:$@ DEPGEN= LD=link SHELL=bash"
+
+test_script:
+# FIXME:  Work in progress.  Disabled due to unfixed compile errors.
+#- "make -k check CC=cl CCAN_CFLAGS=\"-nologo -Zi -FS -W4 -wd4200 -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS\" CFLAGS_FORCE_C_SOURCE=-TC OUTPUT_OPTION=-Fo:$@ DEPGEN= LD=link SHELL=bash"
-- 
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 00/13] configurator: Support for Windows and MSVC

[Kevin Locke]

On Tue, 2016-09-27 at 15:23 +1000, David Gibson wrote:
> On Thu, Sep 22, 2016 at 09:33:03PM -0600, Kevin Locke wrote:
>> This is the second revision of my patch series which adds support for
>> building configurator with Microsoft Visual C++ and running it on
>> Windows.  The motivation is to add sufficient support for Windows to
>> allow using header-only modules with minimal hassle.
> 
> Thanks for sending again.  I've applied a number of the simpler ones
> already.  The remainder I still have some comments on.

Thanks for reviewing again!  I think I've addressed everything, but if
there is anything I've overlooked, let me know.

> Incidentally you are introducing a new concept here which isn't really
> comment on.  The use of more #ifdefs on system macros into
> configurator means that the way our default compiler options will be
> set up is basically going to be dependent on the setup used to compile
> configurator itself.
> 
> That seems like a sensible approach to me; we just need to be a bit
> careful of interactions with make and/or other build systems when we
> could have a system with multiple compilation options (e.g. both MSVC
> and cygwin/gcc installed on a Windows machine).

That is a good point and probably deserves some more thought and
discussion.  I've tried to keep most additions configurable at
run-time (e.g. the new -O configurator flag) so if the compile-time
deduction is wrong it can be overridden.  But it still changes the
configurator behavior based on how it was compiled without any
explicit configuration.  That may or may not be desirable.  Has there
been any discussion of being more explicit about specifying build vs
host system at build time?  Anything in particular you have in mind?

A good example of something which can not be overridden at runtime is
the directory separator passed to popen(3).  I don't think there are
currently any problems, but the choice of directory separator could
easily cause problems between Cygwin and native binaries in other
situations.  Definitely worth thinking about.

Cheers,
Kevin
_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 00/13] configurator: Support for Windows and MSVC

[Kevin Locke]

On Tue, 2016-09-27 at 15:50 +0930, Rusty Russell wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
>> On Thu, Sep 22, 2016 at 09:33:03PM -0600, Kevin Locke wrote:
>>> Hi all,
>>> 
>>> This is the second revision of my patch series which adds support for
>>> building configurator with Microsoft Visual C++ and running it on
>>> Windows.  The motivation is to add sufficient support for Windows to
>>> allow using header-only modules with minimal hassle.
> 
> Thanks for working on this! (Both of you!) I know it's a pain being a
> pioneer, but you'll be owed beers by many future programmers!

My pleasure.  Sounds well worth the reward to me!

Cheers,
Kevin
_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 00/13] configurator: Support for Windows and MSVC

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 2749 bytes --]

On Wed, Sep 28, 2016 at 12:28:03AM -0600, Kevin Locke wrote:
> On Tue, 2016-09-27 at 15:23 +1000, David Gibson wrote:
> > On Thu, Sep 22, 2016 at 09:33:03PM -0600, Kevin Locke wrote:
> >> This is the second revision of my patch series which adds support for
> >> building configurator with Microsoft Visual C++ and running it on
> >> Windows.  The motivation is to add sufficient support for Windows to
> >> allow using header-only modules with minimal hassle.
> > 
> > Thanks for sending again.  I've applied a number of the simpler ones
> > already.  The remainder I still have some comments on.
> 
> Thanks for reviewing again!  I think I've addressed everything, but if
> there is anything I've overlooked, let me know.

Right.  Can you rebase your current patches and post them as a new
series.  That's makes my life easier compared to updated versions of
each patch in various places in the thread.

> 
> > Incidentally you are introducing a new concept here which isn't really
> > comment on.  The use of more #ifdefs on system macros into
> > configurator means that the way our default compiler options will be
> > set up is basically going to be dependent on the setup used to compile
> > configurator itself.
> > 
> > That seems like a sensible approach to me; we just need to be a bit
> > careful of interactions with make and/or other build systems when we
> > could have a system with multiple compilation options (e.g. both MSVC
> > and cygwin/gcc installed on a Windows machine).
> 
> That is a good point and probably deserves some more thought and
> discussion.  I've tried to keep most additions configurable at
> run-time (e.g. the new -O configurator flag) so if the compile-time
> deduction is wrong it can be overridden.  But it still changes the
> configurator behavior based on how it was compiled without any
> explicit configuration.  That may or may not be desirable.

I think it's desirable, on the whole.  But having overrides is
generally a good idea too.

> Has there
> been any discussion of being more explicit about specifying build vs
> host system at build time?  Anything in particular you have in mind?

No, I'm afraid not.

> A good example of something which can not be overridden at runtime is
> the directory separator passed to popen(3).  I don't think there are
> currently any problems, but the choice of directory separator could
> easily cause problems between Cygwin and native binaries in other
> situations.  Definitely worth thinking about.

Yeah.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v3 0/7] configurator: Support for Windows and MSVC

[Kevin Locke]

On Thu, 2016-09-29 at 10:21 +1000, David Gibson wrote:
> Right.  Can you rebase your current patches and post them as a new
> series.  That's makes my life easier compared to updated versions of
> each patch in various places in the thread.

Certainly.  No problem.

Here is the third revision of the patch series which adds support for
building configurator with Microsoft Visual C++ and running it on
Windows.

Significant changes from v2:
* Removing the unnecessary EINTR and short read/write handling code.
* Switching from the musl libc err.h implementation to the CCAN err
  module one in hopes of reducing maintenance burden and code
  proliferation.
* Additional documentation comments for AppVeyor.

Thanks again!
Kevin


Kevin Locke (7):
  configurator: Reimplement run using popen
  configurator: Inline err.h functions
  configurator: Print test source without cat
  configurator: Add output cflag option and macro
  configurator: DEFAULT_{COMPILER,FLAGS} for MSVC
  Makefile: Define CFLAGS_FORCE_C_SOURCE macro
  Add appveyor.yml

 Makefile                          |   2 +-
 Makefile-ccan                     |   1 +
 appveyor.yml                      |  37 +++++++
 tools/compile.c                   |  15 ++-
 tools/configurator/configurator.c | 216 +++++++++++++++++++++++++-------------
 tools/tools.h                     |   6 +-
 6 files changed, 194 insertions(+), 83 deletions(-)
 create mode 100644 appveyor.yml

--
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v3 1/7] configurator: Reimplement run using popen

[Kevin Locke]

Rather than using fork+pipe+system+waitpid, most of which are only
available on POSIX-like systems, use popen which is also available on
Windows (under the name _popen).

Changes since v1:
- Create fread_noeintr to avoid EINTR in fread without reading any data.
- Handle short reads from fread.  This can happen with non-conformant
  libc or if EINTR occurs after reading some data.
- Define _POSIX_C_SOURCE for popen/pclose with strict implementations
  which require it (e.g. gcc with -std=c11).

Changes since v2:
- Revert fread_noeintr and short read changes in v1 as unnecessary.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/configurator/configurator.c | 79 +++++++++++++++------------------------
 1 file changed, 31 insertions(+), 48 deletions(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index c7577c0..2ad3fff 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -21,17 +21,19 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#define _POSIX_C_SOURCE 200809L                /* For pclose, popen, strdup */
+
 #include <stdio.h>
 #include <stdbool.h>
 #include <stdlib.h>
-#include <unistd.h>
 #include <err.h>
-#include <sys/types.h>
-#include <sys/wait.h>
-#include <sys/stat.h>
-#include <fcntl.h>
 #include <string.h>

+#ifdef _MSC_VER
+#define popen _popen
+#define pclose _pclose
+#endif
+
 #define DEFAULT_COMPILER "cc"
 #define DEFAULT_FLAGS "-g3 -ggdb -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition"

@@ -375,20 +377,19 @@ static struct test tests[] = {
 	},
 };

-static char *grab_fd(int fd)
+static char *grab_stream(FILE *file)
 {
-	int ret;
-	size_t max, size = 0;
+	size_t max, ret, size = 0;
 	char *buffer;

-	max = 16384;
-	buffer = malloc(max+1);
-	while ((ret = read(fd, buffer + size, max - size)) > 0) {
+	max = BUFSIZ;
+	buffer = malloc(max);
+	while ((ret = fread(buffer+size, 1, max - size, file)) == max - size) {
 		size += ret;
-		if (size == max)
-			buffer = realloc(buffer, max *= 2);
+		buffer = realloc(buffer, max *= 2);
 	}
-	if (ret < 0)
+	size += ret;
+	if (ferror(file))
 		err(1, "reading from command");
 	buffer[size] = '\0';
 	return buffer;
@@ -396,43 +397,25 @@ static char *grab_fd(int fd)

 static char *run(const char *cmd, int *exitstatus)
 {
-	pid_t pid;
-	int p[2];
+	static const char redir[] = " 2>&1";
+	size_t cmdlen;
+	char *cmdredir;
+	FILE *cmdout;
 	char *ret;
-	int status;

-	if (pipe(p) != 0)
-		err(1, "creating pipe");
-
-	pid = fork();
-	if (pid == -1)
-		err(1, "forking");
-
-	if (pid == 0) {
-		if (dup2(p[1], STDOUT_FILENO) != STDOUT_FILENO
-		    || dup2(p[1], STDERR_FILENO) != STDERR_FILENO
-		    || close(p[0]) != 0
-		    || close(STDIN_FILENO) != 0
-		    || open("/dev/null", O_RDONLY) != STDIN_FILENO)
-			exit(128);
-
-		status = system(cmd);
-		if (WIFEXITED(status))
-			exit(WEXITSTATUS(status));
-		/* Here's a hint... */
-		exit(128 + WTERMSIG(status));
-	}
+	cmdlen = strlen(cmd);
+	cmdredir = malloc(cmdlen + sizeof(redir));
+	memcpy(cmdredir, cmd, cmdlen);
+	memcpy(cmdredir + cmdlen, redir, sizeof(redir));
+
+	cmdout = popen(cmdredir, "r");
+	if (!cmdout)
+		err(1, "popen \"%s\"", cmdredir);
+
+	free(cmdredir);

-	close(p[1]);
-	ret = grab_fd(p[0]);
-	/* This shouldn't fail... */
-	if (waitpid(pid, &status, 0) != pid)
-		err(1, "Failed to wait for child");
-	close(p[0]);
-	if (WIFEXITED(status))
-		*exitstatus = WEXITSTATUS(status);
-	else
-		*exitstatus = -WTERMSIG(status);
+	ret = grab_stream(cmdout);
+	*exitstatus = pclose(cmdout);
 	return ret;
 }

--
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v3 2/7] configurator: Inline err.h functions

[Kevin Locke]

On systems where err.h is not provided (e.g. MSVC) configurator must
provide its own definition.  The err module can not be used by
configurator due to its dependency on config.h, so the relevant source
is copied into configurator with the minimum changes necessary.

Changes since v2:
- Use the CCAN err module sources instead of musl libc sources to avoid
  introducing another implementation of these functions.
- Prefix err.h functions with "c12r_" to avoid name conflicts and
  "static declaration follows non-static" errors.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/configurator/configurator.c | 49 ++++++++++++++++++++++++++++++++-------
 1 file changed, 41 insertions(+), 8 deletions(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index 2ad3fff..a7cbfa5 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -3,6 +3,9 @@
  *
  * Copyright 2011 Rusty Russell <rusty@rustcorp.com.au>.  MIT license.
  *
+ * c12r_err, c12r_errx functions copied from ccan/err/err.c
+ * Copyright Rusty Russell <rusty@rustcorp.com.au>. CC0 (Public domain) License.
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
  * in the Software without restriction, including without limitation the rights
@@ -23,10 +26,11 @@
  */
 #define _POSIX_C_SOURCE 200809L                /* For pclose, popen, strdup */

+#include <errno.h>
 #include <stdio.h>
+#include <stdarg.h>
 #include <stdbool.h>
 #include <stdlib.h>
-#include <err.h>
 #include <string.h>

 #ifdef _MSC_VER
@@ -46,6 +50,7 @@
 #define DIR_SEP   "/"
 #endif

+static const char *progname = "";
 static int verbose;

 enum test_style {
@@ -377,6 +382,31 @@ static struct test tests[] = {
 	},
 };

+static void c12r_err(int eval, const char *fmt, ...)
+{
+	int err_errno = errno;
+	va_list ap;
+
+	fprintf(stderr, "%s: ", progname);
+	va_start(ap, fmt);
+	vfprintf(stderr, fmt, ap);
+	va_end(ap);
+	fprintf(stderr, ": %s\n", strerror(err_errno));
+	exit(eval);
+}
+
+static void c12r_errx(int eval, const char *fmt, ...)
+{
+	va_list ap;
+
+	fprintf(stderr, "%s: ", progname);
+	va_start(ap, fmt);
+	vfprintf(stderr, fmt, ap);
+	va_end(ap);
+	fprintf(stderr, "\n");
+	exit(eval);
+}
+
 static char *grab_stream(FILE *file)
 {
 	size_t max, ret, size = 0;
@@ -390,7 +420,7 @@ static char *grab_stream(FILE *file)
 	}
 	size += ret;
 	if (ferror(file))
-		err(1, "reading from command");
+		c12r_err(1, "reading from command");
 	buffer[size] = '\0';
 	return buffer;
 }
@@ -410,7 +440,7 @@ static char *run(const char *cmd, int *exitstatus)

 	cmdout = popen(cmdredir, "r");
 	if (!cmdout)
-		err(1, "popen \"%s\"", cmdredir);
+		c12r_err(1, "popen \"%s\"", cmdredir);

 	free(cmdredir);

@@ -502,7 +532,7 @@ static bool run_test(const char *cmd, struct test *test)

 	outf = fopen(INPUT_FILE, "w");
 	if (!outf)
-		err(1, "creating %s", INPUT_FILE);
+		c12r_err(1, "creating %s", INPUT_FILE);

 	fprintf(outf, "%s", PRE_BOILERPLATE);
 	switch (test->style & ~(EXECUTE|MAY_NOT_COMPILE)) {
@@ -567,8 +597,8 @@ static bool run_test(const char *cmd, struct test *test)
 			       status ? "fail" : "warning",
 			       test->name, status, output);
 		if ((test->style & EXECUTE) && !(test->style & MAY_NOT_COMPILE))
-			errx(1, "Test for %s did not compile:\n%s",
-			     test->name, output);
+			c12r_errx(1, "Test for %s did not compile:\n%s",
+				  test->name, output);
 		test->answer = false;
 		free(output);
 	} else {
@@ -578,8 +608,8 @@ static bool run_test(const char *cmd, struct test *test)
 		if ((test->style & EXECUTE) || (test->style & INSIDE_MAIN)) {
 			output = run("." DIR_SEP OUTPUT_FILE, &status);
 			if (!(test->style & EXECUTE) && status != 0)
-				errx(1, "Test for %s failed with %i:\n%s",
-				     test->name, status, output);
+				c12r_errx(1, "Test for %s failed with %i:\n%s",
+					  test->name, status, output);
 			if (verbose && status)
 				printf("%s exited %i\n", test->name, status);
 			free(output);
@@ -603,6 +633,9 @@ int main(int argc, const char *argv[])
 	const char *default_args[]
 		= { "", DEFAULT_COMPILER, DEFAULT_FLAGS, NULL };

+	if (argc > 0)
+		progname = argv[0];
+
 	if (argc > 1) {
 		if (strcmp(argv[1], "--help") == 0) {
 			printf("Usage: configurator [-v] [<compiler> <flags>...]\n"
--
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v3 3/7] configurator: Print test source without cat

[Kevin Locke]

Windows does not provide cat.  Instead, copy the test source to stdout
using the file stream to which it was written.

Changes since v1:
- Create fwrite_noeintr to avoid EINTR in fwrite without writing any
  data.
- Handle short reads from fread.  This can happen with non-conformant
  libc or if EINTR occurs after reading some data.
- Handle short writes from fwrite.  This can happen with non-conformant
  libc or if EINTR occurs after writing some data.

Changes since v2:
- Revert fwrite_noeintr and short read/write changes.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/configurator/configurator.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index a7cbfa5..73fdc95 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -407,6 +407,22 @@ static void c12r_errx(int eval, const char *fmt, ...)
 	exit(eval);
 }

+static size_t fcopy(FILE *fsrc, FILE *fdst)
+{
+	char buffer[BUFSIZ];
+	size_t rsize, wsize;
+	size_t copied = 0;
+
+	while ((rsize = fread(buffer, 1, BUFSIZ, fsrc)) > 0) {
+		wsize = fwrite(buffer, 1, rsize, fdst);
+		copied += wsize;
+		if (wsize != rsize)
+			break;
+	}
+
+	return copied;
+}
+
 static char *grab_stream(FILE *file)
 {
 	size_t max, ret, size = 0;
@@ -530,7 +546,7 @@ static bool run_test(const char *cmd, struct test *test)
 		}
 	}

-	outf = fopen(INPUT_FILE, "w");
+	outf = fopen(INPUT_FILE, verbose > 1 ? "w+" : "w");
 	if (!outf)
 		c12r_err(1, "creating %s", INPUT_FILE);

@@ -561,11 +577,13 @@ static bool run_test(const char *cmd, struct test *test)
 		abort();

 	}
-	fclose(outf);

-	if (verbose > 1)
-		if (system("cat " INPUT_FILE) == -1)
-			;
+	if (verbose > 1) {
+		fseek(outf, 0, SEEK_SET);
+		fcopy(outf, stdout);
+	}
+
+	fclose(outf);

 	newcmd = strdup(cmd);

--
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v3 4/7] configurator: Add output cflag option and macro

[Kevin Locke]

Unfortunately, not all compilers support -o as a command-line option for
specifying the output file.  Visual Studio cl.exe issues warning D9035
when -o is given, which is detected as a compile warning by the
configurator.

To support such compilers, add the command-line option -O to
configurator which can be used to specify the cflag for setting the
output executable file name.  Additionally define the macro
CCAN_OUTPUT_EXE_CFLAG in config.h and use it when invoking the compiler
(e.g.  from ccanlint).

For reference, the name CCAN_OUTPUT_EXE_CFLAG was chosen to avoid
potential name conflicts in the future due to cl.exe requiring different
flags for different types of output[1] (e.g. object files are /Fo:).

1.  https://msdn.microsoft.com/en-us/library/f1cb223a.aspx

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/compile.c                   | 15 ++++++++----
 tools/configurator/configurator.c | 50 +++++++++++++++++++++++++++------------
 tools/tools.h                     |  6 +++--
 3 files changed, 49 insertions(+), 22 deletions(-)

diff --git a/tools/compile.c b/tools/compile.c
index b88c1d8..e796e3f 100644
--- a/tools/compile.c
+++ b/tools/compile.c
@@ -7,8 +7,12 @@
 #ifndef CCAN_CFLAGS
 #define CCAN_CFLAGS DEFAULT_CCAN_CFLAGS
 #endif
+#ifndef CCAN_OUTPUT_EXE_CFLAG
+#define CCAN_OUTPUT_EXE_CFLAG DEFAULT_CCAN_OUTPUT_EXE_CFLAG
+#endif
 const char *compiler = CCAN_COMPILER;
 const char *cflags = CCAN_CFLAGS;
+const char *outexecflag = CCAN_OUTPUT_EXE_CFLAG;

 bool compile_verbose = false;

@@ -37,8 +41,9 @@ bool compile_object(const void *ctx, const char *cfile, const char *ccandir,
 	if (compile_verbose)
 		printf("Compiling %s\n", outfile);
 	return run_command(ctx, NULL, output,
-			   "%s %s -I%s -c -o %s %s",
-			   compiler, cflags, ccandir, outfile, cfile);
+			   "%s %s -I%s -c %s%s %s",
+			   compiler, cflags, ccandir,
+			   outexecflag, outfile, cfile);
 }

 /* Compile and link single C file, with object files.
@@ -51,7 +56,7 @@ bool compile_and_link(const void *ctx, const char *cfile, const char *ccandir,
 	if (compile_verbose)
 		printf("Compiling and linking %s\n", outfile);
 	return run_command(ctx, NULL, output,
-			   "%s %s -I%s -o %s %s %s %s",
-			   compiler, cflags,
-			   ccandir, outfile, cfile, objs, libs);
+			   "%s %s -I%s %s%s %s %s %s",
+			   compiler, cflags, ccandir,
+			   outexecflag, outfile, cfile, objs, libs);
 }
diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index 73fdc95..4cb84d0 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -40,6 +40,7 @@

 #define DEFAULT_COMPILER "cc"
 #define DEFAULT_FLAGS "-g3 -ggdb -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition"
+#define DEFAULT_OUTPUT_EXE_FLAG "-o"

 #define OUTPUT_FILE "configurator.out"
 #define INPUT_FILE "configuratortest.c"
@@ -465,10 +466,12 @@ static char *run(const char *cmd, int *exitstatus)
 	return ret;
 }

-static char *connect_args(const char *argv[], const char *extra)
+static char *connect_args(const char *argv[], const char *outflag,
+		const char *files)
 {
-	unsigned int i, len = strlen(extra) + 1;
+	unsigned int i;
 	char *ret;
+	size_t len = strlen(outflag) + strlen(files) + 1;

 	for (i = 1; argv[i]; i++)
 		len += 1 + strlen(argv[i]);
@@ -478,10 +481,12 @@ static char *connect_args(const char *argv[], const char *extra)
 	for (i = 1; argv[i]; i++) {
 		strcpy(ret + len, argv[i]);
 		len += strlen(argv[i]);
-		if (argv[i+1])
+		if (argv[i+1] || *outflag)
 			ret[len++] = ' ';
 	}
-	strcpy(ret + len, extra);
+	strcpy(ret + len, outflag);
+	len += strlen(outflag);
+	strcpy(ret + len, files);
 	return ret;
 }

@@ -650,33 +655,47 @@ int main(int argc, const char *argv[])
 	unsigned int i;
 	const char *default_args[]
 		= { "", DEFAULT_COMPILER, DEFAULT_FLAGS, NULL };
+	const char *outflag = DEFAULT_OUTPUT_EXE_FLAG;

 	if (argc > 0)
 		progname = argv[0];

-	if (argc > 1) {
+	while (argc > 1) {
 		if (strcmp(argv[1], "--help") == 0) {
-			printf("Usage: configurator [-v] [<compiler> <flags>...]\n"
-			       "  <compiler> <flags> will have \"-o <outfile> <infile.c>\" appended\n"
-			       "Default: %s %s\n",
-			       DEFAULT_COMPILER, DEFAULT_FLAGS);
+			printf("Usage: configurator [-v] [-O<outflag>] [<compiler> <flags>...]\n"
+			       "  <compiler> <flags> will have \"<outflag> <outfile> <infile.c>\" appended\n"
+			       "Default: %s %s %s\n",
+			       DEFAULT_COMPILER, DEFAULT_FLAGS,
+			       DEFAULT_OUTPUT_EXE_FLAG);
 			exit(0);
 		}
-		if (strcmp(argv[1], "-v") == 0) {
+		if (strncmp(argv[1], "-O", 2) == 0) {
 			argc--;
 			argv++;
-			verbose = 1;
+			outflag = argv[1] + 2;
+			if (!*outflag) {
+				fprintf(stderr,
+					"%s: option requires an argument -- O\n",
+					argv[0]);
+				exit(1);
+			}
+		} else if (strcmp(argv[1], "-v") == 0) {
+			argc--;
+			argv++;
+			verbose++;
 		} else if (strcmp(argv[1], "-vv") == 0) {
 			argc--;
 			argv++;
-			verbose = 2;
+			verbose += 2;
+		} else {
+			break;
 		}
 	}

 	if (argc == 1)
 		argv = default_args;

-	cmd = connect_args(argv, " -o " OUTPUT_FILE " " INPUT_FILE);
+	cmd = connect_args(argv, outflag, OUTPUT_FILE " " INPUT_FILE);
 	for (i = 0; i < sizeof(tests)/sizeof(tests[0]); i++)
 		run_test(cmd, &tests[i]);
 	free(cmd);
@@ -691,9 +710,10 @@ int main(int argc, const char *argv[])
 	printf("#define _GNU_SOURCE /* Always use GNU extensions. */\n");
 	printf("#endif\n");
 	printf("#define CCAN_COMPILER \"%s\"\n", argv[1]);
-	cmd = connect_args(argv+1, "");
-	printf("#define CCAN_CFLAGS \"%s\"\n\n", cmd);
+	cmd = connect_args(argv + 1, "", "");
+	printf("#define CCAN_CFLAGS \"%s\"\n", cmd);
 	free(cmd);
+	printf("#define CCAN_OUTPUT_EXE_CFLAG \"%s\"\n\n", outflag);
 	/* This one implies "#include <ccan/..." works, eg. for tdb2.h */
 	printf("#define HAVE_CCAN 1\n");
 	for (i = 0; i < sizeof(tests)/sizeof(tests[0]); i++)
diff --git a/tools/tools.h b/tools/tools.h
index 3b5c4be..668c91a 100644
--- a/tools/tools.h
+++ b/tools/tools.h
@@ -11,6 +11,7 @@
 /* These are the defaults. */
 #define DEFAULT_CCAN_COMPILER "cc"
 #define DEFAULT_CCAN_CFLAGS "-g"
+#define DEFAULT_CCAN_OUTPUT_EXE_CFLAG "-o"

 #define IDENT_CHARS	"ABCDEFGHIJKLMNOPQRSTUVWXYZ" \
 			"abcdefghijklmnopqrstuvwxyz" \
@@ -20,8 +21,9 @@

 #define COVERAGE_CFLAGS "-fprofile-arcs -ftest-coverage"

-/* Actual compiler and cflags (defaults to CCAN_COMPILER and CCAN_CFLAGS). */
-extern const char *compiler, *cflags;
+/* Actual compiler and cflags
+ * (defaults to CCAN_COMPILER, CCAN_CFLAGS, CCAN_OUTPUT_EXE_CFLAG). */
+extern const char *compiler, *cflags, *outexecflag;

 /* This compiles up the _info file into a temporary. */
 char *compile_info(const void *ctx, const char *dir);
--
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v3 5/7] configurator: DEFAULT_{COMPILER, FLAGS} for MSVC

[Kevin Locke]

When compiling with Visual Studio, use default compiler name and flags
which are likely to work with the known-available compiler.

This is also a convenience for users who may not know what arguments
cl.exe may need to compile the tests.

Changes since v1:
- Use "-option" instead of "/option" to avoid issues running under msys.
- Disable C4200 warning for use of flexible array members, which MSVC
  considers an extension (since it does not fully support C99).

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 tools/configurator/configurator.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/configurator/configurator.c b/tools/configurator/configurator.c
index 4cb84d0..6ec30bd 100644
--- a/tools/configurator/configurator.c
+++ b/tools/configurator/configurator.c
@@ -38,9 +38,21 @@
 #define pclose _pclose
 #endif

+#ifdef _MSC_VER
+#define DEFAULT_COMPILER "cl"
+/* Note:  Dash options avoid POSIX path conversion when used under msys bash
+ *        and are therefore preferred to slash (e.g. -nologo over /nologo)
+ * Note:  Disable Warning 4200 "nonstandard extension used : zero-sized array
+ *        in struct/union" for flexible array members.
+ */
+#define DEFAULT_FLAGS "-nologo -Zi -W4 -wd4200 " \
+	"-D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS"
+#define DEFAULT_OUTPUT_EXE_FLAG "-Fe:"
+#else
 #define DEFAULT_COMPILER "cc"
 #define DEFAULT_FLAGS "-g3 -ggdb -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition"
 #define DEFAULT_OUTPUT_EXE_FLAG "-o"
+#endif

 #define OUTPUT_FILE "configurator.out"
 #define INPUT_FILE "configuratortest.c"
--
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v3 6/7] Makefile: Define CFLAGS_FORCE_C_SOURCE macro

[Kevin Locke]

This macro holds the C compiler flag(s) to force input files to be
recognized as C sources regardless of extension.  It is defined to allow
overriding on the make command line.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 Makefile      | 2 +-
 Makefile-ccan | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 127b875..37ff17d 100644
--- a/Makefile
+++ b/Makefile
@@ -64,7 +64,7 @@ summary-fastcheck/%: tools/ccanlint/ccanlint $(OBJFILES)
 	$(CCANLINT_FAST) -s ccan/$*

 ccan/%/info: ccan/%/_info config.h
-	$(CC) $(CCAN_CFLAGS) -I. -o $@ -x c $<
+	$(CC) $(CCAN_CFLAGS) -I. -o $@ $(CFLAGS_FORCE_C_SOURCE) $<

 all_info: $(MODS:%=ccan/%/info)

diff --git a/Makefile-ccan b/Makefile-ccan
index 5c8dea2..d4a5528 100644
--- a/Makefile-ccan
+++ b/Makefile-ccan
@@ -5,6 +5,7 @@
 #CCAN_CFLAGS=-g -O3 -Wall -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-declarations -Wpointer-arith -Wwrite-strings -Wundef -DCCAN_STR_DEBUG=1
 CCAN_CFLAGS=-g3 -ggdb -Wall -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-declarations -Wpointer-arith -Wwrite-strings -Wundef -DCCAN_STR_DEBUG=1
 CFLAGS = $(CCAN_CFLAGS) -I. $(DEPGEN)
+CFLAGS_FORCE_C_SOURCE = -x c

 MODS := a_star \
 	aga \
--
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

[PATCH v3 7/7] Add appveyor.yml

[Kevin Locke]

This file defines the AppVeyor CI (appveyor.com) settings.

It builds using make+bash under MSYS2 so that the current build system
can be used with minimal changes.  It currently only builds configurator
and generates config.h.

The build and test commands for more thorough testing are left as
comments in appveyor.yml so interested parties can use them as a
starting point for future work.

Note that several compiler errors not related to configurator are
printed due to make attempting to generate and include test-depends.
Although Windows-specific code could be added to Makefile to avoid
these, it seemed unwarranted if the compile errors may be fixed soon.

Changes since v2:
- Add reference to AppVeyor results for canonical repo and basic
  instructions to setup AppVeyor for forks.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
 appveyor.yml | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 appveyor.yml

diff --git a/appveyor.yml b/appveyor.yml
new file mode 100644
index 0000000..0a930c6
--- /dev/null
+++ b/appveyor.yml
@@ -0,0 +1,37 @@
+# AppVeyor CI Configuration
+#
+# Current build status can be viewed at
+# https://ci.appveyor.com/project/rustyrussell/ccan
+#
+# To build test a fork of this repository using AppVeyor:
+# 1. Visit appveyor.com and create a free account.
+# 2. On the Projects tab, select "New Project" then the project hosting site.
+# 3. Grant OAuth permissions to your project git host.
+# 4. Select the repository for your project fork.
+# 5. Review build results and push updates until the desired result is reached.
+
+version: 0.0.{build}
+
+platform:
+- x86
+- amd64
+
+build_script:
+# Reset %PATH% to avoid msys/cygwin DLL conflicts which cause
+# *** fatal error - cygheap base mismatch detected
+- set PATH=C:\Windows\System32;C:\Windows;C:\Windows\System32\Wbem;C:\msys64\usr\bin
+# Set environment variables for chosen compiler
+- "\"C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\vcvarsall.bat\" %Platform%"
+# Ensure config.h can be generated.
+# Note: Dash options avoid POSIX path conversion by msys
+- "make config.h CC=cl CCAN_CFLAGS=\"-nologo -Zi -FS -W4 -wd4200 -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS\" CFLAGS_FORCE_C_SOURCE=-TC OUTPUT_OPTION=-Fo:$@ DEPGEN= LD=link SHELL=bash"
+# FIXME:  Work in progress.  Disabled due to unfixed compile errors.
+# FIXME:  -j%NUMBER_OF_PROCESSORS% won't work without DEPGEN for config.h
+#         It may be possible to generate .d from cl using /showIncludes
+#         See https://stackoverflow.com/q/37685069
+#- "make tools CC=cl CCAN_CFLAGS=\"-nologo -Zi -FS -W4 -wd4200 -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS\" CFLAGS_FORCE_C_SOURCE=-TC OUTPUT_OPTION=-Fo:$@ DEPGEN= LD=link SHELL=bash"
+#- "make -k CC=cl CCAN_CFLAGS=\"-nologo -Zi -FS -W4 -wd4200 -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS\" CFLAGS_FORCE_C_SOURCE=-TC OUTPUT_OPTION=-Fo:$@ DEPGEN= LD=link SHELL=bash"
+
+test_script:
+# FIXME:  Work in progress.  Disabled due to unfixed compile errors.
+#- "make -k check CC=cl CCAN_CFLAGS=\"-nologo -Zi -FS -W4 -wd4200 -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS\" CFLAGS_FORCE_C_SOURCE=-TC OUTPUT_OPTION=-Fo:$@ DEPGEN= LD=link SHELL=bash"
--
2.9.3

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v3 0/7] configurator: Support for Windows and MSVC

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 1969 bytes --]

On Wed, Sep 28, 2016 at 06:44:41PM -0600, Kevin Locke wrote:
> On Thu, 2016-09-29 at 10:21 +1000, David Gibson wrote:
> > Right.  Can you rebase your current patches and post them as a new
> > series.  That's makes my life easier compared to updated versions of
> > each patch in various places in the thread.
> 
> Certainly.  No problem.
> 
> Here is the third revision of the patch series which adds support for
> building configurator with Microsoft Visual C++ and running it on
> Windows.
> 
> Significant changes from v2:
> * Removing the unnecessary EINTR and short read/write handling code.
> * Switching from the musl libc err.h implementation to the CCAN err
>   module one in hopes of reducing maintenance burden and code
>   proliferation.
> * Additional documentation comments for AppVeyor.

I've merged these to master.

Judging by the FIXMEs in appveyor.yml there's still a lot of things
broken, but it's a start.

I set myself up an appveyor account and tweaked your comment to point
at that instead of the hypothetical one Rusty may or may not set up at
some point.

I've actually set up two appveyor projects: one which tracks my github
ccan repository at:
    https://ci.appveyor.com/project/dgibson/ccan-c44x3

And another which tracks the upstream master git for ccan at:
    https://ci.appveyor.com/project/dgibson/ccan

Bizarrely, the github one is working great, whereas the direct git one
is giving errors, even though they're showing as building exactly the
same commit :/.  It kind of looks like the master one is ignoring the
appveyor.yml for no apparent reason - it seems to be looking for a
vcproj file instead of using the build scripts you've supplied.

Oh well, maybe we'll figure it out in future.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v3 0/7] configurator: Support for Windows and MSVC

[Kevin Locke]

On Sat, 2016-10-01 at 01:08 +1000, David Gibson wrote:
> On Wed, Sep 28, 2016 at 06:44:41PM -0600, Kevin Locke wrote:
>> Here is the third revision of the patch series which adds support for
>> building configurator with Microsoft Visual C++ and running it on
>> Windows.
>> 
>> Significant changes from v2:
>> * Removing the unnecessary EINTR and short read/write handling code.
>> * Switching from the musl libc err.h implementation to the CCAN err
>>   module one in hopes of reducing maintenance burden and code
>>   proliferation.
>> * Additional documentation comments for AppVeyor.
> 
> I've merged these to master.

Thanks!  That's great!

> Judging by the FIXMEs in appveyor.yml there's still a lot of things
> broken, but it's a start.

Very true.  At the moment it only tests configurator, although it
generates and ignores compile errors for ccan_depends due to
"-include test-depends".  Fixing that will require the dgraph/tlist
changes previously discussed[1] and possibly other major changes I
haven't discovered yet.  So it's on hold for now unless anyone is
really eager to push it forward.

> I set myself up an appveyor account and tweaked your comment to point
> at that instead of the hypothetical one Rusty may or may not set up at
> some point.
> 
> I've actually set up two appveyor projects: one which tracks my github
> ccan repository at:
>     https://ci.appveyor.com/project/dgibson/ccan-c44x3
> 
> And another which tracks the upstream master git for ccan at:
>     https://ci.appveyor.com/project/dgibson/ccan
> 
> Bizarrely, the github one is working great, whereas the direct git one
> is giving errors, even though they're showing as building exactly the
> same commit :/.  It kind of looks like the master one is ignoring the
> appveyor.yml for no apparent reason - it seems to be looking for a
> vcproj file instead of using the build scripts you've supplied.

Looks great!  Thanks for taking the time to set that up!

I it looks like this is a result of AppVeyor not using appveyor.yml
for custom Git repositories.[2]  I've added a comment to the issue in
their GitHub issue tracker to see what can be done.  If they don't
move on it, it would be possible to copy the settings from
appveyor.yml into the project settings web interface, with the caveat
that they wouldn't be updated from the repo in the future.  I'll
follow up when/if I hear back from the AppVeyor folks.

Thanks again for all of your effort on this,
Kevin

1.  https://lists.ozlabs.org/pipermail/ccan/2016-September/001248.html
2.  https://github.com/appveyor/ci/issues/545
_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

Re: [PATCH v2 12/13] Makefile: Define CFLAGS_FORCE_C_SOURCE macro

[Rusty Russell]

David Gibson <david@gibson.dropbear.id.au> writes:
> On Tue, Sep 27, 2016 at 03:48:33PM +0930, Paul 'Rusty' Russell wrote:
>> David Gibson <david@gibson.dropbear.id.au> writes:
>> > On Thu, Sep 22, 2016 at 09:33:15PM -0600, Kevin Locke wrote:
>> >> This macro holds the C compiler flag(s) to force input files to be
>> >> recognized as C sources regardless of extension.  It is defined to allow
>> >> overriding on the make command line.
>> >> 
>> >> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
>> >
>> > So.. this approach works.
>> >
>> > But I've been wondering for a while if the weird thing we do of having
>> > the _info files not have a .c extension is a good idea.
>> >
>> > I think the underscore enough is probably enough to mark those
>> > special, so things might be less messy if we just called them _info.c.
>> >
>> > Rusty, thoughts?
>> 
>> _info.c breaks the "compile *.c to get the module" rule, though.
>
> Which is already broken by modules which have a non-trivial Makefile
> fragment (like ntdb).

Not at all!  ntdb's Makefile is a relic from when it was out-of-tree.
It builds other things, but the module builds fine.

Cheers,
Rusty.
_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

AppVeyor custom Git URL support (was Re: [PATCH v3 0/7] configurator: Support for Windows and MSVC)

[Kevin Locke]

On Sun, 2016-10-02 at 21:14 -0600, Kevin Locke wrote:
> On Sat, 2016-10-01 at 01:08 +1000, David Gibson wrote:
>> I set myself up an appveyor account and tweaked your comment to point
>> at that instead of the hypothetical one Rusty may or may not set up at
>> some point.
>> 
>> I've actually set up two appveyor projects: one which tracks my github
>> ccan repository at:
>>     https://ci.appveyor.com/project/dgibson/ccan-c44x3
>> 
>> And another which tracks the upstream master git for ccan at:
>>     https://ci.appveyor.com/project/dgibson/ccan
>> 
>> Bizarrely, the github one is working great, whereas the direct git one
>> is giving errors, even though they're showing as building exactly the
>> same commit :/.  It kind of looks like the master one is ignoring the
>> appveyor.yml for no apparent reason - it seems to be looking for a
>> vcproj file instead of using the build scripts you've supplied.
> 
> I it looks like this is a result of AppVeyor not using appveyor.yml
> for custom Git repositories.[2]  I've added a comment to the issue in
> their GitHub issue tracker to see what can be done. [...]

To follow up on this, AppVeyor just released an update which includes
support for YAML configuration files loaded from a user-specified
URL.[1]  I tried creating an AppVeyor project with the YAML
configuration loaded via gitweb using the following URL:

https://git.ozlabs.org/?p=ccan;a=blob_plain;f=appveyor.yml;hb=HEAD

It built successfully using the project appveyor.yml configuration.
You may want to try specifying the above URL as the "Custom
configuration .yml file name" in the General Settings of your CCAN
AppVeyor project using the git.ozlabs.org Git URL.

1.  https://github.com/appveyor/ci/issues/1089#issuecomment-264549196

-- 
Cheers,      |  kevin@kevinlocke.name    | XMPP: kevin@kevinlocke.name
Kevin        |  https://kevinlocke.name  | IRC:   kevinoid on freenode
_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan