Do not allow NULL keys when setting options

This should fix a bug where command line options for the "manual" location
provider cannot override the config file. To keep compatability with previous
versions the command line parser will still special case parsing "-l LAT:LON"
and set the correct options in "manual".

src/gamma-randr.c

@@ -281,12 +281,6 @@ randr_print_help(FILE *f)
 int
 randr_set_option(randr_state_t *state, const char *key, const char *value)
 {
-	if (key == NULL) {
-		fprintf(stderr, _("Missing value for parameter: `%s'.\n"),
-			value);
-		return -1;
-	}
-
 	if (strcasecmp(key, "screen") == 0) {
 		state->screen_num = atoi(value);
 	} else if (strcasecmp(key, "crtc") == 0) {

src/gamma-vidmode.c

@@ -135,12 +135,6 @@ vidmode_print_help(FILE *f)
 int
 vidmode_set_option(vidmode_state_t *state, const char *key, const char *value)
 {
-	if (key == NULL) {
-		fprintf(stderr, _("Missing value for parameter: `%s'.\n"),
-			key);
-		return -1;
-	}
-
 	if (strcasecmp(key, "screen") == 0) {
 		state->screen_num = atoi(value);
 	} else {

src/location-geoclue.c

@@ -18,6 +18,7 @@
 */
 
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 
 #include <geoclue/geoclue-master.h>
@@ -104,6 +105,8 @@ void
 location_geoclue_free(location_geoclue_state_t *state)
 {
 	if (state->position != NULL) g_object_unref(state->position);
+	if (state->provider != NULL) free(state->provider);
+	if (state->provider_path != NULL) free(state->provider_path);
 }
 
 void
@@ -131,32 +134,22 @@ location_geoclue_set_option(location_geoclue_state_t *state,
 	const char *path = NULL;
 
 	/* Parse string value */
-	if (key != NULL && strcasecmp(key, "name") == 0) {
-		if (value != NULL && strcasecmp(value, "default") == 0) {
+	if (strcasecmp(key, "name") == 0) {
+		if (strcasecmp(value, "default") == 0) {
 			provider = DEFAULT_PROVIDER;
-		} else if (value != NULL) {
-			provider = value;
 		} else {
-			fputs(_("Must specify a provider `name' (or use `default').\n"), stderr);
-			return -1;
+			provider = value;
 		}
 
-		/* TODO I don't think we own the string here, should be copied. */
-		state->provider = provider;
-	} else if (key != NULL && strcasecmp(key, "path") == 0) {
+		state->provider = strdup(provider);
+	} else if (strcasecmp(key, "path") == 0) {
 		if (value != NULL && strcasecmp(value, "default") == 0) {
 			path = DEFAULT_PROVIDER_PATH;
-		} else if (value != NULL) {
-			path = value;
 		} else {
-			fputs(_("Must specify a provider `path' (or use `default').\n"), stderr);
-			return -1;
+			path = value;
 		}
 
-		/* TODO I don't think we own the string here, should be copied. */
-		state->provider_path = path;
-	} else if (key == NULL) {
-		return -1;
+		state->provider_path = strdup(path);
 	} else {
 		fprintf(stderr, _("Unknown method parameter: `%s'.\n"), key);
 		return -1;

src/location-geoclue.h

@@ -24,9 +24,9 @@
 #include <geoclue/geoclue-position.h>
 
 typedef struct {
-	GeocluePosition *position;		/* main geoclue object */
-	const char	*provider;		/* name of a geoclue provider */
-	const char	*provider_path;		/* path of the geoclue provider */
+	GeocluePosition *position;  /* main geoclue object */
+	char *provider;             /* name of a geoclue provider */
+	char *provider_path;        /* path of the geoclue provider */
 } location_geoclue_state_t;
 
 int location_geoclue_init(location_geoclue_state_t *state);

src/location-manual.c

@@ -88,15 +88,10 @@ location_manual_set_option(location_manual_state_t *state, const char *key,
 		return -1;
 	}
 
-	if ((key == NULL && isnan(state->lat)) ||
-	    (key != NULL && strcasecmp(key, "lat") == 0)) {
+	if (strcasecmp(key, "lat") == 0) {
 		state->lat = v;
-	} else if ((key == NULL && isnan(state->lon)) ||
-		   (key != NULL && strcasecmp(key, "lon") == 0)) {
+	} else if (strcasecmp(key, "lon") == 0) {
 		state->lon = v;
-	} else if (key == NULL) {
-		fputs(_("Too many arguments.\n"), stderr);
-		return -1;
 	} else {
 		fprintf(stderr, _("Unknown method parameter: `%s'.\n"), key);
 		return -1;

src/redshift.c

@@ -439,17 +439,30 @@ provider_try_start(const location_provider_t *provider,
 	}
 
 	/* Set provider options from command line. */
+	const char *manual_keys[] = { "lat", "lon" };
+	int i = 0;
 	while (args != NULL) {
 		char *next_arg = strchr(args, ':');
 		if (next_arg != NULL) *(next_arg++) = '\0';
 
-		char *key = NULL;
+		const char *key = args;
 		char *value = strchr(args, '=');
-		if (value != NULL) {
-			key = args;
-			*(value++) = '\0';
+		if (value == NULL) {
+			/* The options for the "manual" method can be set
+			   without keys on the command line for convencience
+			   and for backwards compatability. We add the proper
+			   keys here before calling set_option(). */
+			if (strcmp(provider->name, "manual") == 0 &&
+			    i < sizeof(manual_keys)/sizeof(manual_keys[0])) {
+				key = manual_keys[i];
+				value = args;
+			} else {
+				fprintf(stderr, _("Failed to parse option `%s'.\n"),
+					args);
+				return -1;
+			}
 		} else {
-			value = args;
+			*(value++) = '\0';
 		}
 
 		r = provider->set_option(state, key, value);
@@ -464,6 +477,7 @@ provider_try_start(const location_provider_t *provider,
 		}
 
 		args = next_arg;
+		i += 1;
 	}
 
 	/* Start provider. */
@@ -521,13 +535,14 @@ method_try_start(const gamma_method_t *method,
 		char *next_arg = strchr(args, ':');
 		if (next_arg != NULL) *(next_arg++) = '\0';
 
-		char *key = NULL;
+		const char *key = args;
 		char *value = strchr(args, '=');
-		if (value != NULL) {
-			key = args;
-			*(value++) = '\0';
+		if (value == NULL) {
+			fprintf(stderr, _("Failed to parse option `%s'.\n"),
+				args);
+			return -1;
 		} else {
-			value = args;
+			*(value++) = '\0';
 		}
 
 		r = method->set_option(state, key, value);