{"thread":{"id":"19900","subject":"Re: [PATCH] tools: fread does not return negative on error","startedAt":"2009-06-22T15:34:31Z","lastAt":"2009-06-25T18:31:09Z","messageCount":14,"participants":["Ingo Molnar","roel kluin","René Scharfe","Junio C Hamano","Johannes Schindelin","Christian Couder","Alex Riesen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"116773","messageId":"20090622153431.GA18466@elte.hu","threadId":"19900","inReplyTo":"4A3FB09D.9050903@gmail.com","subject":"Re: [PATCH] tools: fread does not return negative on error","fromName":"Ingo Molnar","fromEmail":"mingo@elte.hu","sentAt":"2009-06-22T15:34:31Z","receivedAt":"2009-06-22T15:34:31Z","isPatch":true,"sender":{"key":"mingo@elte.hu","avatar":null},"body":"\n* Roel Kluin wrote:\n\n> size_t res cannot be less than 0. fread returns 0 on error.\n> \n> Signed-off-by: Roel Kluin \n> ---\n> Is this correct? please review.\n> \n> diff --git a/tools/perf/util/strbuf.c b/tools/perf/util/strbuf.c\n> index eaba093..376a337 100644\n> --- a/tools/perf/util/strbuf.c\n> +++ b/tools/perf/util/strbuf.c\n> @@ -259,7 +259,7 @@ size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f)\n> \tres = fread(sb->buf + sb->len, 1, size, f);\n> \tif (res > 0)\n> \t\tstrbuf_setlen(sb, sb->len + res);\n> -\telse if (res < 0 && oldalloc == 0)\n> +\telse if (res == 0 && oldalloc == 0)\n> \t\tstrbuf_release(sb);\n> \treturn res;\n\nThis comes straight from Git's strbuf.c so i've Cc:-ed the Git list.\n\nRoel, did you get some compiler warning that made you look at this \ncode?\n\n\tIngo\n"},{"id":"116774","messageId":"25e057c00906220847t15425f38maf486c291d1d2468@mail.gmail.com","threadId":"19900","inReplyTo":"20090622153431.GA18466@elte.hu","subject":"Re: [PATCH] tools: fread does not return negative on error","fromName":"roel kluin","fromEmail":"roel.kluin@gmail.com","sentAt":"2009-06-22T15:47:07Z","receivedAt":"2009-06-22T15:47:07Z","isPatch":true,"sender":{"key":"roel.kluin@gmail.com","avatar":null},"body":"On Mon, Jun 22, 2009 at 5:34 PM, Ingo Molnar wrote:\n>\n> * Roel Kluin wrote:\n>\n>> size_t res cannot be less than 0. fread returns 0 on error.\n>>\n>> Signed-off-by: Roel Kluin \n>> ---\n>> Is this correct? please review.\n>>\n>> diff --git a/tools/perf/util/strbuf.c b/tools/perf/util/strbuf.c\n>> index eaba093..376a337 100644\n>> --- a/tools/perf/util/strbuf.c\n>> +++ b/tools/perf/util/strbuf.c\n>> @@ -259,7 +259,7 @@ size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f)\n>>       res = fread(sb->buf + sb->len, 1, size, f);\n>>       if (res > 0)\n>>               strbuf_setlen(sb, sb->len + res);\n>> -     else if (res < 0 && oldalloc == 0)\n>> +     else if (res == 0 && oldalloc == 0)\n>>               strbuf_release(sb);\n>>       return res;\n>\n> This comes straight from Git's strbuf.c so i've Cc:-ed the Git list.\n>\n> Roel, did you get some compiler warning that made you look at this\n> code?\n>\n>        Ingo\n>\n\nNo, I use sed to catch these bugs.\n\nRoel\n"},{"id":"116776","messageId":"4A3FB479.2090902@lsrfire.ath.cx","threadId":"19900","inReplyTo":"25e057c00906220847t15425f38maf486c291d1d2468@mail.gmail.com","subject":"[PATCH] fread does not return negative on error","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2009-06-22T16:42:33Z","receivedAt":"2009-06-22T16:42:33Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Hi,\n\nthe following patch is for git. I just removed the unneeded check for\nres == 0 from your version. Does it look OK?\n\nThanks,\nRené\n\n--- snip! ---\nFrom: Roel Kluin \n\nsize_t res cannot be less than 0. fread returns 0 on error.\n\nReported-by: Ingo Molnar \nSigned-off-by: Roel Kluin \n---\n strbuf.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/strbuf.c b/strbuf.c\nindex a884960..f03d117 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -260,7 +260,7 @@ size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f)\n \tres = fread(sb->buf + sb->len, 1, size, f);\n \tif (res > 0)\n \t\tstrbuf_setlen(sb, sb->len + res);\n-\telse if (res < 0 && oldalloc == 0)\n+\telse if (oldalloc == 0)\n \t\tstrbuf_release(sb);\n \treturn res;\n }\n"},{"id":"116840","messageId":"7vhby64i8f.fsf@alter.siamese.dyndns.org","threadId":"19900","inReplyTo":"4A3FB479.2090902@lsrfire.ath.cx","subject":"Re: [PATCH] fread does not return negative on error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-23T23:56:32Z","receivedAt":"2009-06-23T23:56:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> the following patch is for git. I just removed the unneeded check for\n> res == 0 from your version. Does it look OK?\n\nThe patch looks good, and both of our in-tree users do error out when the\nreturned value is 0 (imap-send.c checks with \"<= 0\" which looks a tad\namateurish, though) correctly.\n\nFunny, there is no caller of this function in the original context this\nbug originally found, which I think is linux-2.6/tools/perf ;-).\n\nThanks.\n\n> From: Roel Kluin \n>\n> size_t res cannot be less than 0. fread returns 0 on error.\n>\n> Reported-by: Ingo Molnar \n> Signed-off-by: Roel Kluin \n> ---\n> strbuf.c | 2 +-\n> 1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> diff --git a/strbuf.c b/strbuf.c\n> index a884960..f03d117 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -260,7 +260,7 @@ size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f)\n> \tres = fread(sb->buf + sb->len, 1, size, f);\n> \tif (res > 0)\n> \t\tstrbuf_setlen(sb, sb->len + res);\n> -\telse if (res < 0 && oldalloc == 0)\n> +\telse if (oldalloc == 0)\n> \t\tstrbuf_release(sb);\n> \treturn res;\n> }\n"},{"id":"116855","messageId":"20090624081819.GA10436@elte.hu","threadId":"19900","inReplyTo":"7vhby64i8f.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fread does not return negative on error","fromName":"Ingo Molnar","fromEmail":"mingo@elte.hu","sentAt":"2009-06-24T08:18:19Z","receivedAt":"2009-06-24T08:18:19Z","isPatch":true,"sender":{"key":"mingo@elte.hu","avatar":null},"body":"\n* Junio C Hamano wrote:\n\n> René Scharfe writes:\n> \n> > the following patch is for git. I just removed the unneeded check for\n> > res == 0 from your version. Does it look OK?\n> \n> The patch looks good, and both of our in-tree users do error out \n> when the returned value is 0 (imap-send.c checks with \"<= 0\" which \n> looks a tad amateurish, though) correctly.\n> \n> Funny, there is no caller of this function in the original context \n> this bug originally found, which I think is linux-2.6/tools/perf \n> ;-).\n\nHehe, yes :-)\n\nBackground: when creating tools/perf/ i cherry-picked all the nice \nGit libraries into tools/perf/util/, to give a standard environment \nfor all tooling things that might come up in the future.\n\nSome of those are not used yet but it looked more logical to pick up \nwhole pieces - some already gained uses. For example config.c is not \ntruly used yet, but very much expected to have a role in the future.\n\n( The only invasive thing i had to do was the s/git_/perf_/ mass \n rename across all the files - having 'git_' in perf looked\n quite confusing. )\n\nAnd our general experience with the Git libraries in \ntools/perf/util/* is: we love them!\n\nFor example parse-options.c is a striking improvement compared to \ngetopt.h we used before, and all the other facilities are sane and \nstraight to the point as well. So in this sense 'perf' is an ... \ninteresting cross-discipline 'fork' of Git's generic libraries.\n\nThe auto-generation of everything out of Documentation/*.txt is \nanother thing we picked up, and that's very nice too.\n\nOne bookeeping issue: i found few explicit credits in those files - \nso i noted in the changelog that i took them from Git and i noted \nthe specific upstream Git sha1 when i copied them. Would be nice to \nupdate each file with names to make credit more explicit:\n\n-rw-rw-r-- 1 mingo mingo 2808 2009-06-23 10:49 abspath.c\n-rw-rw-r-- 1 mingo mingo 1447 2009-06-23 10:49 alias.c\n-rw-rw-r-- 1 mingo mingo 4660 2009-06-23 10:49 cache.h\n-rw-rw-r-- 1 mingo mingo 4817 2009-06-23 10:49 color.c\n-rw-rw-r-- 1 mingo mingo 1187 2009-06-23 10:49 color.h\n-rw-rw-r-- 1 mingo mingo 19149 2009-06-23 10:49 config.c\n-rw-rw-r-- 1 mingo mingo 1041 2009-06-23 10:52 ctype.c\n-rw-rw-r-- 1 mingo mingo 256 2009-06-23 10:49 environment.c\n-rw-rw-r-- 1 mingo mingo 3262 2009-06-23 10:49 exec_cmd.c\n-rw-rw-r-- 1 mingo mingo 496 2009-06-23 10:49 exec_cmd.h\n-rw-rw-r-- 1 mingo mingo 8515 2009-06-23 10:49 help.c\n-rw-rw-r-- 1 mingo mingo 751 2009-06-23 10:49 help.h\n-rw-rw-r-- 1 mingo mingo 2592 2009-06-23 10:49 levenshtein.c\n-rw-rw-r-- 1 mingo mingo 201 2009-06-23 10:49 levenshtein.h\n-rw-rw-r-- 1 mingo mingo 1909 2009-06-23 10:49 pager.c\n-rw-rw-r-- 1 mingo mingo 12454 2009-06-23 10:49 parse-options.c\n-rw-rw-r-- 1 mingo mingo 5693 2009-06-23 10:49 parse-options.h\n-rw-rw-r-- 1 mingo mingo 7986 2009-06-23 10:49 path.c\n-rw-rw-r-- 1 mingo mingo 10442 2009-06-23 10:49 quote.c\n-rw-rw-r-- 1 mingo mingo 2667 2009-06-23 10:49 quote.h\n-rw-rw-r-- 1 mingo mingo 7966 2009-06-23 10:49 run-command.c\n-rw-rw-r-- 1 mingo mingo 2838 2009-06-23 10:49 run-command.h\n-rw-rw-r-- 1 mingo mingo 969 2009-06-23 10:49 sigchain.c\n-rw-rw-r-- 1 mingo mingo 215 2009-06-23 10:49 sigchain.h\n-rw-rw-r-- 1 mingo mingo 7270 2009-06-23 10:49 strbuf.c\n-rw-rw-r-- 1 mingo mingo 4995 2009-06-23 10:49 strbuf.h\n-rw-rw-r-- 1 mingo mingo 556 2009-06-23 10:52 string.c\n-rw-rw-r-- 1 mingo mingo 120 2009-06-23 10:52 string.h\n-rw-rw-r-- 1 mingo mingo 13859 2009-06-24 10:01 symbol.c\n-rw-rw-r-- 1 mingo mingo 1112 2009-06-23 10:52 symbol.h\n-rw-rw-r-- 1 mingo mingo 1690 2009-06-23 10:49 usage.c\n-rw-rw-r-- 1 mingo mingo 9878 2009-06-23 10:52 util.h\n-rw-rw-r-- 1 mingo mingo 4249 2009-06-23 10:49 wrapper.c\n\n\tIngo\n"},{"id":"116859","messageId":"alpine.DEB.1.00.0906241201040.4773@pacific.mpi-cbg.de","threadId":"19900","inReplyTo":"20090624081819.GA10436@elte.hu","subject":"Re: [PATCH] fread does not return negative on error","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-06-24T10:03:29Z","receivedAt":"2009-06-24T10:03:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 24 Jun 2009, Ingo Molnar wrote:\n\n> One bookeeping issue: i found few explicit credits in those files - \n> so i noted in the changelog that i took them from Git and i noted \n> the specific upstream Git sha1 when i copied them. Would be nice to \n> update each file with names to make credit more explicit:\n> \n> -rw-rw-r-- 1 mingo mingo 2808 2009-06-23 10:49 abspath.c\n> -rw-rw-r-- 1 mingo mingo 1447 2009-06-23 10:49 alias.c\n> -rw-rw-r-- 1 mingo mingo 4660 2009-06-23 10:49 cache.h\n> -rw-rw-r-- 1 mingo mingo 4817 2009-06-23 10:49 color.c\n> -rw-rw-r-- 1 mingo mingo 1187 2009-06-23 10:49 color.h\n> -rw-rw-r-- 1 mingo mingo 19149 2009-06-23 10:49 config.c\n> -rw-rw-r-- 1 mingo mingo 1041 2009-06-23 10:52 ctype.c\n> -rw-rw-r-- 1 mingo mingo 256 2009-06-23 10:49 environment.c\n> -rw-rw-r-- 1 mingo mingo 3262 2009-06-23 10:49 exec_cmd.c\n> -rw-rw-r-- 1 mingo mingo 496 2009-06-23 10:49 exec_cmd.h\n> -rw-rw-r-- 1 mingo mingo 8515 2009-06-23 10:49 help.c\n> -rw-rw-r-- 1 mingo mingo 751 2009-06-23 10:49 help.h\n> -rw-rw-r-- 1 mingo mingo 2592 2009-06-23 10:49 levenshtein.c\n> -rw-rw-r-- 1 mingo mingo 201 2009-06-23 10:49 levenshtein.h\n> -rw-rw-r-- 1 mingo mingo 1909 2009-06-23 10:49 pager.c\n> -rw-rw-r-- 1 mingo mingo 12454 2009-06-23 10:49 parse-options.c\n> -rw-rw-r-- 1 mingo mingo 5693 2009-06-23 10:49 parse-options.h\n> -rw-rw-r-- 1 mingo mingo 7986 2009-06-23 10:49 path.c\n> -rw-rw-r-- 1 mingo mingo 10442 2009-06-23 10:49 quote.c\n> -rw-rw-r-- 1 mingo mingo 2667 2009-06-23 10:49 quote.h\n> -rw-rw-r-- 1 mingo mingo 7966 2009-06-23 10:49 run-command.c\n> -rw-rw-r-- 1 mingo mingo 2838 2009-06-23 10:49 run-command.h\n> -rw-rw-r-- 1 mingo mingo 969 2009-06-23 10:49 sigchain.c\n> -rw-rw-r-- 1 mingo mingo 215 2009-06-23 10:49 sigchain.h\n> -rw-rw-r-- 1 mingo mingo 7270 2009-06-23 10:49 strbuf.c\n> -rw-rw-r-- 1 mingo mingo 4995 2009-06-23 10:49 strbuf.h\n> -rw-rw-r-- 1 mingo mingo 556 2009-06-23 10:52 string.c\n> -rw-rw-r-- 1 mingo mingo 120 2009-06-23 10:52 string.h\n> -rw-rw-r-- 1 mingo mingo 13859 2009-06-24 10:01 symbol.c\n> -rw-rw-r-- 1 mingo mingo 1112 2009-06-23 10:52 symbol.h\n> -rw-rw-r-- 1 mingo mingo 1690 2009-06-23 10:49 usage.c\n> -rw-rw-r-- 1 mingo mingo 9878 2009-06-23 10:52 util.h\n> -rw-rw-r-- 1 mingo mingo 4249 2009-06-23 10:49 wrapper.c\n\nThis here script:\n\n-- snip --\nfor file in abspath.c alias.c cache.h color.c color.h config.c ctype.c \\\n\tenvironment.c exec_cmd.c exec_cmd.h help.c help.h levenshtein.c \\\n\tlevenshtein.h pager.c parse-options.c parse-options.h path.c \\\n\tquote.c quote.h run-command.c run-command.h sigchain.c sigchain.h \\\n\tstrbuf.c strbuf.h string.c string.h symbol.c symbol.h usage.c \\\n\tutil.h wrapper.c\ndo\n\techo $file\n\tgit shortlog -n -s $file | head -n 2\ndone\n-- snap --\n\noutputs this (note that a few files you mentioned are not in git.git):\n\nabspath.c\n 2\tJunio C Hamano\n 1\tDmitry Potapov\nalias.c\n 2\tJeff King\n 1\tFelipe Contreras\ncache.h\n 295\tJunio C Hamano\n 98\tLinus Torvalds\ncolor.c\n 4\tJunio C Hamano\n 3\tJeff King\ncolor.h\n 2\tJeff King\n 2\tJohannes Schindelin\nconfig.c\n 65\tJunio C Hamano\n 20\tJohannes Schindelin\nctype.c\n 4\tRené Scharfe\n 2\tJunio C Hamano\nenvironment.c\n 29\tJunio C Hamano\n 11\tJohannes Schindelin\nexec_cmd.c\n 8\tJohannes Sixt\n 7\tJunio C Hamano\nexec_cmd.h\n 2\tJunio C Hamano\n 2\tScott R Parish\nhelp.c\n 18\tJunio C Hamano\n 14\tChristian Couder\nhelp.h\n 2\tMiklos Vajna\n 1\tAlex Riesen\nlevenshtein.c\n 2\tJohannes Schindelin\n 1\tMike Ralphson\nlevenshtein.h\n 1\tJohannes Schindelin\npager.c\n 9\tJunio C Hamano\n 4\tJohannes Schindelin\nparse-options.c\n 18\tPierre Habouzit\n 8\tJunio C Hamano\nparse-options.h\n 15\tPierre Habouzit\n 6\tStephen Boyd\npath.c\n 20\tJunio C Hamano\n 5\tJohannes Sixt\nquote.c\n 12\tJunio C Hamano\n 5\tChristian Couder\nquote.h\n 6\tJunio C Hamano\n 5\tChristian Couder\nrun-command.c\n 11\tShawn O. Pearce\n 7\tJohannes Sixt\nrun-command.h\n 10\tShawn O. Pearce\n 5\tJohannes Sixt\nsigchain.c\n 2\tJeff King\nsigchain.h\n 2\tJeff King\nstrbuf.c\n 9\tPierre Habouzit\n 8\tJunio C Hamano\nstrbuf.h\n 9\tPierre Habouzit\n 4\tJunio C Hamano\nstring.c\nstring.h\nsymbol.c\nsymbol.h\nusage.c\n 3\tLinus Torvalds\n 2\tJunio C Hamano\nutil.h\nwrapper.c\n 4\tJunio C Hamano\n 2\tLinus Torvalds\n\nCiao,\nDscho\n"},{"id":"116860","messageId":"c07716ae0906240353h67932054w3dc3ba6dbb864dff@mail.gmail.com","threadId":"19900","inReplyTo":"20090624081819.GA10436@elte.hu","subject":"Re: [PATCH] fread does not return negative on error","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2009-06-24T10:53:51Z","receivedAt":"2009-06-24T10:53:51Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi,\n\nOn Wed, Jun 24, 2009 at 10:18 AM, Ingo Molnar wrote:\n>\n> * Junio C Hamano wrote:\n>\n>> René Scharfe writes:\n>>\n>> > the following patch is for git.  I just removed the unneeded check for\n>> > res == 0 from your version.  Does it look OK?\n>>\n>> The patch looks good, and both of our in-tree users do error out\n>> when the returned value is 0 (imap-send.c checks with \"<= 0\" which\n>> looks a tad amateurish, though) correctly.\n>>\n>> Funny, there is no caller of this function in the original context\n>> this bug originally found, which I think is linux-2.6/tools/perf\n>> ;-).\n>\n> Hehe, yes :-)\n>\n> Background: when creating tools/perf/ i cherry-picked all the nice\n> Git libraries into tools/perf/util/, to give a standard environment\n> for all tooling things that might come up in the future.\n>\n> Some of those are not used yet but it looked more logical to pick up\n> whole pieces - some already gained uses. For example config.c is not\n> truly used yet, but very much expected to have a role in the future.\n>\n> ( The only invasive thing i had to do was the s/git_/perf_/ mass\n>  rename across all the files - having 'git_' in perf looked\n>  quite confusing. )\n>\n> And our general experience with the Git libraries in\n> tools/perf/util/* is: we love them!\n>\n> For example parse-options.c is a striking improvement compared to\n> getopt.h we used before, and all the other facilities are sane and\n> straight to the point as well. So in this sense 'perf' is an ...\n> interesting cross-discipline 'fork' of Git's generic libraries.\n>\n> The auto-generation of everything out of Documentation/*.txt is\n> another thing we picked up, and that's very nice too.\n>\n> One bookeeping issue: i found few explicit credits in those files -\n> so i noted in the changelog that i took them from Git and i noted\n> the specific upstream Git sha1 when i copied them. Would be nice to\n> update each file with names to make credit more explicit:\n\nFrom http://git.kernel.org/?p=linux/kernel/git/x86/linux-2.6-tip.git;a=tree;f=tools/perf;hb=HEAD\nit looks like there may be some other files like builtin-help.c (and\nperhaps some files in perf/Documentation/ too though there should be\nsome AUTHOR information already in them).\n\nThanks,\nChristian.\n"},{"id":"116861","messageId":"20090624121226.GA20564@elte.hu","threadId":"19900","inReplyTo":"c07716ae0906240353h67932054w3dc3ba6dbb864dff@mail.gmail.com","subject":"Re: [PATCH] fread does not return negative on error","fromName":"Ingo Molnar","fromEmail":"mingo@elte.hu","sentAt":"2009-06-24T12:12:26Z","receivedAt":"2009-06-24T12:12:26Z","isPatch":true,"sender":{"key":"mingo@elte.hu","avatar":null},"body":"\n* Christian Couder wrote:\n\n> Hi,\n> \n> On Wed, Jun 24, 2009 at 10:18 AM, Ingo Molnar wrote:\n> >\n> > * Junio C Hamano wrote:\n> >\n> >> René Scharfe writes:\n> >>\n> >> > the following patch is for git.  I just removed the unneeded check for\n> >> > res == 0 from your version.  Does it look OK?\n> >>\n> >> The patch looks good, and both of our in-tree users do error out\n> >> when the returned value is 0 (imap-send.c checks with \"<= 0\" which\n> >> looks a tad amateurish, though) correctly.\n> >>\n> >> Funny, there is no caller of this function in the original context\n> >> this bug originally found, which I think is linux-2.6/tools/perf\n> >> ;-).\n> >\n> > Hehe, yes :-)\n> >\n> > Background: when creating tools/perf/ i cherry-picked all the nice\n> > Git libraries into tools/perf/util/, to give a standard environment\n> > for all tooling things that might come up in the future.\n> >\n> > Some of those are not used yet but it looked more logical to pick up\n> > whole pieces - some already gained uses. For example config.c is not\n> > truly used yet, but very much expected to have a role in the future.\n> >\n> > ( The only invasive thing i had to do was the s/git_/perf_/ mass\n> >  rename across all the files - having 'git_' in perf looked\n> >  quite confusing. )\n> >\n> > And our general experience with the Git libraries in\n> > tools/perf/util/* is: we love them!\n> >\n> > For example parse-options.c is a striking improvement compared to\n> > getopt.h we used before, and all the other facilities are sane and\n> > straight to the point as well. So in this sense 'perf' is an ...\n> > interesting cross-discipline 'fork' of Git's generic libraries.\n> >\n> > The auto-generation of everything out of Documentation/*.txt is\n> > another thing we picked up, and that's very nice too.\n> >\n> > One bookeeping issue: i found few explicit credits in those files -\n> > so i noted in the changelog that i took them from Git and i noted\n> > the specific upstream Git sha1 when i copied them. Would be nice to\n> > update each file with names to make credit more explicit:\n> \n> >From http://git.kernel.org/?p=linux/kernel/git/x86/linux-2.6-tip.git;a=tree;f=tools/perf;hb=HEAD\n>\n> it looks like there may be some other files like builtin-help.c \n> (and perhaps some files in perf/Documentation/ too though there \n> should be some AUTHOR information already in them).\n\nCorrect - the makefile and the whole glue code (and much else!) \ncomes from Git - see commits:\n\n 0780060: perf_counter tools: add in basic glue from Git\n d24e473: perf_counter: copy in Git's top Makefile\n\nAny suggestions about how best credit everyone there? One central \nlinux/tools/perf/CREDITS file?\n\n\tIngo\n"},{"id":"116866","messageId":"7vws71wquf.fsf@alter.siamese.dyndns.org","threadId":"19900","inReplyTo":"alpine.DEB.1.00.0906241201040.4773@pacific.mpi-cbg.de","subject":"Re: [PATCH] fread does not return negative on error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-24T16:15:20Z","receivedAt":"2009-06-24T16:15:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> This here script:\n>\n> -- snip --\n> for file in abspath.c alias.c cache.h color.c color.h config.c ctype.c \\\n> \tenvironment.c exec_cmd.c exec_cmd.h help.c help.h levenshtein.c \\\n> \tlevenshtein.h pager.c parse-options.c parse-options.h path.c \\\n> \tquote.c quote.h run-command.c run-command.h sigchain.c sigchain.h \\\n> \tstrbuf.c strbuf.h string.c string.h symbol.c symbol.h usage.c \\\n> \tutil.h wrapper.c\n> do\n> \techo $file\n> \tgit shortlog -n -s $file | head -n 2\n> done\n\nI have thought about suggesting this myself, and your output for many of\nthe files matched my intuition, but some were grossly off, so I checked.\n\nThe above procedure counts commits, and a one liner \"s/char \\*/const &/\"\nweighs as heavily as the patch that implemented the whole thing, for a\nfile that was done in one commit almost perfectly except that it needed a\nsmall constness fix. Summarizing output from \"blame\" for each file may\ngive you a more meaningful results:\n\n # timestamp\n ts='[12][0-9][0-9][0-9]-[0-9][0-9]-[0-3][0-9] ..:..:.. [-+]....'\n # linenum\n lno='[1-9][0-9]*'\n git blame \"$file\" |\n sed -e 's/^[^ ]* *(\\([^)]*[^ ]\\) *'\"$ts *$lno\"').*/\\1/' |\n sort |\n uniq -c |\n sort -r -n\n\nFor example, I do not think it is fair to credit me for abspath.c more\nthan Dmitry like this:\n\n> outputs this (note that a few files you mentioned are not in git.git):\n>\n> abspath.c\n> 2\tJunio C Hamano\n> 1\tDmitry Potapov\n\nInitially Dmitry introduced this file with 5b8e6f8 (shrink git-shell by\navoiding redundant dependencies, 2008-06-28) at 68 lines. J6t added 36\nlines for add_path() with 10c4c88 (Allow add_path() to add non-existent\ndirectories to the path, 2008-07-21), I added 12 lines to add a new\nfunction with 90b4a71 (is_directory(): a generic helper function,\n2008-09-09) and then added a two-liner out-of-bounds-then-die check in\n737e31a (make_absolute_path(): check bounds when seeing an overlong\nsymlink, 2008-12-17).\n"},{"id":"116868","messageId":"alpine.DEB.1.00.0906241835240.7946@intel-tinevez-2-302","threadId":"19900","inReplyTo":"7vws71wquf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fread does not return negative on error","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-06-24T16:40:17Z","receivedAt":"2009-06-24T16:40:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 24 Jun 2009, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > This here script:\n> >\n> > -- snip --\n> > for file in abspath.c alias.c cache.h color.c color.h config.c ctype.c \\\n> > \tenvironment.c exec_cmd.c exec_cmd.h help.c help.h levenshtein.c \\\n> > \tlevenshtein.h pager.c parse-options.c parse-options.h path.c \\\n> > \tquote.c quote.h run-command.c run-command.h sigchain.c sigchain.h \\\n> > \tstrbuf.c strbuf.h string.c string.h symbol.c symbol.h usage.c \\\n> > \tutil.h wrapper.c\n> > do\n> > \techo $file\n> > \tgit shortlog -n -s $file | head -n 2\n> > done\n> \n> I have thought about suggesting this myself, and your output for many of\n> the files matched my intuition, but some were grossly off, so I checked.\n> \n> The above procedure counts commits, and a one liner \"s/char \\*/const &/\"\n> weighs as heavily as the patch that implemented the whole thing, for a\n> file that was done in one commit almost perfectly except that it needed a\n> small constness fix. Summarizing output from \"blame\" for each file may\n> give you a more meaningful results:\n> \n> # timestamp\n> ts='[12][0-9][0-9][0-9]-[0-9][0-9]-[0-3][0-9] ..:..:.. [-+]....'\n> # linenum\n> lno='[1-9][0-9]*'\n> git blame \"$file\" |\n> sed -e 's/^[^ ]* *(\\([^)]*[^ ]\\) *'\"$ts *$lno\"').*/\\1/' |\n> sort |\n> uniq -c |\n> sort -r -n\n> \n> For example, I do not think it is fair to credit me for abspath.c more\n> than Dmitry like this:\n> \n> > outputs this (note that a few files you mentioned are not in git.git):\n> >\n> > abspath.c\n> > 2\tJunio C Hamano\n> > 1\tDmitry Potapov\n> \n> Initially Dmitry introduced this file with 5b8e6f8 (shrink git-shell by\n> avoiding redundant dependencies, 2008-06-28) at 68 lines. J6t added 36\n> lines for add_path() with 10c4c88 (Allow add_path() to add non-existent\n> directories to the path, 2008-07-21), I added 12 lines to add a new\n> function with 90b4a71 (is_directory(): a generic helper function,\n> 2008-09-09) and then added a two-liner out-of-bounds-then-die check in\n> 737e31a (make_absolute_path(): check bounds when seeing an overlong\n> symlink, 2008-12-17).\n\nOkay, a script similar to what you propose shows this:\n\nabspath.c\n 67 Dmitry Potapov\t\n 36 Johannes Sixt\t\nalias.c\n 49 Miklos Vajna\t\n 24 Jeff King\t\ncache.h\n 305 Junio C Hamano\t\n 263 Linus Torvalds\t\ncolor.c\n 136 Jeff King\t\n 29 Johannes Schindelin\t\ncolor.h\n 10 Matthias Kestenholz\t\n 10 Jeff King\t\nconfig.c\n 352 Linus Torvalds\t\n 284 Johannes Schindelin\t\nctype.c\n 15 René Scharfe\t\n 11 Linus Torvalds\t\nenvironment.c\n 68 Linus Torvalds\t\n 34 Johannes Schindelin\t\nexec_cmd.c\n 47 Michal Ostrowski\t\n 40 Steffen Prohaska\t\nexec_cmd.h\n 5 Junio C Hamano\t\n 2 Steve Haslam\t\nhelp.c\n 79 Linus Torvalds\t\n 73 Johannes Schindelin\t\nhelp.h\n 25 Miklos Vajna\t\n 3 Alex Riesen\t\nlevenshtein.c\n 82 Johannes Schindelin\t\n 1 Samuel Tardieu\t\nlevenshtein.h\n 8 Johannes Schindelin\t\npager.c\n 34 Jeff King\t\n 25 Johannes Sixt\t\nparse-options.c\n 386 Pierre Habouzit\t\n 81 René Scharfe\t\nparse-options.h\n 151 Pierre Habouzit\t\n 14 René Scharfe\t\npath.c\n 201 Junio C Hamano\t\n 83 Linus Torvalds\t\nquote.c\n 189 Pierre Habouzit\t\n 106 Junio C Hamano\t\nquote.h\n 31 Junio C Hamano\t\n 11 Christian Couder\t\nrun-command.c\n 173 Johannes Sixt\t\n 87 Shawn O. Pearce\t\nrun-command.h\n 46 Johannes Sixt\t\n 20 Shawn O. Pearce\t\nsigchain.c\n 52 Jeff King\t\nsigchain.h\n 11 Jeff King\t\nstrbuf.c\n 178 Johannes Schindelin\t\n 146 Pierre Habouzit\t\nstrbuf.h\n 93 Pierre Habouzit\t\n 16 Junio C Hamano\t\nstring.c\nstring.h\nsymbol.c\nsymbol.h\nusage.c\n 31 Linus Torvalds\t\n 28 Petr Baudis\t\nutil.h\nwrapper.c\n 220 Linus Torvalds\t\n 69 Junio C Hamano\t\n\nObviously I don't like these results as much, as I do not show up as often \nanymore.\n\nBesides, I think it is not fair to put me on top of the list of authors of \nstrbuf.c.\n\nCiao,\nDscho\n"},{"id":"116877","messageId":"20090624175940.GC7576@elte.hu","threadId":"19900","inReplyTo":"alpine.DEB.1.00.0906241835240.7946@intel-tinevez-2-302","subject":"Re: [PATCH] fread does not return negative on error","fromName":"Ingo Molnar","fromEmail":"mingo@elte.hu","sentAt":"2009-06-24T17:59:40Z","receivedAt":"2009-06-24T17:59:40Z","isPatch":true,"sender":{"key":"mingo@elte.hu","avatar":null},"body":"\n* Johannes Schindelin wrote:\n\n> Okay, a script similar to what you propose shows this:\n\nthanks Johannes! I've queued up the commit below, with the names \nadded to a new CREDITS file (sorted alphabetically).\n\n\tIngo\n\n-------------->\nFrom 1b173f77dd0d5fd4f0ff18034aaa79e30da068b9 Mon Sep 17 00:00:00 2001\nFrom: Ingo Molnar \nDate: Wed, 24 Jun 2009 19:54:29 +0200\nSubject: [PATCH] perf_counter tools: Add CREDITS file for Git contributors\n\nMuch of perf's libraries comes from the Git project. I noticed\nthat the files (in tools/perf/util/*.[ch] and elsewhere) are\nquite spartan wrt. credits, so lets add a CREDITS file that\nincludes an (incomplete!) list of main contributors.\n\nThanks guys, these libraries are really useful. Special thanks\ngo to Johannes Schindelin and Junio C Hamano for coming up with\nthis list.\n\nList-Composed-By: Johannes Schindelin \nCc: Junio C Hamano \nSigned-off-by: Ingo Molnar \n---\n tools/perf/CREDITS | 30 ++++++++++++++++++++++++++++++\n 1 files changed, 30 insertions(+), 0 deletions(-)\n\ndiff --git a/tools/perf/CREDITS b/tools/perf/CREDITS\nnew file mode 100644\nindex 0000000..c2ddcb3\n--- /dev/null\n+++ b/tools/perf/CREDITS\n@@ -0,0 +1,30 @@\n+Most of the infrastructure that 'perf' uses here has been reused\n+from the Git project, as of version:\n+\n+ 66996ec: Sync with 1.6.2.4\n+\n+Here is an (incomplete!) list of main contributors to those files\n+in util/* and elsewhere:\n+\n+ Alex Riesen\n+ Christian Couder\n+ Dmitry Potapov\n+ Jeff King\n+ Johannes Schindelin\n+ Johannes Sixt\n+ Junio C Hamano\n+ Linus Torvalds\n+ Matthias Kestenholz\n+ Michal Ostrowski\n+ Miklos Vajna\n+ Petr Baudis\n+ Pierre Habouzit\n+ René Scharfe\n+ Samuel Tardieu\n+ Shawn O. Pearce\n+ Steffen Prohaska\n+ Steve Haslam\n+\n+Thanks guys!\n+\n+The full history of the files can be found in the upstream Git commits.\n"},{"id":"116884","messageId":"81b0412b0906241419q3bdef822laf9a879ce790d8f@mail.gmail.com","threadId":"19900","inReplyTo":"20090624175940.GC7576@elte.hu","subject":"Re: [PATCH] fread does not return negative on error","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-06-24T21:19:54Z","receivedAt":"2009-06-24T21:19:54Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"2009/6/24 Ingo Molnar :\n> +Here is an (incomplete!) list of main contributors to those files\n> +in util/* and elsewhere:\n> +\n> + Alex Riesen\n\nWow. But I actually think it may be more fair to sort the list after\nany kind of rating (even something as dumb as commit count).\n\nAs everyone one the list knows, I am nowhere near a \"major contributor\"\nto the project, as the first place may imply to a casual reader.\n"},{"id":"116886","messageId":"7veit9qosw.fsf@alter.siamese.dyndns.org","threadId":"19900","inReplyTo":"81b0412b0906241419q3bdef822laf9a879ce790d8f@mail.gmail.com","subject":"Re: [PATCH] fread does not return negative on error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-24T21:55:59Z","receivedAt":"2009-06-24T21:55:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen writes:\n\n> 2009/6/24 Ingo Molnar :\n>> +Here is an (incomplete!) list of main contributors to those files\n>> +in util/* and elsewhere:\n>> +\n>> + Alex Riesen\n>\n> Wow. But I actually think it may be more fair to sort the list after\n> any kind of rating (even something as dumb as commit count).\n>\n> As everyone one the list knows, I am nowhere near a \"major contributor\"\n> to the project, as the first place may imply to a casual reader.\n\nHeh, it was kind of obvious to a casual reader that the list was sorted\nalphabetically by the first name (unusual, eh?).\n\nAnd no, you are indeed a valuable contributor to the git project.\n\nThanks.\n"},{"id":"116951","messageId":"7vtz24cgia.fsf@alter.siamese.dyndns.org","threadId":"19900","inReplyTo":"20090624081819.GA10436@elte.hu","subject":"Re: [PATCH] fread does not return negative on error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-25T18:31:09Z","receivedAt":"2009-06-25T18:31:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ingo Molnar writes:\n\n> And our general experience with the Git libraries in \n> tools/perf/util/* is: we love them!\n\n> For example parse-options.c is a striking improvement compared to \n> getopt.h we used before, and all the other facilities are sane and \n> straight to the point as well. So in this sense 'perf' is an ... \n> interesting cross-discipline 'fork' of Git's generic libraries.\n\nKudos to Pierre, then.\n"}]}