discover: Don't depend on tftp failure for type detection

Rather than always trying both TFTP client types, do a runtime detection
on first invocation. This can be fixed at build-time with
--with-tftp=TYPE.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

4 files changed, 107 insertions, 40 deletions

diff --git a/configure.ac.in b/configure.ac.in
index 7f13cca..3374a9a 100644
--- a/configure.ac.in
+++ b/configure.ac.in
@@ -205,6 +205,29 @@ DEFINE_HOST_PROG(WGET, wget, [/usr/bin/wget])
 DEFINE_HOST_PROG(IP, ip, [/sbin/ip])
 DEFINE_HOST_PROG(UDHCPC, udhcpc, [/sbin/udhcpc])
 
+AC_ARG_WITH(
+    [tftp],
+    [AS_HELP_STRING([--with-tftp=TYPE],
+        [Use TYPE-type ftp client (either hpa or busybox) [default=runtime-check]]
+    )],
+    [],
+    [with_tftp=detect]
+)
+
+case x$with_tftp in
+'xhpa')
+    tftp_type='TFTP_TYPE_HPA'
+    ;;
+'xbusybox')
+    tftp_type='TFTP_TYPE_BUSYBOX'
+    ;;
+*)
+    tftp_type='TFTP_TYPE_UNKNOWN'
+    ;;
+esac
+
+AC_DEFINE_UNQUOTED(TFTP_TYPE, $tftp_type, [tftp client type])
+
 default_cflags="--std=gnu99 -g \
 	-Wall -W -Wunused -Wstrict-prototypes -Wmissing-prototypes \
 	-Wmissing-declarations -Wredundant-decls"
diff --git a/discover/paths.c b/discover/paths.c
index dbdf16e..82a82b1 100644
--- a/discover/paths.c
+++ b/discover/paths.c
@@ -186,6 +186,41 @@ fail:
 	return NULL;
 }
 
+static enum tftp_type check_tftp_type(void *ctx)
+{
+	const char *argv[] = { pb_system_apps.tftp, "-V", NULL };
+	struct process *process;
+	enum tftp_type type;
+
+	process = process_create(ctx);
+	process->path = pb_system_apps.tftp;
+	process->argv = argv;
+	process->keep_stdout = true;
+	process_run_sync(process);
+
+	if (!process->stdout_buf || process->stdout_len == 0) {
+		pb_log("Can't check TFTP client type!\n");
+		type = TFTP_TYPE_BROKEN;
+
+	} else if (memmem(process->stdout_buf, process->stdout_len,
+				"tftp-hpa", strlen("tftp-hpa"))) {
+		pb_debug("Found TFTP client type: tftp-hpa\n");
+		type = TFTP_TYPE_HPA;
+
+	} else if (memmem(process->stdout_buf, process->stdout_len,
+				"BusyBox", strlen("BusyBox"))) {
+		pb_debug("Found TFTP client type: BusyBox tftp\n");
+		type = TFTP_TYPE_BUSYBOX;
+
+	} else {
+		pb_log("Unknown TFTP client type!\n");
+		type = TFTP_TYPE_BROKEN;
+	}
+
+	process_release(process);
+	return type;
+}
+
 /**
  * pb_load_tftp - Loads a remote file via tftp and returns the local file path.
  *
@@ -202,24 +237,44 @@ static char *load_tftp(void *ctx, struct pb_url *url,
 	char *local;
 	struct process *process;
 
-	local = local_name(ctx);
+	if (tftp_type == TFTP_TYPE_UNKNOWN)
+		tftp_type = check_tftp_type(ctx);
 
-	if (!local)
+	if (tftp_type == TFTP_TYPE_BROKEN)
 		return NULL;
 
-	/* first try busybox tftp args */
+	local = local_name(ctx);
+	if (!local)
+		return NULL;
 
-	p = argv;
-	*p++ = pb_system_apps.tftp;	/* 1 */
-	*p++ = "-g";			/* 2 */
-	*p++ = "-l";			/* 3 */
-	*p++ = local;			/* 4 */
-	*p++ = "-r";			/* 5 */
-	*p++ = url->path;		/* 6 */
-	*p++ = url->host;		/* 7 */
-	if (url->port)
-		*p++ = url->port;	/* 8 */
-	*p++ = NULL;			/* 9 */
+	if (tftp_type == TFTP_TYPE_BUSYBOX) {
+		/* first try busybox tftp args */
+
+		p = argv;
+		*p++ = pb_system_apps.tftp;	/* 1 */
+		*p++ = "-g";			/* 2 */
+		*p++ = "-l";			/* 3 */
+		*p++ = local;			/* 4 */
+		*p++ = "-r";			/* 5 */
+		*p++ = url->path;		/* 6 */
+		*p++ = url->host;		/* 7 */
+		if (url->port)
+			*p++ = url->port;	/* 8 */
+		*p++ = NULL;			/* 9 */
+	} else {
+		p = argv;
+		*p++ = pb_system_apps.tftp;	/* 1 */
+		*p++ = "-m";			/* 2 */
+		*p++ = "binary";		/* 3 */
+		*p++ = url->host;		/* 4 */
+		if (url->port)
+			*p++ = url->port;	/* 5 */
+		*p++ = "-c";			/* 6 */
+		*p++ = "get";			/* 7 */
+		*p++ = url->path;		/* 8 */
+		*p++ = local;			/* 9 */
+		*p++ = NULL;			/* 10 */
+	}
 
 	if (url_data) {
 		process = process_create(ctx);
@@ -237,32 +292,6 @@ static char *load_tftp(void *ctx, struct pb_url *url,
 	if (!result)
 		return local;
 
-	/* next try tftp-hpa args */
-	p = argv;
-	*p++ = pb_system_apps.tftp;	/* 1 */
-	*p++ = "-m";			/* 2 */
-	*p++ = "binary";		/* 3 */
-	*p++ = url->host;		/* 4 */
-	if (url->port)
-		*p++ = url->port;	/* 5 */
-	*p++ = "-c";			/* 6 */
-	*p++ = "get";			/* 7 */
-	*p++ = url->path;		/* 8 */
-	*p++ = local;			/* 9 */
-	*p++ = NULL;			/* 10 */
-
-	if (url_data) {
-		process->argv = argv;
-		result = process_run_async(process);
-		if (result)
-			process_release(process);
-	} else {
-		result = process_run_simple_argv(ctx, argv);
-	}
-
-	if (!result)
-		return local;
-
 	talloc_free(local);
 	return NULL;
 }
diff --git a/lib/system/system.c b/lib/system/system.c
index 6e80b24..c9fe979 100644
--- a/lib/system/system.c
+++ b/lib/system/system.c
@@ -30,6 +30,12 @@ const struct pb_system_apps pb_system_apps = {
 	.udhcpc		= HOST_PROG_UDHCPC,
 };
 
+#ifndef TFTP_TYPE
+#define TFTP_TYPE TFTP_TYPE_UNKNOWN
+#endif
+
+enum tftp_type tftp_type = TFTP_TYPE;
+
 int pb_mkdir_recursive(const char *dir)
 {
 	struct stat statbuf;
diff --git a/lib/system/system.h b/lib/system/system.h
index 271c435..ab25101 100644
--- a/lib/system/system.h
+++ b/lib/system/system.h
@@ -17,6 +17,15 @@ struct pb_system_apps {
 
 extern const struct pb_system_apps pb_system_apps;
 
+enum tftp_type {
+	TFTP_TYPE_BUSYBOX,
+	TFTP_TYPE_HPA,
+	TFTP_TYPE_UNKNOWN,
+	TFTP_TYPE_BROKEN,
+};
+
+extern enum tftp_type tftp_type;
+
 int pb_run_cmd(const char *const *cmd_argv, int wait, int dry_run);
 int pb_mkdir_recursive(const char *dir);
 int pb_rmdir_recursive(const char *base, const char *dir);