Re: [PATCH v9 5/8] config: add `git_die_config()` to the config-set API

2014-08-08 Thread Tanay Abhra


On 8/8/2014 12:25 AM, Junio C Hamano wrote:
 Tanay Abhra tanay...@gmail.com writes:
 
 diff --git a/Documentation/technical/api-config.txt 
 b/Documentation/technical/api-config.txt
 index 21f280c..0d8b99b 100644
 --- a/Documentation/technical/api-config.txt
 +++ b/Documentation/technical/api-config.txt
 @@ -155,6 +155,19 @@ as well as retrieval for the queried variable, 
 including:
  Similar to `git_config_get_string`, but expands `~` or `~user` into
  the user's home directory when found at the beginning of the path.
  
 +`git_die_config(const char *key, const char *err, ...)`::
 +
 +First prints the error message specified by the caller in `err` and then
 +dies printing the line number and the file name of the highest priority
 +value for the configuration variable `key`.
 
 Reviewed with a wider context, I notice that this entry alone lacks
 the return type.  I am assuming that this is just an oversight, and
 adding 'void ' in front of the filename to match the next entry is
 simple enough.
 

Yikes, yes, you are right, it's just an oversight. I will send an amended patch.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v9 5/8] config: add `git_die_config()` to the config-set API

2014-08-07 Thread Tanay Abhra
Add `git_die_config` that dies printing the line number and the file name
of the highest priority value for the configuration variable `key`. A custom
error message is also printed before dying, specified by the caller, which can
be skipped if `err` argument is set to NULL.

It has usage in non-callback based config value retrieval where we can
raise an error and die if there is a semantic error.
For example,

if (!git_config_get_value(key, value)){
if (!strcmp(value, foo))
git_config_die(key, value: `%s` is illegal, value);
else
/* do work */
}

Signed-off-by: Tanay Abhra tanay...@gmail.com
---
 Documentation/technical/api-config.txt | 13 
 cache.h|  3 +++
 config.c   | 39 --
 3 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/Documentation/technical/api-config.txt 
b/Documentation/technical/api-config.txt
index 21f280c..0d8b99b 100644
--- a/Documentation/technical/api-config.txt
+++ b/Documentation/technical/api-config.txt
@@ -155,6 +155,19 @@ as well as retrieval for the queried variable, including:
Similar to `git_config_get_string`, but expands `~` or `~user` into
the user's home directory when found at the beginning of the path.
 
+`git_die_config(const char *key, const char *err, ...)`::
+
+   First prints the error message specified by the caller in `err` and then
+   dies printing the line number and the file name of the highest priority
+   value for the configuration variable `key`.
+
+`void git_die_config_linenr(const char *key, const char *filename, int 
linenr)`::
+
+   Helper function which formats the die error message according to the
+   parameters entered. Used by `git_die_config()`. It can be used by 
callers
+   handling `git_config_get_value_multi()` to print the correct error 
message
+   for the desired value.
+
 See test-config.c for usage examples.
 
 Value Parsing Helpers
diff --git a/cache.h b/cache.h
index f11ce41..89a0d51 100644
--- a/cache.h
+++ b/cache.h
@@ -1388,6 +1388,9 @@ struct key_value_info {
int linenr;
 };
 
+extern NORETURN void git_die_config(const char *key, const char *err, ...) 
__attribute__((format(printf, 2, 3)));
+extern NORETURN void git_die_config_linenr(const char *key, const char 
*filename, int linenr);
+
 extern int committer_ident_sufficiently_given(void);
 extern int author_ident_sufficiently_given(void);
 
diff --git a/config.c b/config.c
index 4cefa25..5ae9ab0 100644
--- a/config.c
+++ b/config.c
@@ -1469,8 +1469,12 @@ const struct string_list 
*git_config_get_value_multi(const char *key)
 
 int git_config_get_string_const(const char *key, const char **dest)
 {
+   int ret;
git_config_check_init();
-   return git_configset_get_string_const(the_config_set, key, dest);
+   ret = git_configset_get_string_const(the_config_set, key, dest);
+   if (ret  0)
+   git_die_config(key, NULL);
+   return ret;
 }
 
 int git_config_get_string(const char *key, char **dest)
@@ -1511,8 +1515,39 @@ int git_config_get_maybe_bool(const char *key, int *dest)
 
 int git_config_get_pathname(const char *key, const char **dest)
 {
+   int ret;
git_config_check_init();
-   return git_configset_get_pathname(the_config_set, key, dest);
+   ret = git_configset_get_pathname(the_config_set, key, dest);
+   if (ret  0)
+   git_die_config(key, NULL);
+   return ret;
+}
+
+NORETURN
+void git_die_config_linenr(const char *key, const char *filename, int linenr)
+{
+   if (!filename)
+   die(_(unable to parse '%s' from command-line config), key);
+   else
+   die(_(bad config variable '%s' in file '%s' at line %d),
+   key, filename, linenr);
+}
+
+NORETURN __attribute__((format(printf, 2, 3)))
+void git_die_config(const char *key, const char *err, ...)
+{
+   const struct string_list *values;
+   struct key_value_info *kv_info;
+
+   if (err) {
+   va_list params;
+   va_start(params, err);
+   vreportf(error: , err, params);
+   va_end(params);
+   }
+   values = git_config_get_value_multi(key);
+   kv_info = values-items[values-nr - 1].util;
+   git_die_config_linenr(key, kv_info-filename, kv_info-linenr);
 }
 
 /*
-- 
1.9.0.GIT

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v9 5/8] config: add `git_die_config()` to the config-set API

2014-08-07 Thread Junio C Hamano
Tanay Abhra tanay...@gmail.com writes:

 diff --git a/Documentation/technical/api-config.txt 
 b/Documentation/technical/api-config.txt
 index 21f280c..0d8b99b 100644
 --- a/Documentation/technical/api-config.txt
 +++ b/Documentation/technical/api-config.txt
 @@ -155,6 +155,19 @@ as well as retrieval for the queried variable, including:
   Similar to `git_config_get_string`, but expands `~` or `~user` into
   the user's home directory when found at the beginning of the path.
  
 +`git_die_config(const char *key, const char *err, ...)`::
 +
 + First prints the error message specified by the caller in `err` and then
 + dies printing the line number and the file name of the highest priority
 + value for the configuration variable `key`.

Reviewed with a wider context, I notice that this entry alone lacks
the return type.  I am assuming that this is just an oversight, and
adding 'void ' in front of the filename to match the next entry is
simple enough.

 +`void git_die_config_linenr(const char *key, const char *filename, int 
 linenr)`::
 + ...
 +extern NORETURN void git_die_config(const char *key, const char *err, ...) 
 __attribute__((format(printf, 2, 3)));
 ...
 +NORETURN __attribute__((format(printf, 2, 3)))
 +void git_die_config(const char *key, const char *err, ...)
 +{

My first reaction was that it might make the compiler unhappy to
declare that the err is a printf-like format string and then to
allow some callers to pass NULL to the function.  My build however
does not seem to complain, so perhaps this is OK.

 + const struct string_list *values;
 + struct key_value_info *kv_info;
 +
 + if (err) {
 + va_list params;
 + va_start(params, err);
 + vreportf(error: , err, params);
 + va_end(params);
 + }
 + values = git_config_get_value_multi(key);
 + kv_info = values-items[values-nr - 1].util;
 + git_die_config_linenr(key, kv_info-filename, kv_info-linenr);
  }
  
  /*
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html