{"thread":{"id":"29602","subject":"[PATCH] strbuf: move strbuf_readline_fd() from bundle.c to strbuf.{c,h}","startedAt":"2012-02-11T09:50:37Z","lastAt":"2012-02-13T03:54:13Z","messageCount":5,"participants":["徐迪","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"184446","messageId":"CAMocUqRutwERQ64a=9t36Za6Lm8KxpseS0NYbdGKWbixbsXeyw@mail.gmail.com","threadId":"29602","inReplyTo":null,"subject":"[PATCH] strbuf: move strbuf_readline_fd() from bundle.c to strbuf.{c,h}","fromName":"徐迪","fromEmail":"xudifsd@gmail.com","sentAt":"2012-02-11T09:50:37Z","receivedAt":"2012-02-11T09:50:37Z","isPatch":true,"sender":{"key":"xudifsd@gmail.com","avatar":"https://gravatar.com/avatar/260814589d486b8dba6ff105f193d831bf9de466d688a20c53f7a516c7ffe146?d=mp&s=160"},"body":"strbuf_readline_fd() existed in bundle.c since e9ee84cf, but this\nfunction can be used elsewhere, and since it's relevant to strbuf, it\nshould be in strbuf.{c,h}.\n\nSigned-off-by: Xu Di <xudifsd@gmail.com>\n---\n bundle.c |   18 +-----------------\n strbuf.c |   16 ++++++++++++++++\n strbuf.h |    1 +\n 3 files changed, 18 insertions(+), 17 deletions(-)\n\ndiff --git a/bundle.c b/bundle.c\nindex b8acf3c..9344a91 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -7,6 +7,7 @@\n #include \"list-objects.h\"\n #include \"run-command.h\"\n #include \"refs.h\"\n+#include \"strbuf.h\"\n\n static const char bundle_signature[] = \"# v2 git bundle\\n\";\n\n@@ -23,23 +24,6 @@ static void add_to_ref_list(const unsigned char\n*sha1, const char *name,\n \tlist->nr++;\n }\n\n-/* Eventually this should go to strbuf.[ch] */\n-static int strbuf_readline_fd(struct strbuf *sb, int fd)\n-{\n-\tstrbuf_reset(sb);\n-\n-\twhile (1) {\n-\t\tchar ch;\n-\t\tssize_t len = xread(fd, &ch, 1);\n-\t\tif (len <= 0)\n-\t\t\treturn len;\n-\t\tstrbuf_addch(sb, ch);\n-\t\tif (ch == '\\n')\n-\t\t\tbreak;\n-\t}\n-\treturn 0;\n-}\n-\n static int parse_bundle_header(int fd, struct bundle_header *header,\n \t\t\t       const char *report_path)\n {\ndiff --git a/strbuf.c b/strbuf.c\nindex ff0b96b..7532a13 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -282,6 +282,22 @@ void strbuf_addbuf_percentquote(struct strbuf\n*dst, const struct strbuf *src)\n \t}\n }\n\n+int strbuf_readline_fd(struct strbuf *sb, int fd)\n+{\n+\tstrbuf_reset(sb);\n+\n+\twhile (1) {\n+\t\tchar ch;\n+\t\tssize_t len = xread(fd, &ch, 1);\n+\t\tif (len <= 0)\n+\t\t\treturn len;\n+\t\tstrbuf_addch(sb, ch);\n+\t\tif (ch == '\\n')\n+\t\t\tbreak;\n+\t}\n+\treturn 0;\n+}\n+\n size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f)\n {\n \tsize_t res;\ndiff --git a/strbuf.h b/strbuf.h\nindex fbf059f..ecebd11 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -109,6 +109,7 @@ static inline void strbuf_complete_line(struct strbuf *sb)\n }\n\n extern size_t strbuf_fread(struct strbuf *, size_t, FILE *);\n+extern int strbuf_readline_fd(struct strbuf *sb, int fd);\n /* XXX: if read fails, any partial read is undone */\n extern ssize_t strbuf_read(struct strbuf *, int fd, size_t hint);\n extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint);\n-- \n1.7.8.1.749.gb6b3b\n"},{"id":"184507","messageId":"CAMocUqSefruci_ueWHUH6rqz8VFcakHErq8QV_bvghWMyHKVJg@mail.gmail.com","threadId":"29602","inReplyTo":"CAMocUqRutwERQ64a=9t36Za6Lm8KxpseS0NYbdGKWbixbsXeyw@mail.gmail.com","subject":"Re: [PATCH] strbuf: move strbuf_readline_fd() from bundle.c to strbuf.{c,h}","fromName":"徐迪","fromEmail":"xudifsd@gmail.com","sentAt":"2012-02-12T12:25:15Z","receivedAt":"2012-02-12T12:25:15Z","isPatch":true,"sender":{"key":"xudifsd@gmail.com","avatar":"https://gravatar.com/avatar/260814589d486b8dba6ff105f193d831bf9de466d688a20c53f7a516c7ffe146?d=mp&s=160"},"body":"Shouldn't I supposed to receive any information about this patch? Has\nit accepted or not? If not what wrong with this patch?\n\n在 2012年2月11日 下午5:50，徐迪 <xudifsd@gmail.com> 写道：\n> strbuf_readline_fd() existed in bundle.c since e9ee84cf, but this\n> function can be used elsewhere, and since it's relevant to strbuf, it\n> should be in strbuf.{c,h}.\n>\n> Signed-off-by: Xu Di <xudifsd@gmail.com>\n> ---\n>  bundle.c |   18 +-----------------\n>  strbuf.c |   16 ++++++++++++++++\n>  strbuf.h |    1 +\n>  3 files changed, 18 insertions(+), 17 deletions(-)\n>\n> diff --git a/bundle.c b/bundle.c\n> index b8acf3c..9344a91 100644\n> --- a/bundle.c\n> +++ b/bundle.c\n> @@ -7,6 +7,7 @@\n>  #include \"list-objects.h\"\n>  #include \"run-command.h\"\n>  #include \"refs.h\"\n> +#include \"strbuf.h\"\n>\n>  static const char bundle_signature[] = \"# v2 git bundle\\n\";\n>\n> @@ -23,23 +24,6 @@ static void add_to_ref_list(const unsigned char\n> *sha1, const char *name,\n>        list->nr++;\n>  }\n>\n> -/* Eventually this should go to strbuf.[ch] */\n> -static int strbuf_readline_fd(struct strbuf *sb, int fd)\n> -{\n> -       strbuf_reset(sb);\n> -\n> -       while (1) {\n> -               char ch;\n> -               ssize_t len = xread(fd, &ch, 1);\n> -               if (len <= 0)\n> -                       return len;\n> -               strbuf_addch(sb, ch);\n> -               if (ch == '\\n')\n> -                       break;\n> -       }\n> -       return 0;\n> -}\n> -\n>  static int parse_bundle_header(int fd, struct bundle_header *header,\n>                               const char *report_path)\n>  {\n> diff --git a/strbuf.c b/strbuf.c\n> index ff0b96b..7532a13 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -282,6 +282,22 @@ void strbuf_addbuf_percentquote(struct strbuf\n> *dst, const struct strbuf *src)\n>        }\n>  }\n>\n> +int strbuf_readline_fd(struct strbuf *sb, int fd)\n> +{\n> +       strbuf_reset(sb);\n> +\n> +       while (1) {\n> +               char ch;\n> +               ssize_t len = xread(fd, &ch, 1);\n> +               if (len <= 0)\n> +                       return len;\n> +               strbuf_addch(sb, ch);\n> +               if (ch == '\\n')\n> +                       break;\n> +       }\n> +       return 0;\n> +}\n> +\n>  size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f)\n>  {\n>        size_t res;\n> diff --git a/strbuf.h b/strbuf.h\n> index fbf059f..ecebd11 100644\n> --- a/strbuf.h\n> +++ b/strbuf.h\n> @@ -109,6 +109,7 @@ static inline void strbuf_complete_line(struct strbuf *sb)\n>  }\n>\n>  extern size_t strbuf_fread(struct strbuf *, size_t, FILE *);\n> +extern int strbuf_readline_fd(struct strbuf *sb, int fd);\n>  /* XXX: if read fails, any partial read is undone */\n>  extern ssize_t strbuf_read(struct strbuf *, int fd, size_t hint);\n>  extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint);\n> --\n> 1.7.8.1.749.gb6b3b\n"},{"id":"184521","messageId":"7v39agkmjf.fsf@alter.siamese.dyndns.org","threadId":"29602","inReplyTo":"CAMocUqSefruci_ueWHUH6rqz8VFcakHErq8QV_bvghWMyHKVJg@mail.gmail.com","subject":"Re: [PATCH] strbuf: move strbuf_readline_fd() from bundle.c to strbuf.{c,h}","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-12T17:07:48Z","receivedAt":"2012-02-12T17:07:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"徐迪 <xudifsd@gmail.com> writes:\n\n> Shouldn't I supposed to receive any information about this patch?\n\nNot necessarily X-<.\n\nThings can get lost in the noise, and sending a reminder message like you\njust did, after waiting for a few to several days, is considered a good\npractice.\n\n> Has it accepted or not?\n\nAnything and everything being considered for inclusion is reachable from\nthe tip of the 'next' branch; other patches that might turn out to be of\nvalue, or ones that at least deserve more sets of eyeballs, are only in\nthe 'pu' branch.\n\n> If not what wrong with this patch?\n\nThat is not a valid question to ask.\n\nWe do not apply a patch merely because it is _not wrong_.  It must be\nclear why it is a good thing for the project to apply the patch.\n\nYour patch in question is barely a day old and I didn't have a chance to\nlook at it, and I do not think other volunteer reviewers did, either, so\nI don't know if it is wrong or it has merit yet.\n"},{"id":"184522","messageId":"7vy5s8j7d4.fsf@alter.siamese.dyndns.org","threadId":"29602","inReplyTo":"CAMocUqRutwERQ64a=9t36Za6Lm8KxpseS0NYbdGKWbixbsXeyw@mail.gmail.com","subject":"Re: [PATCH] strbuf: move strbuf_readline_fd() from bundle.c to strbuf.{c,h}","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-12T17:20:55Z","receivedAt":"2012-02-12T17:20:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"徐迪 <xudifsd@gmail.com> writes:\n\n> -/* Eventually this should go to strbuf.[ch] */\n> -static int strbuf_readline_fd(struct strbuf *sb, int fd)\n> -{\n\nUnlike other line-oriented input functions in strbuf.c, this hardcodes the\nLF as the terminator, hence is not capable of reading -z output.  As an\ninternal function to the bundle reader implementation, that is perfectly\nfine, because we know lines in bundle ends with '\\n'.\n\nBut if we were to promote this to a generic API, that aspect of this\nfunction needs to be reconsidered, and if we decide to allow non-LF line\nterminator for consistency with other functions, the current callers in\nthis function have to pass an extra parameter \"term\" to it.\n\nWe simply do not have enough datapoint to see if it is worth it to promote\nthis to strbuf.c with such a change, especially when there is no other\ncaller to it.  That is why we said \"Eventually\" here when we introduced\nthis function at e9ee84c (bundle: allowing to read from an unseekable fd,\n2011-10-13).\n\nI do not think the situation changed yet in any way since then.\n"},{"id":"184538","messageId":"CAMocUqTKCptbn5MRkWKusCvsPJXtd2zOA=nCs_wwuYDT6=-z-Q@mail.gmail.com","threadId":"29602","inReplyTo":"7v39agkmjf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] strbuf: move strbuf_readline_fd() from bundle.c to strbuf.{c,h}","fromName":"徐迪","fromEmail":"xudifsd@gmail.com","sentAt":"2012-02-13T03:54:13Z","receivedAt":"2012-02-13T03:54:13Z","isPatch":true,"sender":{"key":"xudifsd@gmail.com","avatar":"https://gravatar.com/avatar/260814589d486b8dba6ff105f193d831bf9de466d688a20c53f7a516c7ffe146?d=mp&s=160"},"body":">> Shouldn't I supposed to receive any information about this patch?\n>\n> Not necessarily X-<.\n>\n> Things can get lost in the noise, and sending a reminder message like you\n> just did, after waiting for a few to several days, is considered a good\n> practice.\n>\n\nOk, excuse me, but this is my first time to make a patch. I'm\ndesperately want to know its fate :)\n\n>> Has it accepted or not?\n>\n> Anything and everything being considered for inclusion is reachable from\n> the tip of the 'next' branch; other patches that might turn out to be of\n> value, or ones that at least deserve more sets of eyeballs, are only in\n> the 'pu' branch.\n>\n\nDoes this mean that we can check 'next' branch to see if any of our\npatches being considered for inclusion?\n"}]}