Re: [PATCH v2 1/2] string-list: Add string_list initializer helper functions
On 06/16/2014 03:59 PM, Junio C Hamano wrote:
Tanay Abhra tanay...@gmail.com writes:
When a compound construct like a string_list within another
struct is used, the default initializer macros are useless.
For such cases add helper functions for string_list
initialization for both DUP and NODUP modes.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
Sorry, but I do not understand the above useless. Do you mean to
say that xyzzy below cannot be initialized that way?
git.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/git.c b/git.c
index d261575..17714d1 100644
--- a/git.c
+++ b/git.c
@@ -595,11 +595,24 @@ static int run_argv(int *argcp, const char ***argv)
}
+#include string-list.h
+
int main(int argc, char **av)
{
const char **argv = (const char **) av;
const char *cmd;
+ struct compound {
+ int frotz;
+ struct string_list nitfol;
+ } xyzzy = {
+ 314,
+ STRING_LIST_INIT_DUP,
+ };
+ printf(dup-strings is set to %s\n,
+xyzzy.nitfol.strdup_strings ? true : false);
+ return 0;
+
startup_info = git_startup_info;
cmd = git_extract_argv0_path(argv[0]);
I was actually explaining for cases like below,
+struct config_cache_entry {
+ struct hashmap_entry ent;
+ char *key;
+ struct string_list value_list;
+};
+static int config_cache_set_value(const char *key, const char *value)
+{
+ struct hashmap *config_cache;
+ struct config_cache_entry *e;
+
+ config_cache = get_config_cache();
+ e = config_cache_find_entry(key);
+ if (!e) {
+ e = xmalloc(sizeof(*e));
+ hashmap_entry_init(e, strhash(key));
+ e-key = xstrdup(key);
+ string_list_init_dup(e-value_list);
+ string_list_append(e-value_list, value);
+ hashmap_add(config_cache, e);
+ } else {
+ string_list_append(e-value_list, value);
+ }
+ return 0;
+}
Here even if we use an initialization list as you have shown above, I would
have to
check contents of 'struct hashmap_entry', thus totally breaking the
encapsulation
that the string_list macro was providing. There may not be default values for
'struct hashmap_entry' as it may be using internal init function.
Also, I have to dynamically allocate the config_cache_entry struct, thus the
initialization list such as above cannot be used. Two previous reviewers of the
patch suggested I put a preparatory patch with string_list_init functions
because
the default macros will be useless in my case.
Is there another way out?
Cheers,
Tanay abhra.
--
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 v2 1/2] string-list: Add string_list initializer helper functions
Tanay Abhra tanay...@gmail.com writes:
On 06/16/2014 03:59 PM, Junio C Hamano wrote:
Tanay Abhra tanay...@gmail.com writes:
When a compound construct like a string_list within another
struct is used, the default initializer macros are useless.
For such cases add helper functions for string_list
initialization for both DUP and NODUP modes.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
Sorry, but I do not understand the above useless. Do you mean to
say that xyzzy below cannot be initialized that way?
...
I was actually explaining for cases like below,
...
+ string_list_init_dup(e-value_list);
If that is what you wanted to refer to, I would have to say
useless is placing a stress on a wrong place. (I do not see
anything wrong with your new code; it was just the way it was
explained in the proposed log message was misleading).
Structure initialisers are not something you can assign to a
variable anyway, and calling them useless is like complaining how
unwieldty hammers are on screws. Hammers are useless on screws
may not be technically wrong per-se, but the readers won't be helped
by hearing it very much.
Instead, you would want to explain what your new invention, a
screwdriver, is and how it is intended to be used.
We of course have precedences for this kind of thing. STRBUF_INIT
is for definition-time initialisation and strbuf_init() is to
initialise an uninitialised piece of memory to be used as a strbuf.
I tend to think it was a long-time misdesign of string-list API to
have the STRING_LIST_INIT* definition-time initialisers without
having runtime string_list_init*() initialisers, and it is a good
idea to add them to complete the API.
If I were writing the log message for this, I would just say:
The string-list API has STRING_LIST_INIT_* macros to be used
to define variables with initialisers, but lacks functions
to initialise an uninitialised piece of memory to be used as
a string-list at the run-time.
Introduce string_list_init_{dup,nodup}() functions for that.
or something.
--
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 v2 1/2] string-list: Add string_list initializer helper functions
When a compound construct like a string_list within another
struct is used, the default initializer macros are useless.
For such cases add helper functions for string_list
initialization for both DUP and NODUP modes.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
string-list.c | 18 ++
string-list.h | 3 +++
2 files changed, 21 insertions(+)
diff --git a/string-list.c b/string-list.c
index aabb25e..8c3a4eb 100644
--- a/string-list.c
+++ b/string-list.c
@@ -1,6 +1,24 @@
#include cache.h
#include string-list.h
+void string_list_init_nodup(struct string_list *list)
+{
+ list-items = NULL;
+ list-nr = 0;
+ list-alloc = 0;
+ list-strdup_strings = 0;
+ list-cmp = NULL;
+}
+
+void string_list_init_dup(struct string_list *list)
+{
+ list-items = NULL;
+ list-nr = 0;
+ list-alloc = 0;
+ list-strdup_strings = 1;
+ list-cmp = NULL;
+}
+
/* if there is no exact match, point to the index where the entry could be
* inserted */
static int get_entry_index(const struct string_list *list, const char *string,
diff --git a/string-list.h b/string-list.h
index de6769c..8ae3376 100644
--- a/string-list.h
+++ b/string-list.h
@@ -18,6 +18,9 @@ struct string_list {
#define STRING_LIST_INIT_NODUP { NULL, 0, 0, 0 }
#define STRING_LIST_INIT_DUP { NULL, 0, 0, 1 }
+void string_list_init_nodup(struct string_list *list);
+void string_list_init_dup(struct string_list *list);
+
void print_string_list(const struct string_list *p, const char *text);
void string_list_clear(struct string_list *list, int free_util);
--
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 v2 1/2] string-list: Add string_list initializer helper functions
Tanay Abhra tanay...@gmail.com writes:
When a compound construct like a string_list within another
struct is used, the default initializer macros are useless.
For such cases add helper functions for string_list
initialization for both DUP and NODUP modes.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
Sorry, but I do not understand the above useless. Do you mean to
say that xyzzy below cannot be initialized that way?
git.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/git.c b/git.c
index d261575..17714d1 100644
--- a/git.c
+++ b/git.c
@@ -595,11 +595,24 @@ static int run_argv(int *argcp, const char ***argv)
}
+#include string-list.h
+
int main(int argc, char **av)
{
const char **argv = (const char **) av;
const char *cmd;
+ struct compound {
+ int frotz;
+ struct string_list nitfol;
+ } xyzzy = {
+ 314,
+ STRING_LIST_INIT_DUP,
+ };
+ printf(dup-strings is set to %s\n,
+ xyzzy.nitfol.strdup_strings ? true : false);
+ return 0;
+
startup_info = git_startup_info;
cmd = git_extract_argv0_path(argv[0]);
--
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
