{"thread":{"id":"31356","subject":"[PATCH 1/3] remote-testsvn.c: Avoid the getline() GNU extension function","startedAt":"2012-08-25T17:13:23Z","lastAt":"2012-08-30T17:25:42Z","messageCount":6,"participants":["Ramsay Jones","Joachim Schmitz","Erik Faye-Lund","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"197838","messageId":"503907B3.9040101@ramsay1.demon.co.uk","threadId":"31356","inReplyTo":null,"subject":"[PATCH 1/3] remote-testsvn.c: Avoid the getline() GNU extension function","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2012-08-25T17:13:23Z","receivedAt":"2012-08-25T17:13:23Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nThe getline() function is a GNU extension (you need to define\n_GNU_SOURCE before including stdio.h) and is, therefore, not\nportable. In particular, getline() is not available on MinGW.\n\nIn order to support non-GNU systems, we replace the call to\ngetline() with (almost) equivalent code using strbuf_getline().\nNote that, unlike getline(), strbuf_getline() removes the\nnewline terminator from the returned string. This difference\nin semantics does not matter at this call-site. Also, we note\nthat the original code was leaking the memory allocated to\n'line' by getline().\n\nSigned-off-by: Ramsay Jones \n---\n\nHi Florian,\n\nCould you please squash this into commit 0320cef0 (\"remote-svn: add\nmarks-file regeneration\", 22-08-2012).\n\nATB,\nRamsay Jones\n\n remote-testsvn.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/remote-testsvn.c b/remote-testsvn.c\nindex 09dc304..d0b81d5 100644\n--- a/remote-testsvn.c\n+++ b/remote-testsvn.c\n@@ -121,9 +121,8 @@ static void regenerate_marks(void)\n \n static void check_or_regenerate_marks(int latestrev) {\n \tFILE *marksfile;\n-\tchar *line = NULL;\n-\tsize_t linelen = 0;\n \tstruct strbuf sb = STRBUF_INIT;\n+\tstruct strbuf line = STRBUF_INIT;\n \tint found = 0;\n \n \tif (latestrev < 1)\n@@ -139,8 +138,8 @@ static void check_or_regenerate_marks(int latestrev) {\n \t\tfclose(marksfile);\n \t} else {\n \t\tstrbuf_addf(&sb, \":%d \", latestrev);\n-\t\twhile (getline(&line, &linelen, marksfile) != -1) {\n-\t\t\tif (!prefixcmp(line, sb.buf)) {\n+\t\twhile (strbuf_getline(&line, marksfile, '\\n') != EOF) {\n+\t\t\tif (!prefixcmp(line.buf, sb.buf)) {\n \t\t\t\tfound++;\n \t\t\t\tbreak;\n \t\t\t}\n@@ -151,6 +150,7 @@ static void check_or_regenerate_marks(int latestrev) {\n \t}\n \tfree_notes(NULL);\n \tstrbuf_release(&sb);\n+\tstrbuf_release(&line);\n }\n \n static int cmd_import(const char *line)\n-- \n1.7.12\n"},{"id":"197843","messageId":"k1b7ij$p8v$1@ger.gmane.org","threadId":"31356","inReplyTo":"503907B3.9040101@ramsay1.demon.co.uk","subject":"Re: [PATCH 1/3] remote-testsvn.c: Avoid the getline() GNU extension function","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2012-08-25T19:03:33Z","receivedAt":"2012-08-25T19:03:33Z","isPatch":true,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"Ramsay Jones wrote:\n> The getline() function is a GNU extension (you need to define\n> _GNU_SOURCE before including stdio.h) and is, therefore, not\n> portable. In particular, getline() is not available on MinGW.\n>\n> In order to support non-GNU systems, we replace the call to\n> getline() with (almost) equivalent code using strbuf_getline().\n> Note that, unlike getline(), strbuf_getline() removes the\n> newline terminator from the returned string. This difference\n> in semantics does not matter at this call-site. Also, we note\n> that the original code was leaking the memory allocated to\n> 'line' by getline().\n>\n> Signed-off-by: Ramsay Jones ramsay@ramsay1.demon.co.uk\n\nTested-by: Joachim Schmitz jojo@schmitz-digital.de\n\n> ---\n>\n> Hi Florian,\n>\n> Could you please squash this into commit 0320cef0 (\"remote-svn: add\n> marks-file regeneration\", 22-08-2012).\n>\n> ATB,\n> Ramsay Jones\n>\n> remote-testsvn.c | 8 ++++----\n> 1 file changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/remote-testsvn.c b/remote-testsvn.c\n> index 09dc304..d0b81d5 100644\n> --- a/remote-testsvn.c\n> +++ b/remote-testsvn.c\n> @@ -121,9 +121,8 @@ static void regenerate_marks(void)\n>\n> static void check_or_regenerate_marks(int latestrev) {\n> FILE *marksfile;\n> - char *line = NULL;\n> - size_t linelen = 0;\n> struct strbuf sb = STRBUF_INIT;\n> + struct strbuf line = STRBUF_INIT;\n> int found = 0;\n>\n> if (latestrev < 1)\n> @@ -139,8 +138,8 @@ static void check_or_regenerate_marks(int\n> latestrev) { fclose(marksfile);\n> } else {\n> strbuf_addf(&sb, \":%d \", latestrev);\n> - while (getline(&line, &linelen, marksfile) != -1) {\n> - if (!prefixcmp(line, sb.buf)) {\n> + while (strbuf_getline(&line, marksfile, '\\n') != EOF) {\n> + if (!prefixcmp(line.buf, sb.buf)) {\n> found++;\n> break;\n> }\n> @@ -151,6 +150,7 @@ static void check_or_regenerate_marks(int\n> latestrev) { }\n> free_notes(NULL);\n> strbuf_release(&sb);\n> + strbuf_release(&line);\n> }\n>\n> static int cmd_import(const char *line)\n\nI'd like to second this request, having the same problem on HP NonStop and \nthis patch fixes it for me too.\n\nBye, Jojo \n"},{"id":"197844","messageId":"CABPQNSZfUO-vjVmBU2+CMbxTXMj_HT=cTFfVbnehUjqOL-vkxw@mail.gmail.com","threadId":"31356","inReplyTo":"503907B3.9040101@ramsay1.demon.co.uk","subject":"Re: [PATCH 1/3] remote-testsvn.c: Avoid the getline() GNU extension function","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-08-25T19:20:44Z","receivedAt":"2012-08-25T19:20:44Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Sat, Aug 25, 2012 at 7:13 PM, Ramsay Jones\n wrote:\n>\n> The getline() function is a GNU extension (you need to define\n> _GNU_SOURCE before including stdio.h) and is, therefore, not\n> portable. In particular, getline() is not available on MinGW.\n\nActually, getline is a POSIX-2008 feature, so it's not (simply) a GNU extension:\n\nhttp://pubs.opengroup.org/onlinepubs/9699919799/functions/getline.html\n"},{"id":"197862","messageId":"7v3939wofb.fsf@alter.siamese.dyndns.org","threadId":"31356","inReplyTo":"CABPQNSZfUO-vjVmBU2+CMbxTXMj_HT=cTFfVbnehUjqOL-vkxw@mail.gmail.com","subject":"Re: [PATCH 1/3] remote-testsvn.c: Avoid the getline() GNU extension function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-26T17:31:04Z","receivedAt":"2012-08-26T17:31:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Erik Faye-Lund writes:\n\n> On Sat, Aug 25, 2012 at 7:13 PM, Ramsay Jones\n> wrote:\n>>\n>> The getline() function is a GNU extension (you need to define\n>> _GNU_SOURCE before including stdio.h) and is, therefore, not\n>> portable. In particular, getline() is not available on MinGW.\n>\n> Actually, getline is a POSIX-2008 feature, so it's not (simply) a GNU extension:\n>\n> http://pubs.opengroup.org/onlinepubs/9699919799/functions/getline.html\n\nTrue, thanks for pointing it out. Justify the change with something\nlike this instead, perhaps?\n\n\tThe getline() function, even though is in POSIX.1-2008, is\n\tnot available on some platforms. Use strbuf_getline() for\n\tportability.\n\nBy the way, the remainder of the proposed log message talks about\nthe difference between getline() that keeps the terminating LF vs\nstrbuf_getline() that drops it. Would strbuf_getwholeline() be a\nbetter alternative?\n"},{"id":"198116","messageId":"503FA0B4.6010401@ramsay1.demon.co.uk","threadId":"31356","inReplyTo":"CABPQNSZfUO-vjVmBU2+CMbxTXMj_HT=cTFfVbnehUjqOL-vkxw@mail.gmail.com","subject":"Re: [PATCH 1/3] remote-testsvn.c: Avoid the getline() GNU extension function","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2012-08-30T17:19:48Z","receivedAt":"2012-08-30T17:19:48Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Erik Faye-Lund wrote:\n> On Sat, Aug 25, 2012 at 7:13 PM, Ramsay Jones\n> wrote:\n>>\n>> The getline() function is a GNU extension (you need to define\n>> _GNU_SOURCE before including stdio.h) and is, therefore, not\n>> portable. In particular, getline() is not available on MinGW.\n> \n> Actually, getline is a POSIX-2008 feature, so it's not (simply) a GNU extension:\n> \n> http://pubs.opengroup.org/onlinepubs/9699919799/functions/getline.html\n\nThanks for the reference. I hadn't noticed this being included into\nPOSIX. Well, it's only been four years ... :-D\n\nThanks again.\n\nATB,\nRamsay Jones\n"},{"id":"198117","messageId":"503FA216.8070704@ramsay1.demon.co.uk","threadId":"31356","inReplyTo":"7v3939wofb.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/3] remote-testsvn.c: Avoid the getline() GNU extension function","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2012-08-30T17:25:42Z","receivedAt":"2012-08-30T17:25:42Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Junio C Hamano wrote:\n> Erik Faye-Lund writes:\n> \n>> On Sat, Aug 25, 2012 at 7:13 PM, Ramsay Jones\n>> wrote:\n>>>\n>>> The getline() function is a GNU extension (you need to define\n>>> _GNU_SOURCE before including stdio.h) and is, therefore, not\n>>> portable. In particular, getline() is not available on MinGW.\n>>\n>> Actually, getline is a POSIX-2008 feature, so it's not (simply) a GNU extension:\n>>\n>> http://pubs.opengroup.org/onlinepubs/9699919799/functions/getline.html\n> \n> True, thanks for pointing it out. Justify the change with something\n> like this instead, perhaps?\n> \n> \tThe getline() function, even though is in POSIX.1-2008, is\n> \tnot available on some platforms. Use strbuf_getline() for\n> \tportability.\n> \n> By the way, the remainder of the proposed log message talks about\n> the difference between getline() that keeps the terminating LF vs\n> strbuf_getline() that drops it. Would strbuf_getwholeline() be a\n> better alternative?\n\nI don't think it matters; I was just making sure Florian was aware\nof the difference (it will not appear in the commit message anyway),\nincluding the memory leak in the original code.\n\nATB,\nRamsay Jones\n"}]}