Determine variable type at runtime

Config variables should be interpreted at run-time, as we don't know if a
zero means false or zero, or if yes means true or "yes".

As a variable has no intrinsic type, git_cvtype is gone and the public
API takes care of enforcing a few rules.

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>

include/git2/config.h

@@ -53,20 +53,6 @@ GIT_EXTERN(int) git_config_open(git_config **cfg_out, const char *path);
 GIT_EXTERN(void) git_config_free(git_config *cfg);
 
 /**
- * Get the value of an integer or boolean config variable.
- *
- * This is a more general function to retrieve the value of a integer
- * or boolean variable.
- *
- * @param cfg where to look for the variable
- * @param name the variable's name
- * @param out pointer to the variable where the value should be stored
- * @param type either GIT_VAR_INT or GIT_VAR_BOOL
- * @return GIT_SUCCESS on success; error code otherwise
- */
-GIT_EXTERN(int) git_config_get(git_config *cfg, const char *name, int *out, git_cvar_type type);
-
-/**
  * Get the value of an integer config variable.
  *
  * @param cfg where to look for the variable
@@ -74,23 +60,20 @@ GIT_EXTERN(int) git_config_get(git_config *cfg, const char *name, int *out, git_
  * @param out pointer to the variable where the value should be stored
  * @return GIT_SUCCESS on success; error code otherwise
  */
-GIT_INLINE(int) git_config_get_int(git_config *cfg, const char *name, int *out)
-{
-	return git_config_get(cfg, name, out, GIT_VAR_INT);
-}
+GIT_EXTERN(int) git_config_get_int(git_config *cfg, const char *name, int *out);
 
 /**
  * Get the value of a boolean config variable.
  *
+ * This function uses the usual C convention of 0 being false and
+ * anything else true.
+ *
  * @param cfg where to look for the variable
  * @param name the variable's name
  * @param out pointer to the variable where the value should be stored
  * @return GIT_SUCCESS on success; error code otherwise
  */
-GIT_INLINE(int) git_config_get_bool(git_config *cfg, const char *name, int *out)
-{
-	return git_config_get(cfg, name, out, GIT_VAR_BOOL);
-}
+GIT_EXTERN(int) git_config_get_bool(git_config *cfg, const char *name, int *out);
 
 /**
  * Get the value of a string config variable.
@@ -104,19 +87,6 @@ GIT_INLINE(int) git_config_get_bool(git_config *cfg, const char *name, int *out)
  * @return GIT_SUCCESS on success; error code otherwise
  */
 GIT_EXTERN(int) git_config_get_string(git_config *cfg, const char *name, const char **out);
-/**
- * Set the value of an integer or boolean config variable.
- *
- * This is a more general function to set the value of a integer or
- * boolean variable.
- *
- * @param cfg where to look for the variable
- * @param name the variable's name
- * @param value the value to store
- * @param type either GIT_VAR_INT or GIT_VAR_BOOL
- * @return GIT_SUCCESS on success; error code otherwise
- */
-GIT_EXTERN(int) git_config_set(git_config *cfg, const char *name, int value, git_cvar_type type);
 
 /**
  * Set the value of an integer config variable.
@@ -126,10 +96,7 @@ GIT_EXTERN(int) git_config_set(git_config *cfg, const char *name, int value, git
  * @param out pointer to the variable where the value should be stored
  * @return GIT_SUCCESS on success; error code otherwise
  */
-GIT_INLINE(int) git_config_set_int(git_config *cfg, const char *name, int value)
-{
-	return git_config_set(cfg, name, value, GIT_VAR_INT);
-}
+GIT_EXTERN(int) git_config_set_int(git_config *cfg, const char *name, int value);
 
 /**
  * Set the value of a boolean config variable.
@@ -139,10 +106,7 @@ GIT_INLINE(int) git_config_set_int(git_config *cfg, const char *name, int value)
  * @param value the value to store
  * @return GIT_SUCCESS on success; error code otherwise
  */
-GIT_INLINE(int) git_config_set_bool(git_config *cfg, const char *name, int value)
-{
-	return git_config_set(cfg, name, value, GIT_VAR_BOOL);
-}
+GIT_EXTERN(int) git_config_set_bool(git_config *cfg, const char *name, int value);
 
 /**
  * Set the value of a string config variable.
@@ -160,17 +124,17 @@ GIT_EXTERN(int) git_config_set_string(git_config *cfg, const char *name, const c
 /**
  * Perform an operation on each config variable.
  *
- * The callback is passed a pointer to a config variable and the data
- * pointer passed to this function. As soon as one of the callback
- * functions returns something other than 0, this function returns
- * that value.
+ * The callback is passed a pointer to a config variable name and the
+ * data pointer passed to this function. As soon as one of the
+ * callback functions returns something other than 0, this function
+ * returns that value.
  *
  * @param cfg where to get the variables from
  * @param callback the function to call on each variable
  * @param data the data to pass to the callback
  * @return GIT_SUCCESS or the return value of the callback which didn't return 0
  */
-GIT_EXTERN(int) git_config_foreach(git_config *cfg, int (*callback)(git_cvar *, void *data), void *data);
+GIT_EXTERN(int) git_config_foreach(git_config *cfg, int (*callback)(const char *, void *data), void *data);
 
 /** @} */
 GIT_END_DECL

include/git2/types.h

@@ -153,13 +153,6 @@ typedef enum {
 	GIT_REF_HAS_PEEL = 8,
 } git_rtype;
 
-/** Config variable type */
-typedef enum {
-	GIT_VAR_INT,  /** Stores an integer value */
-	GIT_VAR_BOOL, /** Stores a boolean value */
-	GIT_VAR_STR   /** Stores a string */
-} git_cvar_type;
-
 /** @} */
 GIT_END_DECL
 

src/config.c

@@ -39,10 +39,8 @@ void git_config_free(git_config *cfg);
 
 static void cvar_free(git_cvar *var)
 {
-	if(var->type == GIT_VAR_STR)
-		free(var->value.string);
-
 	free(var->name);
+	free(var->value);
 	free(var);
 }
 
@@ -133,26 +131,28 @@ void git_config_free(git_config *cfg)
  * Loop over all the variables
  */
 
-int git_config_foreach(git_config *cfg, int (*fn)(git_cvar *, void *), void *data)
+int git_config_foreach(git_config *cfg, int (*fn)(const char *, void *), void *data)
 {
 	int ret = GIT_SUCCESS;
 	git_cvar *var;
-	void *_unused;
+	const void *_unused;
 
 	GIT_HASHTABLE_FOREACH(cfg->vars, _unused, var,
-		ret = fn(var, data);
+		ret = fn(var->name, data);
 		if(ret) break;
 	);
 
 	return ret;
 }
 
-/*
+/**************
  * Setters
- */
+ **************/
 
-int git_config_set(git_config *cfg, const char *name,
-                   int value, git_cvar_type type)
+/*
+ * Internal function to actually set the string value of a variable
+ */
+static int config_set(git_config *cfg, const char *name, const char *value)
 {
 	git_cvar *var = NULL;
 	int error = GIT_SUCCESS;
@@ -170,11 +170,12 @@ int git_config_set(git_config *cfg, const char *name,
 		goto out;
 	}
 
-	var->type = type;
-	if(type == GIT_VAR_BOOL)
-		var->value.boolean = value;
-	else
-		var->value.integer = value;
+	var->value = git__strdup(value);
+	if(var->value == NULL){
+		error = GIT_ENOMEM;
+		cvar_free(var);
+		goto out;
+	}
 
 	error = git_hashtable_insert(cfg->vars, var->name, var);
 	if(error < GIT_SUCCESS)
@@ -184,84 +185,123 @@ int git_config_set(git_config *cfg, const char *name,
 	return error;
 }
 
-int git_config_set_string(git_config *cfg, const char *name, const char *value)
+int git_config_set_int(git_config *cfg, const char *name, int value)
 {
-	git_cvar *var = NULL;
-	int error = GIT_SUCCESS;
-
-	var = git__malloc(sizeof(git_cvar));
-	if(var == NULL){
-		error = GIT_ENOMEM;
-		goto out;
+	char str_value[5]; /* Most numbers should fit in here */
+	int buf_len = sizeof(str_value), ret;
+	char *help_buf;
+
+	if((ret = snprintf(str_value, buf_len, "%d", value)) >= buf_len - 1){
+		/* The number is too large, we need to allocate more memory */
+		buf_len = ret + 1;
+		help_buf = git__malloc(buf_len);
+		snprintf(help_buf, buf_len, "%d", value);
 	}
 
-	var->name = git__strdup(name);
-	if(var->name == NULL){
-		error = GIT_ENOMEM;
-		free(var);
-		goto out;
-	}
+	return config_set(cfg, name, str_value);
+}
 
-	var->value.string = git__strdup(value);
-	if(var->value.string == NULL){
-		error = GIT_ENOMEM;
-		cvar_free(var);
-		goto out;
-	}
+int git_config_set_bool(git_config *cfg, const char *name, int value)
+{
+	const char *str_value;
 
-	var->type = GIT_VAR_STR;
+	if(value == 0)
+		str_value = "false";
+	else
+		str_value = "true";
 
-	error = git_hashtable_insert(cfg->vars, var->name, var);
-	if(error < GIT_SUCCESS)
-		cvar_free(var);
+	return config_set(cfg, name, str_value);
+}
 
- out:
-	return error;
+int git_config_set_string(git_config *cfg, const char *name, const char *value)
+{
+	return config_set(cfg, name, value);
 }
 
+/***********
+ * Getters
+ ***********/
 
 /*
- * Get a config variable's data.
+ * Internal function that actually gets the value in string form
  */
-int git_config_get(git_config *cfg, const char *name,
-                              int *out, git_cvar_type type)
+static int config_get(git_config *cfg, const char *name, const char **out)
 {
 	git_cvar *var;
 	int error = GIT_SUCCESS;
 
 	var = git_hashtable_lookup(cfg->vars, name);
-	if (var == NULL) {
-		error = GIT_ENOTFOUND;
-	} else {
-		if (var->type == type)
-			*out = type == GIT_VAR_INT ?
-			    var->value.integer : var->value.boolean;
+	if (var == NULL)
+		return GIT_ENOTFOUND;
+
+	*out = var->value;
+
+	return error;
+}
+
+int git_config_get_int(git_config *cfg, const char *name, int *out)
+{
+	const char *value;
+	int ret;
+
+	ret = config_get(cfg, name, &value);
+	if(ret < GIT_SUCCESS)
+		return ret;
+
+	ret = sscanf(value, "%d", out);
+	if (ret == 0) /* No items were matched i.e. value isn't a number */
+		return GIT_EINVALIDTYPE;
+	if (ret < 0) {
+		if (errno == EINVAL) /* Format was NULL */
+			return GIT_EINVALIDTYPE;
 		else
-			error = GIT_EINVALIDTYPE;
+			return GIT_EOSERR;
 	}
-	return error;
+
+	return GIT_SUCCESS;
 }
 
-int git_config_get_string(git_config *cfg, const char *name, const char **out)
+int git_config_get_bool(git_config *cfg, const char *name, int *out)
 {
-	git_cvar *var;
+	const char *value;
 	int error = GIT_SUCCESS;
 
-	var = git_hashtable_lookup(cfg->vars, name);
-	if (var == NULL) {
-		error = GIT_ENOTFOUND;
-		goto out;
-	} else if (var->type != GIT_VAR_STR) {
-		error = GIT_EINVALIDTYPE;
-		goto out;
+	error = config_get(cfg, name, &value);
+	if (error < GIT_SUCCESS)
+		return error;
+
+	/* A missing value means true */
+	if (value == NULL) {
+		*out = 1;
+		return GIT_SUCCESS;
 	}
 
-	*out = var->value.string;
+	if (!strcasecmp(value, "true") ||
+		!strcasecmp(value, "yes") ||
+		!strcasecmp(value, "on")){
+		*out = 1;
+		return GIT_SUCCESS;
+	}
+	if (!strcasecmp(value, "false") ||
+		!strcasecmp(value, "no") ||
+		!strcasecmp(value, "off")){
+		*out = 0;
+		return GIT_SUCCESS;
+	}
+
+	/* Try to parse it as an integer */
+	error = git_config_get_int(cfg, name, out);
+	if (error == GIT_SUCCESS)
+		*out = !!(*out);
 
- out:
 	return error;
 }
 
+int git_config_get_string(git_config *cfg, const char *name, const char **out)
+{
+	return config_get(cfg, name, out);
+}
+
 static int cfg_getchar_raw(git_config *cfg)
 {
 	int c;

src/config.h

@@ -17,13 +17,8 @@ struct git_config {
 };
 
 struct git_cvar {
-	git_cvar_type type;
 	char *name;
-	union {
-		unsigned char boolean;
-		long integer;
-		char *string;
-	} value;
+	char *value;
 };
 
 #endif