* Re: [PATCH] tools: fread does not return negative on error
[not found] <4A3FB09D.9050903@gmail.com>
@ 2009-06-22 15:34 ` Ingo Molnar
2009-06-22 15:47 ` roel kluin
0 siblings, 1 reply; 14+ messages in thread
From: Ingo Molnar @ 2009-06-22 15:34 UTC (permalink / raw)
To: Roel Kluin, git; +Cc: LKML, Andrew Morton
* Roel Kluin <roel.kluin@gmail.com> wrote:
> size_t res cannot be less than 0. fread returns 0 on error.
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> Is this correct? please review.
>
> diff --git a/tools/perf/util/strbuf.c b/tools/perf/util/strbuf.c
> index eaba093..376a337 100644
> --- a/tools/perf/util/strbuf.c
> +++ b/tools/perf/util/strbuf.c
> @@ -259,7 +259,7 @@ size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f)
> res = fread(sb->buf + sb->len, 1, size, f);
> if (res > 0)
> strbuf_setlen(sb, sb->len + res);
> - else if (res < 0 && oldalloc == 0)
> + else if (res == 0 && oldalloc == 0)
> strbuf_release(sb);
> return res;
This comes straight from Git's strbuf.c so i've Cc:-ed the Git list.
Roel, did you get some compiler warning that made you look at this
code?
Ingo
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] tools: fread does not return negative on error
2009-06-22 15:34 ` [PATCH] tools: fread does not return negative on error Ingo Molnar
@ 2009-06-22 15:47 ` roel kluin
2009-06-22 16:42 ` [PATCH] " René Scharfe
0 siblings, 1 reply; 14+ messages in thread
From: roel kluin @ 2009-06-22 15:47 UTC (permalink / raw)
To: Ingo Molnar; +Cc: git, LKML, Andrew Morton
On Mon, Jun 22, 2009 at 5:34 PM, Ingo Molnar<mingo@elte.hu> wrote:
>
> * Roel Kluin <roel.kluin@gmail.com> wrote:
>
>> size_t res cannot be less than 0. fread returns 0 on error.
>>
>> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
>> ---
>> Is this correct? please review.
>>
>> diff --git a/tools/perf/util/strbuf.c b/tools/perf/util/strbuf.c
>> index eaba093..376a337 100644
>> --- a/tools/perf/util/strbuf.c
>> +++ b/tools/perf/util/strbuf.c
>> @@ -259,7 +259,7 @@ size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f)
>> res = fread(sb->buf + sb->len, 1, size, f);
>> if (res > 0)
>> strbuf_setlen(sb, sb->len + res);
>> - else if (res < 0 && oldalloc == 0)
>> + else if (res == 0 && oldalloc == 0)
>> strbuf_release(sb);
>> return res;
>
> This comes straight from Git's strbuf.c so i've Cc:-ed the Git list.
>
> Roel, did you get some compiler warning that made you look at this
> code?
>
> Ingo
>
No, I use sed to catch these bugs.
Roel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] fread does not return negative on error
2009-06-22 15:47 ` roel kluin
@ 2009-06-22 16:42 ` René Scharfe
2009-06-23 23:56 ` Junio C Hamano
0 siblings, 1 reply; 14+ messages in thread
From: René Scharfe @ 2009-06-22 16:42 UTC (permalink / raw)
To: roel kluin; +Cc: Ingo Molnar, git, LKML, Andrew Morton, Junio C Hamano
Hi,
the following patch is for git. I just removed the unneeded check for
res == 0 from your version. Does it look OK?
Thanks,
René
--- snip! ---
From: Roel Kluin <roel.kluin@gmail.com>
size_t res cannot be less than 0. fread returns 0 on error.
Reported-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
strbuf.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index a884960..f03d117 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -260,7 +260,7 @@ size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f)
res = fread(sb->buf + sb->len, 1, size, f);
if (res > 0)
strbuf_setlen(sb, sb->len + res);
- else if (res < 0 && oldalloc == 0)
+ else if (oldalloc == 0)
strbuf_release(sb);
return res;
}
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] fread does not return negative on error
2009-06-22 16:42 ` [PATCH] " René Scharfe
@ 2009-06-23 23:56 ` Junio C Hamano
2009-06-24 8:18 ` Ingo Molnar
0 siblings, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2009-06-23 23:56 UTC (permalink / raw)
To: René Scharfe
Cc: roel kluin, Ingo Molnar, git, LKML, Andrew Morton, Junio C Hamano
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> the following patch is for git. I just removed the unneeded check for
> res == 0 from your version. Does it look OK?
The patch looks good, and both of our in-tree users do error out when the
returned value is 0 (imap-send.c checks with "<= 0" which looks a tad
amateurish, though) correctly.
Funny, there is no caller of this function in the original context this
bug originally found, which I think is linux-2.6/tools/perf ;-).
Thanks.
> From: Roel Kluin <roel.kluin@gmail.com>
>
> size_t res cannot be less than 0. fread returns 0 on error.
>
> Reported-by: Ingo Molnar <mingo@elte.hu>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> strbuf.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/strbuf.c b/strbuf.c
> index a884960..f03d117 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -260,7 +260,7 @@ size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f)
> res = fread(sb->buf + sb->len, 1, size, f);
> if (res > 0)
> strbuf_setlen(sb, sb->len + res);
> - else if (res < 0 && oldalloc == 0)
> + else if (oldalloc == 0)
> strbuf_release(sb);
> return res;
> }
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] fread does not return negative on error
2009-06-23 23:56 ` Junio C Hamano
@ 2009-06-24 8:18 ` Ingo Molnar
2009-06-24 10:03 ` Johannes Schindelin
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Ingo Molnar @ 2009-06-24 8:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: René Scharfe, roel kluin, git, LKML, Andrew Morton
* Junio C Hamano <gitster@pobox.com> wrote:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>
> > the following patch is for git. I just removed the unneeded check for
> > res == 0 from your version. Does it look OK?
>
> The patch looks good, and both of our in-tree users do error out
> when the returned value is 0 (imap-send.c checks with "<= 0" which
> looks a tad amateurish, though) correctly.
>
> Funny, there is no caller of this function in the original context
> this bug originally found, which I think is linux-2.6/tools/perf
> ;-).
Hehe, yes :-)
Background: when creating tools/perf/ i cherry-picked all the nice
Git libraries into tools/perf/util/, to give a standard environment
for all tooling things that might come up in the future.
Some of those are not used yet but it looked more logical to pick up
whole pieces - some already gained uses. For example config.c is not
truly used yet, but very much expected to have a role in the future.
( The only invasive thing i had to do was the s/git_/perf_/ mass
rename across all the files - having 'git_' in perf looked
quite confusing. )
And our general experience with the Git libraries in
tools/perf/util/* is: we love them!
For example parse-options.c is a striking improvement compared to
getopt.h we used before, and all the other facilities are sane and
straight to the point as well. So in this sense 'perf' is an ...
interesting cross-discipline 'fork' of Git's generic libraries.
The auto-generation of everything out of Documentation/*.txt is
another thing we picked up, and that's very nice too.
One bookeeping issue: i found few explicit credits in those files -
so i noted in the changelog that i took them from Git and i noted
the specific upstream Git sha1 when i copied them. Would be nice to
update each file with names to make credit more explicit:
-rw-rw-r-- 1 mingo mingo 2808 2009-06-23 10:49 abspath.c
-rw-rw-r-- 1 mingo mingo 1447 2009-06-23 10:49 alias.c
-rw-rw-r-- 1 mingo mingo 4660 2009-06-23 10:49 cache.h
-rw-rw-r-- 1 mingo mingo 4817 2009-06-23 10:49 color.c
-rw-rw-r-- 1 mingo mingo 1187 2009-06-23 10:49 color.h
-rw-rw-r-- 1 mingo mingo 19149 2009-06-23 10:49 config.c
-rw-rw-r-- 1 mingo mingo 1041 2009-06-23 10:52 ctype.c
-rw-rw-r-- 1 mingo mingo 256 2009-06-23 10:49 environment.c
-rw-rw-r-- 1 mingo mingo 3262 2009-06-23 10:49 exec_cmd.c
-rw-rw-r-- 1 mingo mingo 496 2009-06-23 10:49 exec_cmd.h
-rw-rw-r-- 1 mingo mingo 8515 2009-06-23 10:49 help.c
-rw-rw-r-- 1 mingo mingo 751 2009-06-23 10:49 help.h
-rw-rw-r-- 1 mingo mingo 2592 2009-06-23 10:49 levenshtein.c
-rw-rw-r-- 1 mingo mingo 201 2009-06-23 10:49 levenshtein.h
-rw-rw-r-- 1 mingo mingo 1909 2009-06-23 10:49 pager.c
-rw-rw-r-- 1 mingo mingo 12454 2009-06-23 10:49 parse-options.c
-rw-rw-r-- 1 mingo mingo 5693 2009-06-23 10:49 parse-options.h
-rw-rw-r-- 1 mingo mingo 7986 2009-06-23 10:49 path.c
-rw-rw-r-- 1 mingo mingo 10442 2009-06-23 10:49 quote.c
-rw-rw-r-- 1 mingo mingo 2667 2009-06-23 10:49 quote.h
-rw-rw-r-- 1 mingo mingo 7966 2009-06-23 10:49 run-command.c
-rw-rw-r-- 1 mingo mingo 2838 2009-06-23 10:49 run-command.h
-rw-rw-r-- 1 mingo mingo 969 2009-06-23 10:49 sigchain.c
-rw-rw-r-- 1 mingo mingo 215 2009-06-23 10:49 sigchain.h
-rw-rw-r-- 1 mingo mingo 7270 2009-06-23 10:49 strbuf.c
-rw-rw-r-- 1 mingo mingo 4995 2009-06-23 10:49 strbuf.h
-rw-rw-r-- 1 mingo mingo 556 2009-06-23 10:52 string.c
-rw-rw-r-- 1 mingo mingo 120 2009-06-23 10:52 string.h
-rw-rw-r-- 1 mingo mingo 13859 2009-06-24 10:01 symbol.c
-rw-rw-r-- 1 mingo mingo 1112 2009-06-23 10:52 symbol.h
-rw-rw-r-- 1 mingo mingo 1690 2009-06-23 10:49 usage.c
-rw-rw-r-- 1 mingo mingo 9878 2009-06-23 10:52 util.h
-rw-rw-r-- 1 mingo mingo 4249 2009-06-23 10:49 wrapper.c
Ingo
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] fread does not return negative on error
2009-06-24 8:18 ` Ingo Molnar
@ 2009-06-24 10:03 ` Johannes Schindelin
2009-06-24 16:15 ` Junio C Hamano
2009-06-24 10:53 ` Christian Couder
2009-06-25 18:31 ` Junio C Hamano
2 siblings, 1 reply; 14+ messages in thread
From: Johannes Schindelin @ 2009-06-24 10:03 UTC (permalink / raw)
To: Ingo Molnar
Cc: Junio C Hamano, René Scharfe, roel kluin, git, LKML,
Andrew Morton
[-- Attachment #1: Type: TEXT/PLAIN, Size: 4341 bytes --]
Hi,
On Wed, 24 Jun 2009, Ingo Molnar wrote:
> One bookeeping issue: i found few explicit credits in those files -
> so i noted in the changelog that i took them from Git and i noted
> the specific upstream Git sha1 when i copied them. Would be nice to
> update each file with names to make credit more explicit:
>
> -rw-rw-r-- 1 mingo mingo 2808 2009-06-23 10:49 abspath.c
> -rw-rw-r-- 1 mingo mingo 1447 2009-06-23 10:49 alias.c
> -rw-rw-r-- 1 mingo mingo 4660 2009-06-23 10:49 cache.h
> -rw-rw-r-- 1 mingo mingo 4817 2009-06-23 10:49 color.c
> -rw-rw-r-- 1 mingo mingo 1187 2009-06-23 10:49 color.h
> -rw-rw-r-- 1 mingo mingo 19149 2009-06-23 10:49 config.c
> -rw-rw-r-- 1 mingo mingo 1041 2009-06-23 10:52 ctype.c
> -rw-rw-r-- 1 mingo mingo 256 2009-06-23 10:49 environment.c
> -rw-rw-r-- 1 mingo mingo 3262 2009-06-23 10:49 exec_cmd.c
> -rw-rw-r-- 1 mingo mingo 496 2009-06-23 10:49 exec_cmd.h
> -rw-rw-r-- 1 mingo mingo 8515 2009-06-23 10:49 help.c
> -rw-rw-r-- 1 mingo mingo 751 2009-06-23 10:49 help.h
> -rw-rw-r-- 1 mingo mingo 2592 2009-06-23 10:49 levenshtein.c
> -rw-rw-r-- 1 mingo mingo 201 2009-06-23 10:49 levenshtein.h
> -rw-rw-r-- 1 mingo mingo 1909 2009-06-23 10:49 pager.c
> -rw-rw-r-- 1 mingo mingo 12454 2009-06-23 10:49 parse-options.c
> -rw-rw-r-- 1 mingo mingo 5693 2009-06-23 10:49 parse-options.h
> -rw-rw-r-- 1 mingo mingo 7986 2009-06-23 10:49 path.c
> -rw-rw-r-- 1 mingo mingo 10442 2009-06-23 10:49 quote.c
> -rw-rw-r-- 1 mingo mingo 2667 2009-06-23 10:49 quote.h
> -rw-rw-r-- 1 mingo mingo 7966 2009-06-23 10:49 run-command.c
> -rw-rw-r-- 1 mingo mingo 2838 2009-06-23 10:49 run-command.h
> -rw-rw-r-- 1 mingo mingo 969 2009-06-23 10:49 sigchain.c
> -rw-rw-r-- 1 mingo mingo 215 2009-06-23 10:49 sigchain.h
> -rw-rw-r-- 1 mingo mingo 7270 2009-06-23 10:49 strbuf.c
> -rw-rw-r-- 1 mingo mingo 4995 2009-06-23 10:49 strbuf.h
> -rw-rw-r-- 1 mingo mingo 556 2009-06-23 10:52 string.c
> -rw-rw-r-- 1 mingo mingo 120 2009-06-23 10:52 string.h
> -rw-rw-r-- 1 mingo mingo 13859 2009-06-24 10:01 symbol.c
> -rw-rw-r-- 1 mingo mingo 1112 2009-06-23 10:52 symbol.h
> -rw-rw-r-- 1 mingo mingo 1690 2009-06-23 10:49 usage.c
> -rw-rw-r-- 1 mingo mingo 9878 2009-06-23 10:52 util.h
> -rw-rw-r-- 1 mingo mingo 4249 2009-06-23 10:49 wrapper.c
This here script:
-- snip --
for file in abspath.c alias.c cache.h color.c color.h config.c ctype.c \
environment.c exec_cmd.c exec_cmd.h help.c help.h levenshtein.c \
levenshtein.h pager.c parse-options.c parse-options.h path.c \
quote.c quote.h run-command.c run-command.h sigchain.c sigchain.h \
strbuf.c strbuf.h string.c string.h symbol.c symbol.h usage.c \
util.h wrapper.c
do
echo $file
git shortlog -n -s $file | head -n 2
done
-- snap --
outputs this (note that a few files you mentioned are not in git.git):
abspath.c
2 Junio C Hamano
1 Dmitry Potapov
alias.c
2 Jeff King
1 Felipe Contreras
cache.h
295 Junio C Hamano
98 Linus Torvalds
color.c
4 Junio C Hamano
3 Jeff King
color.h
2 Jeff King
2 Johannes Schindelin
config.c
65 Junio C Hamano
20 Johannes Schindelin
ctype.c
4 René Scharfe
2 Junio C Hamano
environment.c
29 Junio C Hamano
11 Johannes Schindelin
exec_cmd.c
8 Johannes Sixt
7 Junio C Hamano
exec_cmd.h
2 Junio C Hamano
2 Scott R Parish
help.c
18 Junio C Hamano
14 Christian Couder
help.h
2 Miklos Vajna
1 Alex Riesen
levenshtein.c
2 Johannes Schindelin
1 Mike Ralphson
levenshtein.h
1 Johannes Schindelin
pager.c
9 Junio C Hamano
4 Johannes Schindelin
parse-options.c
18 Pierre Habouzit
8 Junio C Hamano
parse-options.h
15 Pierre Habouzit
6 Stephen Boyd
path.c
20 Junio C Hamano
5 Johannes Sixt
quote.c
12 Junio C Hamano
5 Christian Couder
quote.h
6 Junio C Hamano
5 Christian Couder
run-command.c
11 Shawn O. Pearce
7 Johannes Sixt
run-command.h
10 Shawn O. Pearce
5 Johannes Sixt
sigchain.c
2 Jeff King
sigchain.h
2 Jeff King
strbuf.c
9 Pierre Habouzit
8 Junio C Hamano
strbuf.h
9 Pierre Habouzit
4 Junio C Hamano
string.c
string.h
symbol.c
symbol.h
usage.c
3 Linus Torvalds
2 Junio C Hamano
util.h
wrapper.c
4 Junio C Hamano
2 Linus Torvalds
Ciao,
Dscho
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] fread does not return negative on error
2009-06-24 8:18 ` Ingo Molnar
2009-06-24 10:03 ` Johannes Schindelin
@ 2009-06-24 10:53 ` Christian Couder
2009-06-24 12:12 ` Ingo Molnar
2009-06-25 18:31 ` Junio C Hamano
2 siblings, 1 reply; 14+ messages in thread
From: Christian Couder @ 2009-06-24 10:53 UTC (permalink / raw)
To: Ingo Molnar
Cc: Junio C Hamano, René Scharfe, roel kluin, git, LKML,
Andrew Morton
Hi,
On Wed, Jun 24, 2009 at 10:18 AM, Ingo Molnar<mingo@elte.hu> wrote:
>
> * Junio C Hamano <gitster@pobox.com> wrote:
>
>> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>>
>> > the following patch is for git. I just removed the unneeded check for
>> > res == 0 from your version. Does it look OK?
>>
>> The patch looks good, and both of our in-tree users do error out
>> when the returned value is 0 (imap-send.c checks with "<= 0" which
>> looks a tad amateurish, though) correctly.
>>
>> Funny, there is no caller of this function in the original context
>> this bug originally found, which I think is linux-2.6/tools/perf
>> ;-).
>
> Hehe, yes :-)
>
> Background: when creating tools/perf/ i cherry-picked all the nice
> Git libraries into tools/perf/util/, to give a standard environment
> for all tooling things that might come up in the future.
>
> Some of those are not used yet but it looked more logical to pick up
> whole pieces - some already gained uses. For example config.c is not
> truly used yet, but very much expected to have a role in the future.
>
> ( The only invasive thing i had to do was the s/git_/perf_/ mass
> rename across all the files - having 'git_' in perf looked
> quite confusing. )
>
> And our general experience with the Git libraries in
> tools/perf/util/* is: we love them!
>
> For example parse-options.c is a striking improvement compared to
> getopt.h we used before, and all the other facilities are sane and
> straight to the point as well. So in this sense 'perf' is an ...
> interesting cross-discipline 'fork' of Git's generic libraries.
>
> The auto-generation of everything out of Documentation/*.txt is
> another thing we picked up, and that's very nice too.
>
> One bookeeping issue: i found few explicit credits in those files -
> so i noted in the changelog that i took them from Git and i noted
> the specific upstream Git sha1 when i copied them. Would be nice to
> update each file with names to make credit more explicit:
From http://git.kernel.org/?p=linux/kernel/git/x86/linux-2.6-tip.git;a=tree;f=tools/perf;hb=HEAD
it looks like there may be some other files like builtin-help.c (and
perhaps some files in perf/Documentation/ too though there should be
some AUTHOR information already in them).
Thanks,
Christian.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] fread does not return negative on error
2009-06-24 10:53 ` Christian Couder
@ 2009-06-24 12:12 ` Ingo Molnar
0 siblings, 0 replies; 14+ messages in thread
From: Ingo Molnar @ 2009-06-24 12:12 UTC (permalink / raw)
To: Christian Couder
Cc: Junio C Hamano, René Scharfe, roel kluin, git, LKML,
Andrew Morton
* Christian Couder <christian.couder@gmail.com> wrote:
> Hi,
>
> On Wed, Jun 24, 2009 at 10:18 AM, Ingo Molnar<mingo@elte.hu> wrote:
> >
> > * Junio C Hamano <gitster@pobox.com> wrote:
> >
> >> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> >>
> >> > the following patch is for git. I just removed the unneeded check for
> >> > res == 0 from your version. Does it look OK?
> >>
> >> The patch looks good, and both of our in-tree users do error out
> >> when the returned value is 0 (imap-send.c checks with "<= 0" which
> >> looks a tad amateurish, though) correctly.
> >>
> >> Funny, there is no caller of this function in the original context
> >> this bug originally found, which I think is linux-2.6/tools/perf
> >> ;-).
> >
> > Hehe, yes :-)
> >
> > Background: when creating tools/perf/ i cherry-picked all the nice
> > Git libraries into tools/perf/util/, to give a standard environment
> > for all tooling things that might come up in the future.
> >
> > Some of those are not used yet but it looked more logical to pick up
> > whole pieces - some already gained uses. For example config.c is not
> > truly used yet, but very much expected to have a role in the future.
> >
> > ( The only invasive thing i had to do was the s/git_/perf_/ mass
> > rename across all the files - having 'git_' in perf looked
> > quite confusing. )
> >
> > And our general experience with the Git libraries in
> > tools/perf/util/* is: we love them!
> >
> > For example parse-options.c is a striking improvement compared to
> > getopt.h we used before, and all the other facilities are sane and
> > straight to the point as well. So in this sense 'perf' is an ...
> > interesting cross-discipline 'fork' of Git's generic libraries.
> >
> > The auto-generation of everything out of Documentation/*.txt is
> > another thing we picked up, and that's very nice too.
> >
> > One bookeeping issue: i found few explicit credits in those files -
> > so i noted in the changelog that i took them from Git and i noted
> > the specific upstream Git sha1 when i copied them. Would be nice to
> > update each file with names to make credit more explicit:
>
> >From http://git.kernel.org/?p=linux/kernel/git/x86/linux-2.6-tip.git;a=tree;f=tools/perf;hb=HEAD
>
> it looks like there may be some other files like builtin-help.c
> (and perhaps some files in perf/Documentation/ too though there
> should be some AUTHOR information already in them).
Correct - the makefile and the whole glue code (and much else!)
comes from Git - see commits:
0780060: perf_counter tools: add in basic glue from Git
d24e473: perf_counter: copy in Git's top Makefile
Any suggestions about how best credit everyone there? One central
linux/tools/perf/CREDITS file?
Ingo
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] fread does not return negative on error
2009-06-24 10:03 ` Johannes Schindelin
@ 2009-06-24 16:15 ` Junio C Hamano
2009-06-24 16:40 ` Johannes Schindelin
0 siblings, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2009-06-24 16:15 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Ingo Molnar, René Scharfe, roel kluin, git, LKML,
Andrew Morton
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> This here script:
>
> -- snip --
> for file in abspath.c alias.c cache.h color.c color.h config.c ctype.c \
> environment.c exec_cmd.c exec_cmd.h help.c help.h levenshtein.c \
> levenshtein.h pager.c parse-options.c parse-options.h path.c \
> quote.c quote.h run-command.c run-command.h sigchain.c sigchain.h \
> strbuf.c strbuf.h string.c string.h symbol.c symbol.h usage.c \
> util.h wrapper.c
> do
> echo $file
> git shortlog -n -s $file | head -n 2
> done
I have thought about suggesting this myself, and your output for many of
the files matched my intuition, but some were grossly off, so I checked.
The above procedure counts commits, and a one liner "s/char \*/const &/"
weighs as heavily as the patch that implemented the whole thing, for a
file that was done in one commit almost perfectly except that it needed a
small constness fix. Summarizing output from "blame" for each file may
give you a more meaningful results:
# timestamp
ts='[12][0-9][0-9][0-9]-[0-9][0-9]-[0-3][0-9] ..:..:.. [-+]....'
# linenum
lno='[1-9][0-9]*'
git blame "$file" |
sed -e 's/^[^ ]* *(\([^)]*[^ ]\) *'"$ts *$lno"').*/\1/' |
sort |
uniq -c |
sort -r -n
For example, I do not think it is fair to credit me for abspath.c more
than Dmitry like this:
> outputs this (note that a few files you mentioned are not in git.git):
>
> abspath.c
> 2 Junio C Hamano
> 1 Dmitry Potapov
Initially Dmitry introduced this file with 5b8e6f8 (shrink git-shell by
avoiding redundant dependencies, 2008-06-28) at 68 lines. J6t added 36
lines for add_path() with 10c4c88 (Allow add_path() to add non-existent
directories to the path, 2008-07-21), I added 12 lines to add a new
function with 90b4a71 (is_directory(): a generic helper function,
2008-09-09) and then added a two-liner out-of-bounds-then-die check in
737e31a (make_absolute_path(): check bounds when seeing an overlong
symlink, 2008-12-17).
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] fread does not return negative on error
2009-06-24 16:15 ` Junio C Hamano
@ 2009-06-24 16:40 ` Johannes Schindelin
2009-06-24 17:59 ` Ingo Molnar
0 siblings, 1 reply; 14+ messages in thread
From: Johannes Schindelin @ 2009-06-24 16:40 UTC (permalink / raw)
To: Junio C Hamano
Cc: Ingo Molnar, René Scharfe, roel kluin, git, LKML,
Andrew Morton
[-- Attachment #1: Type: TEXT/PLAIN, Size: 3999 bytes --]
Hi,
On Wed, 24 Jun 2009, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > This here script:
> >
> > -- snip --
> > for file in abspath.c alias.c cache.h color.c color.h config.c ctype.c \
> > environment.c exec_cmd.c exec_cmd.h help.c help.h levenshtein.c \
> > levenshtein.h pager.c parse-options.c parse-options.h path.c \
> > quote.c quote.h run-command.c run-command.h sigchain.c sigchain.h \
> > strbuf.c strbuf.h string.c string.h symbol.c symbol.h usage.c \
> > util.h wrapper.c
> > do
> > echo $file
> > git shortlog -n -s $file | head -n 2
> > done
>
> I have thought about suggesting this myself, and your output for many of
> the files matched my intuition, but some were grossly off, so I checked.
>
> The above procedure counts commits, and a one liner "s/char \*/const &/"
> weighs as heavily as the patch that implemented the whole thing, for a
> file that was done in one commit almost perfectly except that it needed a
> small constness fix. Summarizing output from "blame" for each file may
> give you a more meaningful results:
>
> # timestamp
> ts='[12][0-9][0-9][0-9]-[0-9][0-9]-[0-3][0-9] ..:..:.. [-+]....'
> # linenum
> lno='[1-9][0-9]*'
> git blame "$file" |
> sed -e 's/^[^ ]* *(\([^)]*[^ ]\) *'"$ts *$lno"').*/\1/' |
> sort |
> uniq -c |
> sort -r -n
>
> For example, I do not think it is fair to credit me for abspath.c more
> than Dmitry like this:
>
> > outputs this (note that a few files you mentioned are not in git.git):
> >
> > abspath.c
> > 2 Junio C Hamano
> > 1 Dmitry Potapov
>
> Initially Dmitry introduced this file with 5b8e6f8 (shrink git-shell by
> avoiding redundant dependencies, 2008-06-28) at 68 lines. J6t added 36
> lines for add_path() with 10c4c88 (Allow add_path() to add non-existent
> directories to the path, 2008-07-21), I added 12 lines to add a new
> function with 90b4a71 (is_directory(): a generic helper function,
> 2008-09-09) and then added a two-liner out-of-bounds-then-die check in
> 737e31a (make_absolute_path(): check bounds when seeing an overlong
> symlink, 2008-12-17).
Okay, a script similar to what you propose shows this:
abspath.c
67 Dmitry Potapov
36 Johannes Sixt
alias.c
49 Miklos Vajna
24 Jeff King
cache.h
305 Junio C Hamano
263 Linus Torvalds
color.c
136 Jeff King
29 Johannes Schindelin
color.h
10 Matthias Kestenholz
10 Jeff King
config.c
352 Linus Torvalds
284 Johannes Schindelin
ctype.c
15 René Scharfe
11 Linus Torvalds
environment.c
68 Linus Torvalds
34 Johannes Schindelin
exec_cmd.c
47 Michal Ostrowski
40 Steffen Prohaska
exec_cmd.h
5 Junio C Hamano
2 Steve Haslam
help.c
79 Linus Torvalds
73 Johannes Schindelin
help.h
25 Miklos Vajna
3 Alex Riesen
levenshtein.c
82 Johannes Schindelin
1 Samuel Tardieu
levenshtein.h
8 Johannes Schindelin
pager.c
34 Jeff King
25 Johannes Sixt
parse-options.c
386 Pierre Habouzit
81 René Scharfe
parse-options.h
151 Pierre Habouzit
14 René Scharfe
path.c
201 Junio C Hamano
83 Linus Torvalds
quote.c
189 Pierre Habouzit
106 Junio C Hamano
quote.h
31 Junio C Hamano
11 Christian Couder
run-command.c
173 Johannes Sixt
87 Shawn O. Pearce
run-command.h
46 Johannes Sixt
20 Shawn O. Pearce
sigchain.c
52 Jeff King
sigchain.h
11 Jeff King
strbuf.c
178 Johannes Schindelin
146 Pierre Habouzit
strbuf.h
93 Pierre Habouzit
16 Junio C Hamano
string.c
string.h
symbol.c
symbol.h
usage.c
31 Linus Torvalds
28 Petr Baudis
util.h
wrapper.c
220 Linus Torvalds
69 Junio C Hamano
Obviously I don't like these results as much, as I do not show up as often
anymore.
Besides, I think it is not fair to put me on top of the list of authors of
strbuf.c.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] fread does not return negative on error
2009-06-24 16:40 ` Johannes Schindelin
@ 2009-06-24 17:59 ` Ingo Molnar
2009-06-24 21:19 ` Alex Riesen
0 siblings, 1 reply; 14+ messages in thread
From: Ingo Molnar @ 2009-06-24 17:59 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Junio C Hamano, René Scharfe, roel kluin, git, LKML,
Andrew Morton
* Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Okay, a script similar to what you propose shows this:
thanks Johannes! I've queued up the commit below, with the names
added to a new CREDITS file (sorted alphabetically).
Ingo
-------------->
From 1b173f77dd0d5fd4f0ff18034aaa79e30da068b9 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Wed, 24 Jun 2009 19:54:29 +0200
Subject: [PATCH] perf_counter tools: Add CREDITS file for Git contributors
Much of perf's libraries comes from the Git project. I noticed
that the files (in tools/perf/util/*.[ch] and elsewhere) are
quite spartan wrt. credits, so lets add a CREDITS file that
includes an (incomplete!) list of main contributors.
Thanks guys, these libraries are really useful. Special thanks
go to Johannes Schindelin and Junio C Hamano for coming up with
this list.
List-Composed-By: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
tools/perf/CREDITS | 30 ++++++++++++++++++++++++++++++
1 files changed, 30 insertions(+), 0 deletions(-)
diff --git a/tools/perf/CREDITS b/tools/perf/CREDITS
new file mode 100644
index 0000000..c2ddcb3
--- /dev/null
+++ b/tools/perf/CREDITS
@@ -0,0 +1,30 @@
+Most of the infrastructure that 'perf' uses here has been reused
+from the Git project, as of version:
+
+ 66996ec: Sync with 1.6.2.4
+
+Here is an (incomplete!) list of main contributors to those files
+in util/* and elsewhere:
+
+ Alex Riesen
+ Christian Couder
+ Dmitry Potapov
+ Jeff King
+ Johannes Schindelin
+ Johannes Sixt
+ Junio C Hamano
+ Linus Torvalds
+ Matthias Kestenholz
+ Michal Ostrowski
+ Miklos Vajna
+ Petr Baudis
+ Pierre Habouzit
+ René Scharfe
+ Samuel Tardieu
+ Shawn O. Pearce
+ Steffen Prohaska
+ Steve Haslam
+
+Thanks guys!
+
+The full history of the files can be found in the upstream Git commits.
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] fread does not return negative on error
2009-06-24 17:59 ` Ingo Molnar
@ 2009-06-24 21:19 ` Alex Riesen
2009-06-24 21:55 ` Junio C Hamano
0 siblings, 1 reply; 14+ messages in thread
From: Alex Riesen @ 2009-06-24 21:19 UTC (permalink / raw)
To: Ingo Molnar
Cc: Johannes Schindelin, Junio C Hamano, René Scharfe,
roel kluin, git, LKML, Andrew Morton
2009/6/24 Ingo Molnar <mingo@elte.hu>:
> +Here is an (incomplete!) list of main contributors to those files
> +in util/* and elsewhere:
> +
> + Alex Riesen
Wow. But I actually think it may be more fair to sort the list after
any kind of rating (even something as dumb as commit count).
As everyone one the list knows, I am nowhere near a "major contributor"
to the project, as the first place may imply to a casual reader.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] fread does not return negative on error
2009-06-24 21:19 ` Alex Riesen
@ 2009-06-24 21:55 ` Junio C Hamano
0 siblings, 0 replies; 14+ messages in thread
From: Junio C Hamano @ 2009-06-24 21:55 UTC (permalink / raw)
To: Alex Riesen
Cc: Ingo Molnar, Johannes Schindelin, Junio C Hamano,
René Scharfe, roel kluin, git, LKML, Andrew Morton
Alex Riesen <raa.lkml@gmail.com> writes:
> 2009/6/24 Ingo Molnar <mingo@elte.hu>:
>> +Here is an (incomplete!) list of main contributors to those files
>> +in util/* and elsewhere:
>> +
>> + Alex Riesen
>
> Wow. But I actually think it may be more fair to sort the list after
> any kind of rating (even something as dumb as commit count).
>
> As everyone one the list knows, I am nowhere near a "major contributor"
> to the project, as the first place may imply to a casual reader.
Heh, it was kind of obvious to a casual reader that the list was sorted
alphabetically by the first name (unusual, eh?).
And no, you are indeed a valuable contributor to the git project.
Thanks.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] fread does not return negative on error
2009-06-24 8:18 ` Ingo Molnar
2009-06-24 10:03 ` Johannes Schindelin
2009-06-24 10:53 ` Christian Couder
@ 2009-06-25 18:31 ` Junio C Hamano
2 siblings, 0 replies; 14+ messages in thread
From: Junio C Hamano @ 2009-06-25 18:31 UTC (permalink / raw)
To: Ingo Molnar
Cc: Junio C Hamano, René Scharfe, roel kluin, git, LKML,
Andrew Morton, Pierre Habouzit
Ingo Molnar <mingo@elte.hu> writes:
> And our general experience with the Git libraries in
> tools/perf/util/* is: we love them!
> For example parse-options.c is a striking improvement compared to
> getopt.h we used before, and all the other facilities are sane and
> straight to the point as well. So in this sense 'perf' is an ...
> interesting cross-discipline 'fork' of Git's generic libraries.
Kudos to Pierre, then.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2009-06-25 18:31 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <4A3FB09D.9050903@gmail.com>
2009-06-22 15:34 ` [PATCH] tools: fread does not return negative on error Ingo Molnar
2009-06-22 15:47 ` roel kluin
2009-06-22 16:42 ` [PATCH] " René Scharfe
2009-06-23 23:56 ` Junio C Hamano
2009-06-24 8:18 ` Ingo Molnar
2009-06-24 10:03 ` Johannes Schindelin
2009-06-24 16:15 ` Junio C Hamano
2009-06-24 16:40 ` Johannes Schindelin
2009-06-24 17:59 ` Ingo Molnar
2009-06-24 21:19 ` Alex Riesen
2009-06-24 21:55 ` Junio C Hamano
2009-06-24 10:53 ` Christian Couder
2009-06-24 12:12 ` Ingo Molnar
2009-06-25 18:31 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).