{"thread":{"id":"26915","subject":"[RFC 2/2] Future Proofing: Pass around pointers to either functions or data","startedAt":"2011-03-29T15:02:30Z","lastAt":"2011-03-30T22:46:27Z","messageCount":12,"participants":["Michael Witten","Erik Faye-Lund","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"164615","messageId":"3c6b883f-8860-4da2-b328-d912019a4145-mfwitten@gmail.com","threadId":"26915","inReplyTo":"522d5a8a-a674-40bd-91bf-b5b8f88e0f78-mfwitten@gmail.com","subject":"[RFC 1/2] Portability: Convert strictly between function pointers","fromName":"Michael Witten","fromEmail":"mfwitten@gmail.com","sentAt":"2011-03-29T15:02:30Z","receivedAt":"2011-03-29T15:02:30Z","isPatch":false,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"Currently, building git with:\n\n  CFLAGS=\"-std=c99 -pedantic -Wall -Werror -g -02\"\n\ncauses gcc 4.5.2 to fail with:\n\n  ISO C forbids initialization between function pointer and 'void *'\n\nThe C99 standard isn't very explicit about this fact, but the\nfollowing paragraphs seem relevant:\n\n  6.3.2.3.1:\n  A pointer to void may be converted to or from a pointer to\n  any incomplete or object type. A pointer to any incomplete\n  or object type may be converted to a pointer to void and\n  back again; the result shall compare equal to the original\n  pointer.\n\n  6.3.2.3.8:\n  A pointer to a function of one type may be converted to a\n  pointer to a function of another type and back again; the\n  result shall compare equal to the original pointer. If a\n  converted pointer is used to call a function whose type is\n  not compatible with the pointed-to type, the behavior is\n  undefined.\n\n  6.5.4.3:\n  Conversions that involve pointers, other than where\n  permitted by the constraints of 6.5.16.1, shall be\n  specified by means of an explicit cast.\n\nAlso, this website:\n\n  http://www.safercode.com/blog/2008/11/25/generic-function-pointers-in-c-and-void.html\n\nprovides some inspiring comments:\n\n  Why can't we use void* for a Generic Function Pointer?\n  ------------------------------------------------------\n\n  This is because a void* is a pointer to a generic\n  \"data\" type. A void * is used to denote pointers to\n  objects and in some systems, pointers to functions can\n  be larger than pointers to objects. So, if you convert\n  amongst them, you'll lose information and hence, the\n  situation would be undefined and implementation dependent.\n  ...\n\nHowever, that seems like a hypothetical justification for the\nimplicitly undefined behavior of using a variable of type\npointer to void to carry around a value of type\npointer to function.\n\nIn any case, the solution is clear: Rather than converting\nto and from a pointer to void, one can instead convert to\nand from a pointer to any other function type, say:\n\n  void (*)(void)\n\nIs this silly? Probably, but at least it's standards conforming.\n\nUnfortunately (or fortunately?), this requires more\nexplicit casting, as per 6.5.4.3.\n\nSigned-off-by: Michael Witten <mfwitten@gmail.com>\n---\n builtin/fetch-pack.c   |    2 +-\n builtin/receive-pack.c |    2 +-\n cache.h                |    5 +++--\n sha1_file.c            |    2 +-\n transport.c            |    4 ++--\n transport.h            |    4 ++--\n 6 files changed, 10 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex 1724b76..ded1784 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -225,7 +225,7 @@ static void insert_one_alternate_ref(const struct ref *ref, void *unused)\n \n static void insert_alternate_refs(void)\n {\n-\tforeach_alt_odb(refs_from_alternate_cb, insert_one_alternate_ref);\n+\tforeach_alt_odb(refs_from_alternate_cb, (alt_odb_fn_cb)insert_one_alternate_ref);\n }\n \n #define INITIAL_FLUSH 16\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 27050e7..4ce9241 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -738,7 +738,7 @@ static void add_one_alternate_ref(const struct ref *ref, void *unused)\n \n static void add_alternate_refs(void)\n {\n-\tforeach_alt_odb(refs_from_alternate_cb, add_one_alternate_ref);\n+\tforeach_alt_odb(refs_from_alternate_cb, (alt_odb_fn_cb)add_one_alternate_ref);\n }\n \n int cmd_receive_pack(int argc, const char **argv, const char *prefix)\ndiff --git a/cache.h b/cache.h\nindex 9f06d21..ab3407a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -892,8 +892,9 @@ extern struct alternate_object_database {\n } *alt_odb_list;\n extern void prepare_alt_odb(void);\n extern void add_to_alternates_file(const char *reference);\n+typedef void (*alt_odb_fn_cb)(void);\n-typedef int alt_odb_fn(struct alternate_object_database *, void *);\n+typedef int (*alt_odb_fn)(struct alternate_object_database *, alt_odb_fn_cb);\n-extern void foreach_alt_odb(alt_odb_fn, void*);\n+extern void foreach_alt_odb(alt_odb_fn, alt_odb_fn_cb);\n \n struct pack_window {\n \tstruct pack_window *next;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex df0edba..8555dbe 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -388,7 +388,7 @@ void add_to_alternates_file(const char *reference)\n \t\tlink_alt_odb_entries(alt, alt + strlen(alt), '\\n', NULL, 0);\n }\n \n-void foreach_alt_odb(alt_odb_fn fn, void *cb)\n+void foreach_alt_odb(alt_odb_fn fn, alt_odb_fn_cb cb)\n {\n \tstruct alternate_object_database *ent;\n \ndiff --git a/transport.c b/transport.c\nindex a02f79a..f572fa8 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1190,14 +1190,14 @@ literal_copy:\n \treturn xstrdup(url);\n }\n \n-int refs_from_alternate_cb(struct alternate_object_database *e, void *cb)\n+int refs_from_alternate_cb(struct alternate_object_database *e, alt_odb_fn_cb cb)\n {\n \tchar *other;\n \tsize_t len;\n \tstruct remote *remote;\n \tstruct transport *transport;\n \tconst struct ref *extra;\n-\talternate_ref_fn *ref_fn = cb;\n+\talternate_ref_fn ref_fn = (alternate_ref_fn)cb;\n \n \te->name[-1] = '\\0';\n \tother = xstrdup(real_path(e->base));\ndiff --git a/transport.h b/transport.h\nindex efb1968..47ea41a 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -166,7 +166,7 @@ int transport_refs_pushed(struct ref *ref);\n void transport_print_push_status(const char *dest, struct ref *refs,\n \t\t  int verbose, int porcelain, int *nonfastforward);\n \n-typedef void alternate_ref_fn(const struct ref *, void *);\n+typedef void (*alternate_ref_fn)(const struct ref *, void *);\n-extern int refs_from_alternate_cb(struct alternate_object_database *e, void *cb);\n+extern int refs_from_alternate_cb(struct alternate_object_database *e, alt_odb_fn_cb cb);\n \n #endif\n-- \n1.7.4.2.417.g32d76d\n"},{"id":"164614","messageId":"522d5a8a-a674-40bd-91bf-b5b8f88e0f78-mfwitten@gmail.com","threadId":"26915","inReplyTo":"3c6b883f-8860-4da2-b328-d912019a4145-mfwitten@gmail.com","subject":"[RFC 2/2] Future Proofing: Pass around pointers to either functions or data","fromName":"Michael Witten","fromEmail":"mfwitten@gmail.com","sentAt":"2011-03-29T15:28:30Z","receivedAt":"2011-03-29T15:28:30Z","isPatch":false,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"The seemingly unnecessary generality of the alternate object\ndatabase callback infrastructure suggests that there are plans\nfor passing around not only callbacks to callbacks, but also\narbitrary data to callbacks. Unfortunately, as discussed in the\nprevious commit's message, C[99] does not allow for values of type\npointer to function to be stored in variables of type pointer to\nvoid.\n\nThe only unified solution, then, is to pass around a value of\ntype union of both pointer to function and pointer to void; this\nsolution is voiced here as well:\n\n  http://www.safercode.com/blog/2008/11/25/generic-function-pointers-in-c-and-void.html\n\nThat article states:\n\n  How can I use the same variable for object / data\n  -------------------------------------------------\n  and function pointers inter-changeably?\n  ---------------------------------------\n\n  Simple, use a union of a void * and the function pointer\n  (like we described above) and you need not worry about\n  the size because the compiler will automatically choose\n  the largest size for this union which can hold a function\n  pointer.\n\nThis commit introduces such a union and a couple of macros\nfor using it effectively.\n\nSigned-off-by: Michael Witten <mfwitten@gmail.com>\n---\n builtin/fetch-pack.c   |    3 ++-\n builtin/receive-pack.c |    3 ++-\n cache.h                |   16 ++++++++++++++--\n sha1_file.c            |    4 ++--\n transport.c            |    4 ++--\n transport.h            |    2 +-\n 6 files changed, 23 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex ded1784..737b382 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -225,7 +225,8 @@ static void insert_one_alternate_ref(const struct ref *ref, void *unused)\n \n static void insert_alternate_refs(void)\n {\n+\tALT_ODB_FN_INFO_FUNC(info,insert_one_alternate_ref);\n-\tforeach_alt_odb(refs_from_alternate_cb, (alt_odb_fn_cb)insert_one_alternate_ref);\n+\tforeach_alt_odb(refs_from_alternate_cb, info);\n }\n \n #define INITIAL_FLUSH 16\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 4ce9241..51e66d2 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -738,7 +738,8 @@ static void add_one_alternate_ref(const struct ref *ref, void *unused)\n \n static void add_alternate_refs(void)\n {\n+\tALT_ODB_FN_INFO_FUNC(info,add_one_alternate_ref);\n-\tforeach_alt_odb(refs_from_alternate_cb, (alt_odb_fn_cb)add_one_alternate_ref);\n+\tforeach_alt_odb(refs_from_alternate_cb, info);\n }\n \n int cmd_receive_pack(int argc, const char **argv, const char *prefix)\ndiff --git a/cache.h b/cache.h\nindex ab3407a..ca8574a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -893,8 +893,20 @@ extern struct alternate_object_database {\n extern void prepare_alt_odb(void);\n extern void add_to_alternates_file(const char *reference);\n typedef void (*alt_odb_fn_cb)(void);\n+union alt_odb_fn_info {\n+\talt_odb_fn_cb func;\n+\tvoid *data;\n+};\n+#define ALT_ODB_FN_INFO_FUNC(variable, function) \\\n+\tunion alt_odb_fn_info variable = { \\\n+\t\t.func = (alt_odb_fn_cb)function \\\n+\t}\n+#define ALT_ODB_FN_INFO_DATA(variable, data_) \\\n+\tunion alt_odb_fn_info variable = { \\\n+\t\t.data = data_ \\\n+\t}\n-typedef int (*alt_odb_fn)(struct alternate_object_database *, alt_odb_fn_cb);\n+typedef int (*alt_odb_fn)(struct alternate_object_database *, union alt_odb_fn_info);\n-extern void foreach_alt_odb(alt_odb_fn, alt_odb_fn_cb);\n+extern void foreach_alt_odb(alt_odb_fn, union alt_odb_fn_info);\n \n struct pack_window {\n \tstruct pack_window *next;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 8555dbe..2709fea 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -388,13 +388,13 @@ void add_to_alternates_file(const char *reference)\n \t\tlink_alt_odb_entries(alt, alt + strlen(alt), '\\n', NULL, 0);\n }\n \n-void foreach_alt_odb(alt_odb_fn fn, alt_odb_fn_cb cb)\n+void foreach_alt_odb(alt_odb_fn fn, union alt_odb_fn_info info)\n {\n \tstruct alternate_object_database *ent;\n \n \tprepare_alt_odb();\n \tfor (ent = alt_odb_list; ent; ent = ent->next)\n-\t\tif (fn(ent, cb))\n+\t\tif (fn(ent, info))\n \t\t\treturn;\n }\n \ndiff --git a/transport.c b/transport.c\nindex f572fa8..3b44b27 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1190,14 +1190,14 @@ literal_copy:\n \treturn xstrdup(url);\n }\n \n-int refs_from_alternate_cb(struct alternate_object_database *e, alt_odb_fn_cb cb)\n+int refs_from_alternate_cb(struct alternate_object_database *e, union alt_odb_fn_info info)\n {\n \tchar *other;\n \tsize_t len;\n \tstruct remote *remote;\n \tstruct transport *transport;\n \tconst struct ref *extra;\n-\talternate_ref_fn ref_fn = (alternate_ref_fn)cb;\n+\talternate_ref_fn ref_fn = (alternate_ref_fn)info.func;\n \n \te->name[-1] = '\\0';\n \tother = xstrdup(real_path(e->base));\ndiff --git a/transport.h b/transport.h\nindex 47ea41a..d00e547 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -167,6 +167,6 @@ void transport_print_push_status(const char *dest, struct ref *refs,\n \t\t  int verbose, int porcelain, int *nonfastforward);\n \n typedef void (*alternate_ref_fn)(const struct ref *, void *);\n-extern int refs_from_alternate_cb(struct alternate_object_database *e, alt_odb_fn_cb cb);\n+extern int refs_from_alternate_cb(struct alternate_object_database *e, union alt_odb_fn_info info);\n \n #endif\n-- \n1.7.4.2.417.g32d76d\n"},{"id":"164679","messageId":"AANLkTi=nxA0BrLruQ9zc_SajH2ghpZX14Ra7QOo9fMBx@mail.gmail.com","threadId":"26915","inReplyTo":"522d5a8a-a674-40bd-91bf-b5b8f88e0f78-mfwitten@gmail.com","subject":"Re: [RFC 2/2] Future Proofing: Pass around pointers to either functions or data","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-03-30T10:09:46Z","receivedAt":"2011-03-30T10:09:46Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Mar 29, 2011 at 5:28 PM, Michael Witten <mfwitten@gmail.com> wrote:\n> +#define ALT_ODB_FN_INFO_FUNC(variable, function) \\\n> +       union alt_odb_fn_info variable = { \\\n> +               .func = (alt_odb_fn_cb)function \\\n> +       }\n> +#define ALT_ODB_FN_INFO_DATA(variable, data_) \\\n> +       union alt_odb_fn_info variable = { \\\n> +               .data = data_ \\\n> +       }\n\nWe try to stay away from C99 features like this, as it doesn't work on\nall compilers we target.\n"},{"id":"164693","messageId":"429c193b-6bc6-4f13-bd69-af113a4bd145-mfwitten@gmail.com","threadId":"26915","inReplyTo":"AANLkTi=nxA0BrLruQ9zc_SajH2ghpZX14Ra7QOo9fMBx@mail.gmail.com","subject":"Re: [RFC 2/2] Future Proofing: Pass around pointers to either functions or data","fromName":"Michael Witten","fromEmail":"mfwitten@gmail.com","sentAt":"2011-03-30T15:02:28Z","receivedAt":"2011-03-30T15:02:28Z","isPatch":false,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"On Wed, 30 Mar 2011 12:09:46 +0200, Erik Faye-Lund <kusmabite@gmail.com> wrote:\n> On Tue, Mar 29, 2011 at 5:28 PM, Michael Witten <mfwitten@gmail.com> wrote:\n>> +#define ALT_ODB_FN_INFO_FUNC(variable, function) \\\n>> +       union alt_odb_fn_info variable = { \\\n>> +               .func = (alt_odb_fn_cb)function \\\n>> +       }\n>> +#define ALT_ODB_FN_INFO_DATA(variable, data_) \\\n>> +       union alt_odb_fn_info variable = { \\\n>> +               .data = data_ \\\n>> +       }\n>\n> We try to stay away from C99 features like this, as it doesn't work on\n> all compilers we target.\n\nHow about changing it to this:\n\n#define ALT_ODB_FN_INFO_FUNC(variable, function) \\\n\tunion alt_odb_fn_info variable; do { \\\n\t\tvariable.func = (alt_odb_fn_cb)function; \\\n\t} while (0)\n#define ALT_ODB_FN_INFO_DATA(variable, data_) \\\n\tunion alt_odb_fn_info variable; do { \\\n\t\tvariable.data = data_ \\\n\t} while (0)\n"},{"id":"164717","messageId":"7vlizwmm4o.fsf@alter.siamese.dyndns.org","threadId":"26915","inReplyTo":"AANLkTi=nxA0BrLruQ9zc_SajH2ghpZX14Ra7QOo9fMBx@mail.gmail.com","subject":"Re: [RFC 2/2] Future Proofing: Pass around pointers to either functions or data","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-30T19:41:43Z","receivedAt":"2011-03-30T19:41:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Erik Faye-Lund <kusmabite@gmail.com> writes:\n\n> On Tue, Mar 29, 2011 at 5:28 PM, Michael Witten <mfwitten@gmail.com> wrote:\n>> +#define ALT_ODB_FN_INFO_FUNC(variable, function) \\\n>> +       union alt_odb_fn_info variable = { \\\n>> +               .func = (alt_odb_fn_cb)function \\\n>> +       }\n>> +#define ALT_ODB_FN_INFO_DATA(variable, data_) \\\n>> +       union alt_odb_fn_info variable = { \\\n>> +               .data = data_ \\\n>> +       }\n>\n> We try to stay away from C99 features like this, as it doesn't work on\n> all compilers we target.\n\nI had an impression that 1/2 is a good fix but 2/2 is going overboard with\na theoretical exercise without real gain.\n"},{"id":"164731","messageId":"5ed7886ec5e12c8e3a5f45d19625a4f5d2cdd38a.1301521243.git.mfwitten@gmail.com","threadId":"26915","inReplyTo":"cover.1301521243.git.mfwitten@gmail.com","subject":"[RFC 1/3] Alternates API: Untangle the interface","fromName":"Michael Witten","fromEmail":"mfwitten@gmail.com","sentAt":"2011-03-30T20:43:18Z","receivedAt":"2011-03-30T20:43:18Z","isPatch":false,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"These 2 functions:\n\n      cache.h:896:    foreach_alt_odb\n  transport.h:170:    refs_from_alternate_cb\n\nwere unnecessarily coupled; the callback used by:\n\n  refs_from_alternate_cb\n\nwas being passed to the higher-level:\n\n  foreach_alt_odb\n\nwhich necessitated some casting ugliness that has\nactually been invoking undefined behavior[0].\n\nThis commit decouples this relationship, resulting\nin a simpler (albeit slightly more verbose) usage.\nAs a bonus, the undefined behavior is automatically\nresolved.\n\nReferences\n----------\n\n[0] Variables of type pointer to void were being used\nto pass around values of type pointer to function; see:\n\n  [RFC 1/2] Portability: Convert strictly between function pointers\n  Message-ID: <3c6b883f-8860-4da2-b328-d912019a4145-mfwitten@gmail.com>\n\nSigned-off-by: Michael Witten <mfwitten@gmail.com>\n---\n builtin/fetch-pack.c   |    7 ++++++-\n builtin/receive-pack.c |    7 ++++++-\n cache.h                |    4 ++--\n sha1_file.c            |    4 ++--\n transport.c            |    3 +--\n transport.h            |    4 ++--\n 6 files changed, 19 insertions(+), 10 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 9f06d21..71cfcef 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -892,8 +892,8 @@ extern struct alternate_object_database {\n } *alt_odb_list;\n extern void prepare_alt_odb(void);\n extern void add_to_alternates_file(const char *reference);\n-typedef int alt_odb_fn(struct alternate_object_database *, void *);\n+typedef int (*alt_odb_fn)(struct alternate_object_database *);\n-extern void foreach_alt_odb(alt_odb_fn, void*);\n+extern void foreach_alt_odb(alt_odb_fn);\n \n struct pack_window {\n \tstruct pack_window *next;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex df0edba..e55a496 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -388,13 +388,13 @@ void add_to_alternates_file(const char *reference)\n \t\tlink_alt_odb_entries(alt, alt + strlen(alt), '\\n', NULL, 0);\n }\n \n-void foreach_alt_odb(alt_odb_fn fn, void *cb)\n+void foreach_alt_odb(alt_odb_fn fn)\n {\n \tstruct alternate_object_database *ent;\n \n \tprepare_alt_odb();\n \tfor (ent = alt_odb_list; ent; ent = ent->next)\n-\t\tif (fn(ent, cb))\n+\t\tif (fn(ent))\n \t\t\treturn;\n }\n \ndiff --git a/transport.h b/transport.h\nindex efb1968..72a692f 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -166,7 +166,7 @@ int transport_refs_pushed(struct ref *ref);\n void transport_print_push_status(const char *dest, struct ref *refs,\n \t\t  int verbose, int porcelain, int *nonfastforward);\n \n-typedef void alternate_ref_fn(const struct ref *, void *);\n+typedef void (*alternate_ref_fn)(const struct ref *, void *);\n-extern int refs_from_alternate_cb(struct alternate_object_database *e, void *cb);\n+extern int refs_from_alternate_cb(struct alternate_object_database *, alternate_ref_fn);\n \n #endif\ndiff --git a/transport.c b/transport.c\nindex a02f79a..c61723f 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1190,14 +1190,13 @@ literal_copy:\n \treturn xstrdup(url);\n }\n \n-int refs_from_alternate_cb(struct alternate_object_database *e, void *cb)\n+int refs_from_alternate_cb(struct alternate_object_database *e, alternate_ref_fn ref_fn)\n {\n \tchar *other;\n \tsize_t len;\n \tstruct remote *remote;\n \tstruct transport *transport;\n \tconst struct ref *extra;\n-\talternate_ref_fn *ref_fn = cb;\n \n \te->name[-1] = '\\0';\n \tother = xstrdup(real_path(e->base));\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex 85aff02..62ebdb4 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -224,9 +224,14 @@ static void insert_one_alternate_ref(const struct ref *ref, void *unused)\n \trev_list_insert_ref(NULL, ref->old_sha1, 0, NULL);\n }\n \n+static int alt_odb_callback(struct alternate_object_database *d)\n+{\n+\treturn refs_from_alternate_cb(d,insert_one_alternate_ref);\n+}\n+\n static void insert_alternate_refs(void)\n {\n-\tforeach_alt_odb(refs_from_alternate_cb, insert_one_alternate_ref);\n+\tforeach_alt_odb(alt_odb_callback);\n }\n \n #define INITIAL_FLUSH 16\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 27050e7..8ef6301 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -736,9 +736,14 @@ static void add_one_alternate_ref(const struct ref *ref, void *unused)\n \tadd_extra_ref(\".have\", ref->old_sha1, 0);\n }\n \n+static int alt_odb_callback(struct alternate_object_database *d)\n+{\n+\treturn refs_from_alternate_cb(d,add_one_alternate_ref);\n+}\n+\n static void add_alternate_refs(void)\n {\n-\tforeach_alt_odb(refs_from_alternate_cb, add_one_alternate_ref);\n+\tforeach_alt_odb(alt_odb_callback);\n }\n \n int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n-- \n1.7.4.18.g68fe8\n"},{"id":"164732","messageId":"a0e5204fe8c03b3b6af2150198bad51f86b126c4.1301521243.git.mfwitten@gmail.com","threadId":"26915","inReplyTo":"cover.1301521243.git.mfwitten@gmail.com","subject":"[RFC 2/3] Alternates API: Improve naming","fromName":"Michael Witten","fromEmail":"mfwitten@gmail.com","sentAt":"2011-03-30T21:24:24Z","receivedAt":"2011-03-30T21:24:24Z","isPatch":false,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"This should make the API a bit more cohesive.\n\nSigned-off-by: Michael Witten <mfwitten@gmail.com>\n---\n builtin/fetch-pack.c   |    2 +-\n builtin/receive-pack.c |    2 +-\n sha1_file.c            |    6 +++---\n transport.c            |    8 ++++----\n transport.h            |    4 ++--\n 5 files changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/transport.h b/transport.h\nindex 72a692f..88fa484 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -166,7 +166,7 @@ int transport_refs_pushed(struct ref *ref);\n void transport_print_push_status(const char *dest, struct ref *refs,\n \t\t  int verbose, int porcelain, int *nonfastforward);\n \n-typedef void (*alternate_ref_fn)(const struct ref *, void *);\n+typedef void (*transport_alt_refs_fn)(const struct ref *, void *);\n-extern int refs_from_alternate_cb(struct alternate_object_database *, alternate_ref_fn);\n+extern int transport_alt_refs(struct alternate_object_database *, transport_alt_refs_fn);\n \n #endif\ndiff --git a/transport.c b/transport.c\nindex c61723f..53d37a9 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1190,7 +1190,7 @@ literal_copy:\n \treturn xstrdup(url);\n }\n \n-int refs_from_alternate_cb(struct alternate_object_database *e, alternate_ref_fn ref_fn)\n+int transport_alt_refs(struct alternate_object_database *d, transport_alt_refs_fn ref_fn)\n {\n \tchar *other;\n \tsize_t len;\n@@ -1198,9 +1198,9 @@ int refs_from_alternate_cb(struct alternate_object_database *e, alternate_ref_fn\n \tstruct transport *transport;\n \tconst struct ref *extra;\n \n-\te->name[-1] = '\\0';\n+\td->name[-1] = '\\0';\n-\tother = xstrdup(real_path(e->base));\n+\tother = xstrdup(real_path(d->base));\n-\te->name[-1] = '/';\n+\td->name[-1] = '/';\n \tlen = strlen(other);\n \n \twhile (other[len-1] == '/')\ndiff --git a/sha1_file.c b/sha1_file.c\nindex e55a496..a6c1f1f 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -390,11 +390,11 @@ void add_to_alternates_file(const char *reference)\n \n void foreach_alt_odb(alt_odb_fn fn)\n {\n-\tstruct alternate_object_database *ent;\n+\tstruct alternate_object_database *d;\n \n \tprepare_alt_odb();\n-\tfor (ent = alt_odb_list; ent; ent = ent->next)\n+\tfor (d = alt_odb_list; d; d = d->next)\n-\t\tif (fn(ent))\n+\t\tif (fn(d))\n \t\t\treturn;\n }\n \ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex 62ebdb4..b3a9f74 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -226,7 +226,7 @@ static void insert_one_alternate_ref(const struct ref *ref, void *unused)\n \n static int alt_odb_callback(struct alternate_object_database *d)\n {\n-\treturn refs_from_alternate_cb(d,insert_one_alternate_ref);\n+\treturn transport_alt_refs(d,insert_one_alternate_ref);\n }\n \n static void insert_alternate_refs(void)\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 8ef6301..a761332 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -738,7 +738,7 @@ static void add_one_alternate_ref(const struct ref *ref, void *unused)\n \n static int alt_odb_callback(struct alternate_object_database *d)\n {\n-\treturn refs_from_alternate_cb(d,add_one_alternate_ref);\n+\treturn transport_alt_refs(d,add_one_alternate_ref);\n }\n \n static void add_alternate_refs(void)\n-- \n1.7.4.18.g68fe8\n"},{"id":"164733","messageId":"3c357acd119aa836f6cd3298d62cc5098f4c053c.1301521244.git.mfwitten@gmail.com","threadId":"26915","inReplyTo":"cover.1301521243.git.mfwitten@gmail.com","subject":"[RFC 3/3] Alternates API: Remove unused parameter","fromName":"Michael Witten","fromEmail":"mfwitten@gmail.com","sentAt":"2011-03-30T21:28:11Z","receivedAt":"2011-03-30T21:28:11Z","isPatch":false,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"Signed-off-by: Michael Witten <mfwitten@gmail.com>\n---\n builtin/fetch-pack.c   |    2 +-\n builtin/receive-pack.c |    2 +-\n transport.c            |    2 +-\n transport.h            |    2 +-\n 4 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/transport.h b/transport.h\nindex 88fa484..cf6fda4 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -166,7 +166,7 @@ int transport_refs_pushed(struct ref *ref);\n void transport_print_push_status(const char *dest, struct ref *refs,\n \t\t  int verbose, int porcelain, int *nonfastforward);\n \n-typedef void (*transport_alt_refs_fn)(const struct ref *, void *);\n+typedef void (*transport_alt_refs_fn)(const struct ref *);\n extern int transport_alt_refs(struct alternate_object_database *, transport_alt_refs_fn);\n \n #endif\ndiff --git a/transport.c b/transport.c\nindex 53d37a9..165f69e 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1217,7 +1217,7 @@ int transport_alt_refs(struct alternate_object_database *d, transport_alt_refs_f\n \tfor (extra = transport_get_remote_refs(transport);\n \t     extra;\n \t     extra = extra->next)\n-\t\tref_fn(extra, NULL);\n+\t\tref_fn(extra);\n \ttransport_disconnect(transport);\n \tfree(other);\n \treturn 0;\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex b3a9f74..d2e0111 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -219,7 +219,7 @@ static void send_request(int fd, struct strbuf *buf)\n \t\tsafe_write(fd, buf->buf, buf->len);\n }\n \n-static void insert_one_alternate_ref(const struct ref *ref, void *unused)\n+static void insert_one_alternate_ref(const struct ref *ref)\n {\n \trev_list_insert_ref(NULL, ref->old_sha1, 0, NULL);\n }\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex a761332..47a5aaa 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -731,7 +731,7 @@ static int delete_only(struct command *commands)\n \treturn 1;\n }\n \n-static void add_one_alternate_ref(const struct ref *ref, void *unused)\n+static void add_one_alternate_ref(const struct ref *ref)\n {\n \tadd_extra_ref(\".have\", ref->old_sha1, 0);\n }\n-- \n1.7.4.18.g68fe8\n"},{"id":"164730","messageId":"cover.1301521243.git.mfwitten@gmail.com","threadId":"26915","inReplyTo":"3c357acd119aa836f6cd3298d62cc5098f4c053c.1301521244.git.mfwitten@gmail.com","subject":"[RFC 0/3] Alternates API","fromName":"Michael Witten","fromEmail":"mfwitten@gmail.com","sentAt":"2011-03-30T21:40:43Z","receivedAt":"2011-03-30T21:40:43Z","isPatch":false,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"Michael Witten (3):\n  Alternates API: Untangle the interface\n  Alternates API: Improve naming\n  Alternates API: Remove unused parameter\n\n builtin/fetch-pack.c   |    9 +++++++--\n builtin/receive-pack.c |    9 +++++++--\n cache.h                |    4 ++--\n sha1_file.c            |    8 ++++----\n transport.c            |   11 +++++------\n transport.h            |    4 ++--\n 6 files changed, 27 insertions(+), 18 deletions(-)\n\n-- \n1.7.4.18.g68fe8\n"},{"id":"164735","messageId":"7v1v1omeec.fsf@alter.siamese.dyndns.org","threadId":"26915","inReplyTo":"5ed7886ec5e12c8e3a5f45d19625a4f5d2cdd38a.1301521243.git.mfwitten@gmail.com","subject":"Re: [RFC 1/3] Alternates API: Untangle the interface","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-30T22:28:43Z","receivedAt":"2011-03-30T22:28:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Witten <mfwitten@gmail.com> writes:\n\n> diff --git a/transport.c b/transport.c\n> index a02f79a..c61723f 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -1190,14 +1190,13 @@ literal_copy:\n>  \treturn xstrdup(url);\n>  }\n>  \n> -int refs_from_alternate_cb(struct alternate_object_database *e, void *cb)\n> +int refs_from_alternate_cb(struct alternate_object_database *e, alternate_ref_fn ref_fn)\n>  {\n\nAdding an extra \"fn\" is Ok, but I'd rather not to see removal of generic\ncallback data pointer if the _only_ reason you are doing so is \"currently\nnobody uses it\".\n\nBesides, I see \"a function pointer and a data pointer cannot be held in a\nsingle variable\" as an academic mental mastu^wexercise that is useless in\nreal life.  Are there real platforms that matter where (void *) cannot\nhold a pointer to a function?\n"},{"id":"164736","messageId":"7vwrjgkzps.fsf@alter.siamese.dyndns.org","threadId":"26915","inReplyTo":"a0e5204fe8c03b3b6af2150198bad51f86b126c4.1301521243.git.mfwitten@gmail.com","subject":"Re: [RFC 2/3] Alternates API: Improve naming","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-30T22:31:11Z","receivedAt":"2011-03-30T22:31:11Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Witten <mfwitten@gmail.com> writes:\n\n> diff --git a/sha1_file.c b/sha1_file.c\n> index e55a496..a6c1f1f 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -390,11 +390,11 @@ void add_to_alternates_file(const char *reference)\n>  \n>  void foreach_alt_odb(alt_odb_fn fn)\n>  {\n> -\tstruct alternate_object_database *ent;\n> +\tstruct alternate_object_database *d;\n>  \n>  \tprepare_alt_odb();\n> -\tfor (ent = alt_odb_list; ent; ent = ent->next)\n> +\tfor (d = alt_odb_list; d; d = d->next)\n> -\t\tif (fn(ent))\n> +\t\tif (fn(d))\n\nI see this as a naming regression; that pointer variable points at an\nentry in a list, and the callback is very well aware of that too (so the\ns/e/d/g in an earlier hunk also is a naming regression).\n"},{"id":"164737","messageId":"7vsju4kz0c.fsf@alter.siamese.dyndns.org","threadId":"26915","inReplyTo":"3c357acd119aa836f6cd3298d62cc5098f4c053c.1301521244.git.mfwitten@gmail.com","subject":"Re: [RFC 3/3] Alternates API: Remove unused parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-30T22:46:27Z","receivedAt":"2011-03-30T22:46:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Curious; why are your recent patch series all have screwed-up threading?\n\nHere is an earlier one:\n\n [ 120: Michael Witten         ] [PATCH 4/4] Clean: Remove useless parameters from both get_commit_info() functions\n  [  16: Michael Witten         ] [PATCH 0/4] Miscellaneous Improvements\n  [  51: Michael Witten         ] [PATCH 1/4] Typos: t/README\n  [  56: Michael Witten         ] [PATCH 2/4] Clean: Remove superfluous strbuf 'docs'\n  [  21: Michael Witten         ] [PATCH 3/4] Clean: Remove unnecessary `\\' (line continuation)\n\nand this series looks like this:\n\n [  63: Michael Witten         ] [RFC 3/3] Alternates API: Remove unused parameter\n  [  16: Michael Witten         ] [RFC 0/3] Alternates API\n  [ 152: Michael Witten         ] [RFC 1/3] Alternates API: Untangle the interface\n  [  99: Michael Witten         ] [RFC 2/3] Alternates API: Improve naming\n\nDid we break git-send-email recently?  I would personally be worried more\nabout that than the alternates callbacks.\n\nHaving said all that, I didn't like refs_from_alternate_cb interface\nmyself.  I should probably have done for_each_ref_in_alternates() that\ntakes two callback functions, one that decides if it wants to deal with an\nalternate repository given the path to it, and the other that is fed the\nrefs from the alternate.  Both callback functions should take the usual\n\"void *cb_data\" in addition, of course.\n\nIOW, something like...\n\n\ttypedef int (*skip_alternate_fn)(const char *, void *);\n\n\tint for_each_ref_in_alternates(each_ref_fn ref_fn,\n        \t\t\tskip_alternate_fn skip_fn,\n                                void *cb_data);\n"}]}