Skip to contentSkip to conversation

cldcli: better error messages

PATCH In review Pete Zaitcev 2 participants 2 messages Aug 5, 2009

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 cldcli: better error messages

Produce more specific error messages for those who think that host does not necesserily imply port. Also, clean up a bit.

tools/cldcli.c +20 -14
index 758385d..1367b43 100644
@@ -9,13 +9,10 @@
 #include <argp.h>
 #include <poll.h>
 #include <locale.h>
-#include <syslog.h>
 #include <stdarg.h>
 #include <ctype.h>
 #include <cldc.h>
 
-#define PROGRAM_NAME "cld"
-
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
 
 const char *argp_program_version = PACKAGE_VERSION;
@@ -32,8 +29,8 @@ enum thread_codes {
 static struct argp_option options[] = {
 	{ "debug", 'D', "LEVEL", 0,
 	  "Set debug output to LEVEL (0 = off, 2 = max verbose)" },
-	{ "host", 'h', "HOST", 0,
-	  "Connect to remote host.  Used once in normal case (DNS SRV records), or may be specified multiple times." },
+	{ "host", 'h', "HOST:PORT", 0,
+	  "Connect to remote CLD at specified HOST:PORT" },
 	{ "user", 'u', "USER", 0,
 	  "Set username to USER" },
 	{ }
@@ -749,22 +746,32 @@ static bool push_host(const char *arg)
 	struct cldc_host *dr;
 
 	dr = malloc(sizeof(*dr));
-	if (!dr)
-		return false;
+	if (!dr) {
+		fprintf(stderr, "no core for %u", (int) sizeof(*dr));
+		goto err;
+	}
 	memset(dr, 0, sizeof(*dr));
 
 	dr->host = strdup(arg);
-	if (!dr->host)
+	if (!dr->host) {
+		fprintf(stderr, "no core for host"); // don't print, too long
 		goto err_out;
+	}
 
 	colon = strrchr(dr->host, ':');
-	if (!colon)
+	if (!colon) {
+		fprintf(stderr, "no port in host specifier `%s'\n", dr->host);
 		goto err_out_host;
+	}
 
-	if (sscanf(colon, ":%u", &port) != 1)
+	if (sscanf(colon, ":%u", &port) != 1) {
+		fprintf(stderr, "port `%s' is invalid\n", colon+1);
 		goto err_out_host;
-	if (port < 1 || port > 65535)
+	}
+	if (port < 1 || port > 65535) {
+		fprintf(stderr, "port `%s' is out of range\n", colon+1);
 		goto err_out_host;
+	}
 
 	dr->port = port;
 
@@ -778,6 +785,7 @@ err_out_host:
 	free(dr->host);
 err_out:
 	free(dr);
+err:
 	return false;
 }
 
@@ -793,10 +801,8 @@ static error_t parse_opt (int key, char *arg, struct argp_state *state)
 		}
 		break;
 	case 'h':
-		if (!push_host(arg)) {
-			fprintf(stderr, "invalid host: '%s'\n", arg);
+		if (!push_host(arg))
 			argp_usage(state);
-		}
 		break;
 	case 'u':
 		if (strlen(arg) >= CLD_MAX_USERNAME) {
#1 lore raw
Quoted 4 lines · Pete Zaitcev

Produce more specific error messages for those who think that host does not necesserily imply port. Also, clean up a bit.

Generally OK, but needs some minor revisions...

Quoted 36 lines
tools/cldcli.c +3 -7
index 758385d..1367b43 100644
@@ -9,13 +9,10 @@
 #include <argp.h>
 #include <poll.h>
 #include <locale.h>
-#include <syslog.h>
 #include <stdarg.h>
 #include <ctype.h>
 #include <cldc.h>
 
-#define PROGRAM_NAME "cld"
-
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
 
 const char *argp_program_version = PACKAGE_VERSION;
@@ -32,8 +29,8 @@ enum thread_codes {
 static struct argp_option options[] = {
 	{ "debug", 'D', "LEVEL", 0,
 	  "Set debug output to LEVEL (0 = off, 2 = max verbose)" },
-	{ "host", 'h', "HOST", 0,
-	  "Connect to remote host.  Used once in normal case (DNS SRV records), or may be specified multiple times." },
+	{ "host", 'h', "HOST:PORT", 0,
+	  "Connect to remote CLD at specified HOST:PORT" },
 	{ "user", 'u', "USER", 0,
 	  "Set username to USER" },
 	{ }
@@ -749,22 +746,32 @@ static bool push_host(const char *arg)
 	struct cldc_host *dr;
 
 	dr = malloc(sizeof(*dr));
-	if (!dr)
-		return false;
+	if (!dr) {

tools/cldcli.c static bool push_host(const char *arg) · line 758

Quoted 10 lines
tools/cldcli.c +5 -1
+		fprintf(stderr, "no core for %u", (int) sizeof(*dr));
+		goto err;
+	}
 	memset(dr, 0, sizeof(*dr));
 
 	dr->host = strdup(arg);
-	if (!dr->host)
+	if (!dr->host) {
+		fprintf(stderr, "no core for host"); // don't print, too long
 		goto err_out;

Both of these need newlines. And additional consistency would be nice:
the rest of cld/chunkd/tabled tends to use the simple "OOM" string,
though perhaps "OOM (%u bytes)" would be better?

Quoted 41 lines
tools/cldcli.c +11 -6
 	colon = strrchr(dr->host, ':');
-	if (!colon)
+	if (!colon) {
+		fprintf(stderr, "no port in host specifier `%s'\n", dr->host);
 		goto err_out_host;
+	}
 
-	if (sscanf(colon, ":%u", &port) != 1)
+	if (sscanf(colon, ":%u", &port) != 1) {
+		fprintf(stderr, "port `%s' is invalid\n", colon+1);
 		goto err_out_host;
-	if (port < 1 || port > 65535)
+	}
+	if (port < 1 || port > 65535) {
+		fprintf(stderr, "port `%s' is out of range\n", colon+1);
 		goto err_out_host;
+	}
 
 	dr->port = port;
 
@@ -778,6 +785,7 @@ err_out_host:
 	free(dr->host);
 err_out:
 	free(dr);
+err:
 	return false;
 }
 
@@ -793,10 +801,8 @@ static error_t parse_opt (int key, char *arg, struct argp_state *state)
 		}
 		break;
 	case 'h':
-		if (!push_host(arg)) {
-			fprintf(stderr, "invalid host: '%s'\n", arg);
+		if (!push_host(arg))
 			argp_usage(state);
-		}
 		break;
 	case 'u':
 		if (strlen(arg) >= CLD_MAX_USERNAME) {