{"thread":{"id":"35998","subject":"[PATCH v2 0/2] More object-related docstrings","startedAt":"2014-02-28T16:29:15Z","lastAt":"2014-02-28T21:17:25Z","messageCount":5,"participants":["Michael Haggerty","Nicolas Pitre","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"235633","messageId":"1393604957-7571-1-git-send-email-mhagger@alum.mit.edu","threadId":"35998","inReplyTo":null,"subject":"[PATCH v2 0/2] More object-related docstrings","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-02-28T16:29:15Z","receivedAt":"2014-02-28T16:29:15Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This patch series applies on top of mh/replace-refs-variable-rename,\nsimply because one of the comments refers to the global variable\ncheck_replace_refs by its new name.\n\nThis is a re-roll of patches 1/6 and 6/6 of the series\n\"mh/object-code-cleanup\" that I submitted earlier [1].  Patches 2-5 of\nthat series have already been queued.\n\nThe first patch emphasizes that do_lookup_replace_object() is only\nmeant for internal use, and moves the real docstring for that function\nfrom the header file to the implementation file.\n\nThe second patch changes the docstring for hash_obj() to mention that\nits return value is not consistent across architectures, and adds a\ncomment within the function explaining some points about the\nimplementation that were suggested in the discussion about v1.\n\nThanks to Junio, Christian, Nicolas, Jakub, and Jonathan for their\ncomments on v1.\n\nMichael\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/242469\n\nMichael Haggerty (2):\n  Add docstrings for lookup_replace_object() and\n    do_lookup_replace_object()\n  Document some functions defined in object.c\n\n cache.h          | 13 +++++++++++++\n object.c         | 29 ++++++++++++++++++++++++++++-\n object.h         |  7 +++++++\n replace_object.c |  7 +++++++\n 4 files changed, 55 insertions(+), 1 deletion(-)\n\n-- \n1.8.5.3\n"},{"id":"235634","messageId":"1393604957-7571-2-git-send-email-mhagger@alum.mit.edu","threadId":"35998","inReplyTo":"1393604957-7571-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 1/2] Add docstrings for lookup_replace_object() and do_lookup_replace_object()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-02-28T16:29:16Z","receivedAt":"2014-02-28T16:29:16Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n cache.h          | 13 +++++++++++++\n replace_object.c |  7 +++++++\n 2 files changed, 20 insertions(+)\n\ndiff --git a/cache.h b/cache.h\nindex b039abc..9407560 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -798,13 +798,26 @@ static inline void *read_sha1_file(const unsigned char *sha1, enum object_type *\n {\n \treturn read_sha1_file_extended(sha1, type, size, LOOKUP_REPLACE_OBJECT);\n }\n+\n+/*\n+ * This internal function is only declared here for the benefit of\n+ * lookup_replace_object().  Please do not call it directly.\n+ */\n extern const unsigned char *do_lookup_replace_object(const unsigned char *sha1);\n+\n+/*\n+ * If object sha1 should be replaced, return the replacement object's\n+ * name (replaced recursively, if necessary).  The return value is\n+ * either sha1 or a pointer to a permanently-allocated value.  When\n+ * object replacement is suppressed, always return sha1.\n+ */\n static inline const unsigned char *lookup_replace_object(const unsigned char *sha1)\n {\n \tif (!check_replace_refs)\n \t\treturn sha1;\n \treturn do_lookup_replace_object(sha1);\n }\n+\n static inline const unsigned char *lookup_replace_object_extended(const unsigned char *sha1, unsigned flag)\n {\n \tif (!(flag & LOOKUP_REPLACE_OBJECT))\ndiff --git a/replace_object.c b/replace_object.c\nindex c5cf9f4..31fabde 100644\n--- a/replace_object.c\n+++ b/replace_object.c\n@@ -92,6 +92,13 @@ static void prepare_replace_object(void)\n /* We allow \"recursive\" replacement. Only within reason, though */\n #define MAXREPLACEDEPTH 5\n \n+/*\n+ * If a replacement for object sha1 has been set up, return the\n+ * replacement object's name (replaced recursively, if necessary).\n+ * The return value is either sha1 or a pointer to a\n+ * permanently-allocated value.  This function always respects replace\n+ * references, regardless of the value of check_replace_refs.\n+ */\n const unsigned char *do_lookup_replace_object(const unsigned char *sha1)\n {\n \tint pos, depth = MAXREPLACEDEPTH;\n-- \n1.8.5.3\n"},{"id":"235635","messageId":"1393604957-7571-3-git-send-email-mhagger@alum.mit.edu","threadId":"35998","inReplyTo":"1393604957-7571-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 2/2] Document some functions defined in object.c","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-02-28T16:29:17Z","receivedAt":"2014-02-28T16:29:17Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n object.c | 29 ++++++++++++++++++++++++++++-\n object.h |  7 +++++++\n 2 files changed, 35 insertions(+), 1 deletion(-)\n\ndiff --git a/object.c b/object.c\nindex 584f7ac..57a0890 100644\n--- a/object.c\n+++ b/object.c\n@@ -43,14 +43,32 @@ int type_from_string(const char *str)\n \tdie(\"invalid object type \\\"%s\\\"\", str);\n }\n \n+/*\n+ * Return a numerical hash value between 0 and n-1 for the object with\n+ * the specified sha1.  n must be a power of 2.  Please note that the\n+ * return value is *not* consistent across computer architectures.\n+ */\n static unsigned int hash_obj(const unsigned char *sha1, unsigned int n)\n {\n \tunsigned int hash;\n+\n+\t/*\n+\t * Since the sha1 is essentially random, we just take the\n+\t * required number of bits directly from the first\n+\t * sizeof(unsigned int) bytes of sha1.  First we have to copy\n+\t * the bytes into a properly aligned integer.  If we cared\n+\t * about getting consistent results across architectures, we\n+\t * would have to call ntohl() here, too.\n+\t */\n \tmemcpy(&hash, sha1, sizeof(unsigned int));\n-\t/* Assumes power-of-2 hash sizes in grow_object_hash */\n \treturn hash & (n - 1);\n }\n \n+/*\n+ * Insert obj into the hash table hash, which has length size (which\n+ * must be a power of 2).  On collisions, simply overflow to the next\n+ * empty bucket.\n+ */\n static void insert_obj_hash(struct object *obj, struct object **hash, unsigned int size)\n {\n \tunsigned int j = hash_obj(obj->sha1, size);\n@@ -63,6 +81,10 @@ static void insert_obj_hash(struct object *obj, struct object **hash, unsigned i\n \thash[j] = obj;\n }\n \n+/*\n+ * Look up the record for the given sha1 in the hash map stored in\n+ * obj_hash.  Return NULL if it was not found.\n+ */\n struct object *lookup_object(const unsigned char *sha1)\n {\n \tunsigned int i, first;\n@@ -92,6 +114,11 @@ struct object *lookup_object(const unsigned char *sha1)\n \treturn obj;\n }\n \n+/*\n+ * Increase the size of the hash map stored in obj_hash to the next\n+ * power of 2 (but at least 32).  Copy the existing values to the new\n+ * hash map.\n+ */\n static void grow_object_hash(void)\n {\n \tint i;\ndiff --git a/object.h b/object.h\nindex dc5df8c..732bf4d 100644\n--- a/object.h\n+++ b/object.h\n@@ -42,7 +42,14 @@ struct object {\n extern const char *typename(unsigned int type);\n extern int type_from_string(const char *str);\n \n+/*\n+ * Return the current number of buckets in the object hashmap.\n+ */\n extern unsigned int get_max_object_index(void);\n+\n+/*\n+ * Return the object from the specified bucket in the object hashmap.\n+ */\n extern struct object *get_indexed_object(unsigned int);\n \n /*\n-- \n1.8.5.3\n"},{"id":"235646","messageId":"alpine.LFD.2.11.1402281248470.17677@knanqh.ubzr","threadId":"35998","inReplyTo":"1393604957-7571-3-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH v2 2/2] Document some functions defined in object.c","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2014-02-28T17:49:11Z","receivedAt":"2014-02-28T17:49:11Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Fri, 28 Feb 2014, Michael Haggerty wrote:\n\n> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n\nAcked-by: Nicolas Pitre <nico@fluxnic.net>\n\n> ---\n>  object.c | 29 ++++++++++++++++++++++++++++-\n>  object.h |  7 +++++++\n>  2 files changed, 35 insertions(+), 1 deletion(-)\n> \n> diff --git a/object.c b/object.c\n> index 584f7ac..57a0890 100644\n> --- a/object.c\n> +++ b/object.c\n> @@ -43,14 +43,32 @@ int type_from_string(const char *str)\n>  \tdie(\"invalid object type \\\"%s\\\"\", str);\n>  }\n>  \n> +/*\n> + * Return a numerical hash value between 0 and n-1 for the object with\n> + * the specified sha1.  n must be a power of 2.  Please note that the\n> + * return value is *not* consistent across computer architectures.\n> + */\n>  static unsigned int hash_obj(const unsigned char *sha1, unsigned int n)\n>  {\n>  \tunsigned int hash;\n> +\n> +\t/*\n> +\t * Since the sha1 is essentially random, we just take the\n> +\t * required number of bits directly from the first\n> +\t * sizeof(unsigned int) bytes of sha1.  First we have to copy\n> +\t * the bytes into a properly aligned integer.  If we cared\n> +\t * about getting consistent results across architectures, we\n> +\t * would have to call ntohl() here, too.\n> +\t */\n>  \tmemcpy(&hash, sha1, sizeof(unsigned int));\n> -\t/* Assumes power-of-2 hash sizes in grow_object_hash */\n>  \treturn hash & (n - 1);\n>  }\n>  \n> +/*\n> + * Insert obj into the hash table hash, which has length size (which\n> + * must be a power of 2).  On collisions, simply overflow to the next\n> + * empty bucket.\n> + */\n>  static void insert_obj_hash(struct object *obj, struct object **hash, unsigned int size)\n>  {\n>  \tunsigned int j = hash_obj(obj->sha1, size);\n> @@ -63,6 +81,10 @@ static void insert_obj_hash(struct object *obj, struct object **hash, unsigned i\n>  \thash[j] = obj;\n>  }\n>  \n> +/*\n> + * Look up the record for the given sha1 in the hash map stored in\n> + * obj_hash.  Return NULL if it was not found.\n> + */\n>  struct object *lookup_object(const unsigned char *sha1)\n>  {\n>  \tunsigned int i, first;\n> @@ -92,6 +114,11 @@ struct object *lookup_object(const unsigned char *sha1)\n>  \treturn obj;\n>  }\n>  \n> +/*\n> + * Increase the size of the hash map stored in obj_hash to the next\n> + * power of 2 (but at least 32).  Copy the existing values to the new\n> + * hash map.\n> + */\n>  static void grow_object_hash(void)\n>  {\n>  \tint i;\n> diff --git a/object.h b/object.h\n> index dc5df8c..732bf4d 100644\n> --- a/object.h\n> +++ b/object.h\n> @@ -42,7 +42,14 @@ struct object {\n>  extern const char *typename(unsigned int type);\n>  extern int type_from_string(const char *str);\n>  \n> +/*\n> + * Return the current number of buckets in the object hashmap.\n> + */\n>  extern unsigned int get_max_object_index(void);\n> +\n> +/*\n> + * Return the object from the specified bucket in the object hashmap.\n> + */\n>  extern struct object *get_indexed_object(unsigned int);\n>  \n>  /*\n> -- \n> 1.8.5.3\n> \n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n> \n"},{"id":"235668","messageId":"xmqqk3cfszru.fsf@gitster.dls.corp.google.com","threadId":"35998","inReplyTo":"1393604957-7571-2-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH v2 1/2] Add docstrings for lookup_replace_object() and do_lookup_replace_object()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-28T21:17:25Z","receivedAt":"2014-02-28T21:17:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n> ---\n\nReviewed-by: Junio C Hamano <gitster@pobox.com>\n\nThanks.\n\n>  cache.h          | 13 +++++++++++++\n>  replace_object.c |  7 +++++++\n>  2 files changed, 20 insertions(+)\n>\n> diff --git a/cache.h b/cache.h\n> index b039abc..9407560 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -798,13 +798,26 @@ static inline void *read_sha1_file(const unsigned char *sha1, enum object_type *\n>  {\n>  \treturn read_sha1_file_extended(sha1, type, size, LOOKUP_REPLACE_OBJECT);\n>  }\n> +\n> +/*\n> + * This internal function is only declared here for the benefit of\n> + * lookup_replace_object().  Please do not call it directly.\n> + */\n>  extern const unsigned char *do_lookup_replace_object(const unsigned char *sha1);\n> +\n> +/*\n> + * If object sha1 should be replaced, return the replacement object's\n> + * name (replaced recursively, if necessary).  The return value is\n> + * either sha1 or a pointer to a permanently-allocated value.  When\n> + * object replacement is suppressed, always return sha1.\n> + */\n>  static inline const unsigned char *lookup_replace_object(const unsigned char *sha1)\n>  {\n>  \tif (!check_replace_refs)\n>  \t\treturn sha1;\n>  \treturn do_lookup_replace_object(sha1);\n>  }\n> +\n>  static inline const unsigned char *lookup_replace_object_extended(const unsigned char *sha1, unsigned flag)\n>  {\n>  \tif (!(flag & LOOKUP_REPLACE_OBJECT))\n> diff --git a/replace_object.c b/replace_object.c\n> index c5cf9f4..31fabde 100644\n> --- a/replace_object.c\n> +++ b/replace_object.c\n> @@ -92,6 +92,13 @@ static void prepare_replace_object(void)\n>  /* We allow \"recursive\" replacement. Only within reason, though */\n>  #define MAXREPLACEDEPTH 5\n>  \n> +/*\n> + * If a replacement for object sha1 has been set up, return the\n> + * replacement object's name (replaced recursively, if necessary).\n> + * The return value is either sha1 or a pointer to a\n> + * permanently-allocated value.  This function always respects replace\n> + * references, regardless of the value of check_replace_refs.\n> + */\n>  const unsigned char *do_lookup_replace_object(const unsigned char *sha1)\n>  {\n>  \tint pos, depth = MAXREPLACEDEPTH;\n"}]}