* [PATCH 4/6] In handle_body only read a line if we don't already have one.
From: Eric W. Biederman @ 2006-05-23 19:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <m1verwikvj.fsf_-_@ebiederm.dsl.xmission.com>
This prepares for detecting non-email patches that don't have
mail headers. In which case we have already read the first
line so handle_body should not ignore it.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
mailinfo.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
3ad0c255a351d771c7f301d4a4e9bfb6fdcbde5f
diff --git a/mailinfo.c b/mailinfo.c
index 3fa9505..99989c2 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -724,7 +724,7 @@ static void handle_body(void)
{
int seen = 0;
- if (fgets(line, sizeof(line), stdin) != NULL) {
+ if (line[0] || fgets(line, sizeof(line), stdin) != NULL) {
handle_commit_msg(&seen);
handle_patch();
}
--
1.3.2.g5041c-dirty
^ permalink raw reply related
* [PATCH 3/6] Refactor commit messge handling.
From: Eric W. Biederman @ 2006-05-23 19:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <m1zmh8ikym.fsf_-_@ebiederm.dsl.xmission.com>
- Move handle_info into main so it is called once
after everything has been parsed. This allows the removal
of a static variable and removes two duplicate calls.
- Move parsing of inbody headers into handle_commit.
This means we parse the in-body headers after we have decoded
the character set, and it removes code duplication between
handle_multipart_one_part and handle_body.
- Change the flag indicating that we have seen an in body
prefix header into another bit in seen.
This is a little more general and allows the possibility of parsing
in body headers after the body message has begun.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
mailinfo.c | 58 ++++++++++++++++++++++------------------------------------
1 files changed, 22 insertions(+), 36 deletions(-)
3f6fe4d5e86c3d8d1fad75bfeb71f398966813d4
diff --git a/mailinfo.c b/mailinfo.c
index bee7b20..3fa9505 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -237,38 +237,41 @@ static int eatspace(char *line)
#define SEEN_FROM 01
#define SEEN_DATE 02
#define SEEN_SUBJECT 04
+#define SEEN_PREFIX 0x08
/* First lines of body can have From:, Date:, and Subject: */
-static int handle_inbody_header(int *seen, char *line)
+static void handle_inbody_header(int *seen, char *line)
{
+ if (*seen & SEEN_PREFIX)
+ return;
if (!memcmp("From:", line, 5) && isspace(line[5])) {
if (!(*seen & SEEN_FROM) && handle_from(line+6)) {
*seen |= SEEN_FROM;
- return 1;
+ return;
}
}
if (!memcmp("Date:", line, 5) && isspace(line[5])) {
if (!(*seen & SEEN_DATE)) {
handle_date(line+6);
*seen |= SEEN_DATE;
- return 1;
+ return;
}
}
if (!memcmp("Subject:", line, 8) && isspace(line[8])) {
if (!(*seen & SEEN_SUBJECT)) {
handle_subject(line+9);
*seen |= SEEN_SUBJECT;
- return 1;
+ return;
}
}
if (!memcmp("[PATCH]", line, 7) && isspace(line[7])) {
if (!(*seen & SEEN_SUBJECT)) {
handle_subject(line);
*seen |= SEEN_SUBJECT;
- return 1;
+ return;
}
}
- return 0;
+ *seen |= SEEN_PREFIX;
}
static char *cleanup_subject(char *subject)
@@ -590,12 +593,7 @@ static void decode_transfer_encoding(cha
static void handle_info(void)
{
char *sub;
- static int done_info = 0;
-
- if (done_info)
- return;
- done_info = 1;
sub = cleanup_subject(subject);
cleanup_space(name);
cleanup_space(date);
@@ -609,7 +607,7 @@ static void handle_info(void)
/* We are inside message body and have read line[] already.
* Spit out the commit log.
*/
-static int handle_commit_msg(void)
+static int handle_commit_msg(int *seen)
{
if (!cmitmsg)
return 0;
@@ -633,6 +631,11 @@ static int handle_commit_msg(void)
decode_transfer_encoding(line);
if (metainfo_charset)
convert_to_utf8(line, charset);
+
+ handle_inbody_header(seen, line);
+ if (!(*seen & SEEN_PREFIX))
+ continue;
+
fputs(line, cmitmsg);
} while (fgets(line, sizeof(line), stdin) != NULL);
fclose(cmitmsg);
@@ -664,26 +667,16 @@ static void handle_patch(void)
* that the first part to contain commit message and a patch, and
* handle other parts as pure patches.
*/
-static int handle_multipart_one_part(void)
+static int handle_multipart_one_part(int *seen)
{
- int seen = 0;
int n = 0;
- int len;
while (fgets(line, sizeof(line), stdin) != NULL) {
again:
- len = eatspace(line);
n++;
- if (!len)
- continue;
if (is_multipart_boundary(line))
break;
- if (0 <= seen && handle_inbody_header(&seen, line))
- continue;
- seen = -1; /* no more inbody headers */
- line[len] = '\n';
- handle_info();
- if (handle_commit_msg())
+ if (handle_commit_msg(seen))
goto again;
handle_patch();
break;
@@ -695,6 +688,7 @@ static int handle_multipart_one_part(voi
static void handle_multipart_body(void)
{
+ int seen = 0;
int part_num = 0;
/* Skip up to the first boundary */
@@ -709,7 +703,7 @@ static void handle_multipart_body(void)
while (1) {
int hdr = read_one_header_line(line, sizeof(line), stdin);
if (!hdr) {
- if (handle_multipart_one_part() < 0)
+ if (handle_multipart_one_part(&seen) < 0)
return;
/* Reset per part headers */
transfer_encoding = TE_DONTCARE;
@@ -730,18 +724,9 @@ static void handle_body(void)
{
int seen = 0;
- while (fgets(line, sizeof(line), stdin) != NULL) {
- int len = eatspace(line);
- if (!len)
- continue;
- if (0 <= seen && handle_inbody_header(&seen, line))
- continue;
- seen = -1; /* no more inbody headers */
- line[len] = '\n';
- handle_info();
- handle_commit_msg();
+ if (fgets(line, sizeof(line), stdin) != NULL) {
+ handle_commit_msg(&seen);
handle_patch();
- break;
}
fclose(patchfile);
if (!patch_lines) {
@@ -791,6 +776,7 @@ int main(int argc, char **argv)
handle_multipart_body();
else
handle_body();
+ handle_info();
break;
}
check_header_line(line);
--
1.3.2.g5041c-dirty
^ permalink raw reply related
* [PATCH 2/6] Move B and Q decoding into check header.
From: Eric W. Biederman @ 2006-05-23 19:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <m14pzgjzlg.fsf@ebiederm.dsl.xmission.com>
B and Q decoding is not appropriate for in body headers, so move
it up to where we explicitly know we have a real email header.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
mailinfo.c | 12 +++++-------
1 files changed, 5 insertions(+), 7 deletions(-)
3cccc5a0728a981cc6f4ea72e81513fd902e29a2
diff --git a/mailinfo.c b/mailinfo.c
index 83a2986..bee7b20 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -324,6 +324,7 @@ static void cleanup_space(char *buf)
}
}
+static void decode_header_bq(char *it);
typedef int (*header_fn_t)(char *);
struct header_def {
const char *name;
@@ -343,6 +344,10 @@ static void check_header(char *line, str
int len = header[i].namelen;
if (!strncasecmp(line, header[i].name, len) &&
line[len] == ':' && isspace(line[len + 1])) {
+ /* Unwrap inline B and Q encoding, and optionally
+ * normalize the meta information to utf8.
+ */
+ decode_header_bq(line + len + 2);
header[i].func(line + len + 2);
break;
}
@@ -597,13 +602,6 @@ static void handle_info(void)
cleanup_space(email);
cleanup_space(sub);
- /* Unwrap inline B and Q encoding, and optionally
- * normalize the meta information to utf8.
- */
- decode_header_bq(name);
- decode_header_bq(date);
- decode_header_bq(email);
- decode_header_bq(sub);
printf("Author: %s\nEmail: %s\nSubject: %s\nDate: %s\n\n",
name, email, sub, date);
}
--
1.3.2.g5041c-dirty
^ permalink raw reply related
* [PATCH 1/6] Make read_one_header_line return a flag not a length.
From: Eric W. Biederman @ 2006-05-23 19:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <m18xosjznu.fsf@ebiederm.dsl.xmission.com>
Currently we only use the return value from read_one_header line
to tell if the line we have read is a header or not. So make
it a flag. This paves the way for better email detection.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
mailinfo.c | 22 +++++++++++-----------
1 files changed, 11 insertions(+), 11 deletions(-)
40f4ca44ec851e435ce9453c682c71b9c67063b9
diff --git a/mailinfo.c b/mailinfo.c
index b276519..83a2986 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -331,7 +331,7 @@ struct header_def {
int namelen;
};
-static void check_header(char *line, int len, struct header_def *header)
+static void check_header(char *line, struct header_def *header)
{
int i;
@@ -349,7 +349,7 @@ static void check_header(char *line, int
}
}
-static void check_subheader_line(char *line, int len)
+static void check_subheader_line(char *line)
{
static struct header_def header[] = {
{ "Content-Type", handle_subcontent_type },
@@ -357,9 +357,9 @@ static void check_subheader_line(char *l
handle_content_transfer_encoding },
{ NULL },
};
- check_header(line, len, header);
+ check_header(line, header);
}
-static void check_header_line(char *line, int len)
+static void check_header_line(char *line)
{
static struct header_def header[] = {
{ "From", handle_from },
@@ -370,7 +370,7 @@ static void check_header_line(char *line
handle_content_transfer_encoding },
{ NULL },
};
- check_header(line, len, header);
+ check_header(line, header);
}
static int read_one_header_line(char *line, int sz, FILE *in)
@@ -709,8 +709,8 @@ static void handle_multipart_body(void)
return;
/* We are on boundary line. Start slurping the subhead. */
while (1) {
- int len = read_one_header_line(line, sizeof(line), stdin);
- if (!len) {
+ int hdr = read_one_header_line(line, sizeof(line), stdin);
+ if (!hdr) {
if (handle_multipart_one_part() < 0)
return;
/* Reset per part headers */
@@ -718,7 +718,7 @@ static void handle_multipart_body(void)
charset[0] = 0;
}
else
- check_subheader_line(line, len);
+ check_subheader_line(line);
}
fclose(patchfile);
if (!patch_lines) {
@@ -787,15 +787,15 @@ int main(int argc, char **argv)
exit(1);
}
while (1) {
- int len = read_one_header_line(line, sizeof(line), stdin);
- if (!len) {
+ int hdr = read_one_header_line(line, sizeof(line), stdin);
+ if (!hdr) {
if (multipart_boundary[0])
handle_multipart_body();
else
handle_body();
break;
}
- check_header_line(line, len);
+ check_header_line(line);
}
return 0;
}
--
1.3.2.g5041c-dirty
^ permalink raw reply related
* [PATCH 0/6] Detect non email patches in git-mailinfo
From: Eric W. Biederman @ 2006-05-23 19:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
After looking at a number of additional quit patches I noticed
a small problem with using the current git-mailinfo. On patches
with out any leading headers git-mailinfo can get confused and
loose a bit of information.
So far I have only seen this in the quilt from Andi Kleen but
it is fairly straight forward to fix.
What follows is a small patch series that one small step at
a time refactors (and I think simplifies) git-mailinfo
so that it can detect and cope with a file without any
email headers.
Eric
^ permalink raw reply
* Re: [PATCH 2/2] cvsimport: cleanup commit function
From: Linus Torvalds @ 2006-05-23 19:36 UTC (permalink / raw)
To: Martin Langhoff; +Cc: Junio C Hamano, Matthias Urlichs, git
In-Reply-To: <Pine.LNX.4.64.0605230948280.5623@g5.osdl.org>
On Tue, 23 May 2006, Linus Torvalds wrote:
>
> Hmm. Is it just me, or does the current "git cvsimport" have new problems:
>
> [torvalds@merom git]$ git cvsimport -d ~/CVS gentoo-x86
>
> causes
>
> Committing initial tree 34bd3dcd4bfd79bad35ce3fb08b2e21108195db8
> Server has gone away while fetching BUGS-TODO 1.1, retrying...
> Retry failed at /home/torvalds/bin/git-cvsimport line 366, <GEN2656> line 9.
>
> and that's it for the import.
>
> I don't see what would have caused it in the changes, but it definitely
> worked earlier..
Martin, that problem seems to go away when I initialize $res to 0 in
_fetchfile.
I don't know perl, and maybe local variables are pre-initialized to empty.
It's entirely possible that the fact that it now seems to work for me is
purely timing-related, since I also ended up using "-P cvsps-output" to
avoid having a huge cvsps binary in memory at the same time.
Linus "perl illiterate" Torvalds
^ permalink raw reply
* Re: [PATCH] Avoid segfault in diff --stat rename output.
From: Torgil Svensson @ 2006-05-23 19:06 UTC (permalink / raw)
To: git
In-Reply-To: <BAYC1-PASMTP115C9137E5BDABD705881BAE9B0@CEZ.ICE>
This patch fixed the issue for me.
On 5/23/06, Sean <seanlkml@sympatico.ca> wrote:
>
> Signed-off-by: Sean Estabrooks <seanlkml@sympatico.ca>
> ---
> diff.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> On Tue, 23 May 2006 01:09:43 +0200
> "Torgil Svensson" <torgil.svensson@gmail.com> wrote:
>
> > Hi
> >
> > It seems like git-diff-tree has some problems with moved files:
> >
> > $ git-diff-tree -p --stat --summary -M
> > 348f179e3195448cea49c98a79cce8c7f446ce26
> > 343ca16424ba031b37e4df49afddaee098a8f347 | wc -l
> > *** glibc detected *** free(): invalid pointer: 0x12ecbbf0 ***
> > 6101
>
>
> diff --git a/diff.c b/diff.c
> index 7f35e59..a7bb9b9 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -237,7 +237,7 @@ static char *pprint_rename(const char *a
> if (a_midlen < 0) a_midlen = 0;
> if (b_midlen < 0) b_midlen = 0;
>
> - name = xmalloc(len_a + len_b - pfx_length - sfx_length + 7);
> + name = xmalloc(pfx_length + a_midlen + b_midlen + sfx_length + 7);
> sprintf(name, "%.*s{%.*s => %.*s}%s",
> pfx_length, a,
> a_midlen, a + pfx_length,
> --
> 1.3.GIT
>
>
^ permalink raw reply
* Re: Git 1.3.2 on Solaris
From: Linus Torvalds @ 2006-05-23 18:48 UTC (permalink / raw)
To: Jason Riedy; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0605231110230.5623@g5.osdl.org>
On Tue, 23 May 2006, Linus Torvalds wrote:
>
> I absolutely agree. That is why the OS has a "access()" system call. It's
> there to ask the OS whether the file is executable (or readable/writable).
Side note: I'm not claiming that "access()" is a wonderful thing. I do
agree that we might want to replace it with something else inside of git,
if only because of portability concerns.
So I'm really just ranting my normal "standards lawyerese doesn't mean
much" rant..
(access() also has other isses: X_OK obviously means different things for
directories and for regular files, so quite often you need to do a stat()
on the thing _anyway_ just to determine whether it's "executable in the
'execve()' sense" or "executable in the 'path lookup' sense").
Linus
^ permalink raw reply
* Re: Git 1.3.2 on Solaris
From: Edgar Toernig @ 2006-05-23 18:43 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Jason Riedy, Stefan Pfetzing, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0605230829020.5623@g5.osdl.org>
Linus Torvalds wrote:
>
> [ And how the heck does anybody still run 2.0, btw? ]
Why not? There was never the need for an upgrade.
All connected hardware is well supported. Sure,
there are no binary packages for it and you have to
compile from sources but most apps can be made to
run on it (on the way you learn to hate autoconf).
And I wish the 2.6 system were as reliable as the
2.0 one ...
Ciao, ET.
^ permalink raw reply
* Re: Git 1.3.2 on Solaris
From: Linus Torvalds @ 2006-05-23 18:24 UTC (permalink / raw)
To: Jason Riedy; +Cc: Git Mailing List
In-Reply-To: <19270.1148407414@lotus.CS.Berkeley.EDU>
On Tue, 23 May 2006, Jason Riedy wrote:
>
> - Btw, even SuS says:
> [...]
> - New implementations are discouraged from returning X_OK unless at
> - least one execution permission bit is set."
>
> Now there is one possible, cross-OS problem that I
> haven't tested. You can chmod a-x and then use
> setfacl to grant one person execute access. I'm not
> sure if access works in that case, but that might
> possibly just say that current ACL systems are crap.
I absolutely agree. That is why the OS has a "access()" system call. It's
there to ask the OS whether the file is executable (or readable/writable).
Otherwise, we'd just do
static inline int executable(const char *path)
{
struct stat st;
return !stat(pathname, &st) &&
S_ISREG(st.st_mode) &&
(st.st_mode & 0111) != 0;
}
and be done with it. But exactly because the OS knows what "executable"
means, we ask it. We don't know about all the ACL's etc, the OS does.
(Similar issues are true for writability too - the file may be "writable"
in the sense that the write permission bits are on, but if the filesystem
is mounted read-only, it sure as hell ain't W_OK _anyway_).
> Hmm. Does access handle SELinux or the other systems?
Yup.
Modulo bugs, of course, but yes, access() on linux should check both
POSIX ACL's and SELinux security extensions. It uses exactly the same
code-paths that open()/execve() does: it uses the "vfs_permission()"
function which is also what execve() uses.
Now, I think access() actually misses a no-exec mount (it doesn't seem to
check MNT_NOEXEC for X_OK), and that looks like it might actually be a
real bug.
Linus
^ permalink raw reply
* [PATCH 2/2] Again: add more informative error messages to git-mktag
From: Björn Engelmann @ 2006-05-23 18:20 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 1 bytes --]
[-- Attachment #2: 0002-add-more-informative-error-messages-to-git-mktag.txt --]
[-- Type: text/plain, Size: 2838 bytes --]
Signed-off-by: Björn Engelmann <BjEngelmann@gmx.de>
---
51465a979a25c4363010cbab5798d95ac9f102e7
mktag.c | 28 ++++++++++++++++------------
1 files changed, 16 insertions(+), 12 deletions(-)
51465a979a25c4363010cbab5798d95ac9f102e7
diff --git a/mktag.c b/mktag.c
index f1598db..6bd45df 100644
--- a/mktag.c
+++ b/mktag.c
@@ -46,41 +46,45 @@ static int verify_tag(char *buffer, unsi
const char *object, *type_line, *tag_line, *tagger_line;
if (size < 64)
- return -1;
+ return error("wanna fool me ? you obviously got the size wrong !\n");
+
buffer[size] = 0;
/* Verify object line */
object = buffer;
if (memcmp(object, "object ", 7))
- return -1;
+ return error("char%d: does not start with \"object \"\n", 0);
+
if (get_sha1_hex(object + 7, sha1))
- return -1;
+ return error("char%d: could not get SHA1 hash\n", 7);
/* Verify type line */
type_line = object + 48;
if (memcmp(type_line - 1, "\ntype ", 6))
- return -1;
+ return error("char%d: could not find \"\\ntype \"\n", 47);
/* Verify tag-line */
tag_line = strchr(type_line, '\n');
if (!tag_line)
- return -1;
+ return error("char%td: could not find next \"\\n\"\n", type_line - buffer);
tag_line++;
if (memcmp(tag_line, "tag ", 4) || tag_line[4] == '\n')
- return -1;
+ return error("char%td: no \"tag \" found\n", tag_line - buffer);
/* Get the actual type */
typelen = tag_line - type_line - strlen("type \n");
if (typelen >= sizeof(type))
- return -1;
+ return error("char%d: type too long\n", (int)(type_line - buffer) + strlen("type \n"));
+
memcpy(type, type_line+5, typelen);
type[typelen] = 0;
/* Verify that the object matches */
if (get_sha1_hex(object + 7, sha1))
- return -1;
+ return error("char%d: could not get SHA1 hash but this is really odd since i got it before !\n", 7);
+
if (verify_object(sha1, type))
- return -1;
+ return error("char%d: could not verify object %s\n", 7, sha1);
/* Verify the tag-name: we don't allow control characters or spaces in it */
tag_line += 4;
@@ -90,14 +94,14 @@ static int verify_tag(char *buffer, unsi
break;
if (c > ' ')
continue;
- return -1;
+ return error("char%td: could not verify tag name\n", tag_line - buffer);
}
/* Verify the tagger line */
tagger_line = tag_line;
if (memcmp(tagger_line, "tagger", 6) || (tagger_line[6] == '\n'))
- return -1;
+ return error("char%td: could not find \"tagger\"\n", tagger_line - buffer);
/* The actual stuff afterwards we don't care about.. */
return 0;
@@ -118,7 +122,7 @@ int main(int argc, char **argv)
free(buffer);
die("could not read from stdin");
}
-
+
// Verify it for some basic sanity: it needs to start with "object <sha1>\ntype\ntagger "
if (verify_tag(buffer, size) < 0)
die("invalid tag signature file");
--
1.3.3.g4309-dirty
^ permalink raw reply related
* [PATCH 1/2] Again: remove the artificial restriction tagsize < 8kb
From: Björn Engelmann @ 2006-05-23 18:19 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 1 bytes --]
[-- Attachment #2: 0001-remove-the-artificial-restriction-tagsize-8kb.txt --]
[-- Type: text/plain, Size: 4182 bytes --]
Signed-off-by: Björn Engelmann <BjEngelmann@gmx.de>
---
430953ceafbc722226e2300b489e38938220d435
cache.h | 1 +
mktag.c | 19 +++++++++----------
sha1_file.c | 46 ++++++++++++++++++++++++++++++++++++----------
3 files changed, 46 insertions(+), 20 deletions(-)
430953ceafbc722226e2300b489e38938220d435
diff --git a/cache.h b/cache.h
index 4b7a439..19e90eb 100644
--- a/cache.h
+++ b/cache.h
@@ -154,6 +154,7 @@ extern int ce_match_stat(struct cache_en
extern int ce_modified(struct cache_entry *ce, struct stat *st, int);
extern int ce_path_match(const struct cache_entry *ce, const char **pathspec);
extern int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object, const char *type);
+extern int read_pipe(int fd, char** return_buf, unsigned long* return_size);
extern int index_pipe(unsigned char *sha1, int fd, const char *type, int write_object);
extern int index_path(unsigned char *sha1, const char *path, struct stat *st, int write_object);
extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st);
diff --git a/mktag.c b/mktag.c
index 2328878..f1598db 100644
--- a/mktag.c
+++ b/mktag.c
@@ -45,7 +45,7 @@ static int verify_tag(char *buffer, unsi
unsigned char sha1[20];
const char *object, *type_line, *tag_line, *tagger_line;
- if (size < 64 || size > MAXSIZE-1)
+ if (size < 64)
return -1;
buffer[size] = 0;
@@ -105,8 +105,8 @@ static int verify_tag(char *buffer, unsi
int main(int argc, char **argv)
{
- unsigned long size;
- char buffer[MAXSIZE];
+ unsigned long size = 4096;
+ char *buffer = malloc(size);
unsigned char result_sha1[20];
if (argc != 1)
@@ -114,13 +114,9 @@ int main(int argc, char **argv)
setup_git_directory();
- // Read the signature
- size = 0;
- for (;;) {
- int ret = xread(0, buffer + size, MAXSIZE - size);
- if (ret <= 0)
- break;
- size += ret;
+ if (read_pipe(0, &buffer, &size)) {
+ free(buffer);
+ die("could not read from stdin");
}
// Verify it for some basic sanity: it needs to start with "object <sha1>\ntype\ntagger "
@@ -129,6 +125,9 @@ int main(int argc, char **argv)
if (write_sha1_file(buffer, size, tag_type, result_sha1) < 0)
die("unable to write tag file");
+
+ free(buffer);
+
printf("%s\n", sha1_to_hex(result_sha1));
return 0;
}
diff --git a/sha1_file.c b/sha1_file.c
index 2230010..e444d9d 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1645,16 +1645,24 @@ int has_sha1_file(const unsigned char *s
return find_sha1_file(sha1, &st) ? 1 : 0;
}
-int index_pipe(unsigned char *sha1, int fd, const char *type, int write_object)
+/*
+ * reads from fd as long as possible into a supplied buffer of size bytes.
+ * If neccessary the buffer's size is increased using realloc()
+ *
+ * returns 0 if anything went fine and -1 otherwise
+ *
+ * NOTE: both buf and size may change, but even when -1 is returned
+ * you still have to free() it yourself.
+ */
+int read_pipe(int fd, char** return_buf, unsigned long* return_size)
{
- unsigned long size = 4096;
- char *buf = malloc(size);
- int iret, ret;
+ char* buf = *return_buf;
+ unsigned long size = *return_size;
+ int iret;
unsigned long off = 0;
- unsigned char hdr[50];
- int hdrlen;
+
do {
- iret = read(fd, buf + off, size - off);
+ iret = xread(fd, buf + off, size - off);
if (iret > 0) {
off += iret;
if (off == size) {
@@ -1663,16 +1671,34 @@ int index_pipe(unsigned char *sha1, int
}
}
} while (iret > 0);
- if (iret < 0) {
+
+ *return_buf = buf;
+ *return_size = off;
+
+ if (iret < 0)
+ return -1;
+ return 0;
+}
+
+int index_pipe(unsigned char *sha1, int fd, const char *type, int write_object)
+{
+ unsigned long size = 4096;
+ char *buf = malloc(size);
+ int ret;
+ unsigned char hdr[50];
+ int hdrlen;
+
+ if (read_pipe(fd, &buf, &size)) {
free(buf);
return -1;
}
+
if (!type)
type = blob_type;
if (write_object)
- ret = write_sha1_file(buf, off, type, sha1);
+ ret = write_sha1_file(buf, size, type, sha1);
else {
- write_sha1_file_prepare(buf, off, type, sha1, hdr, &hdrlen);
+ write_sha1_file_prepare(buf, size, type, sha1, hdr, &hdrlen);
ret = 0;
}
free(buf);
--
1.3.3.g4309-dirty
^ permalink raw reply related
* Re: Git 1.3.2 on Solaris
From: Jason Riedy @ 2006-05-23 18:03 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0605230744350.5623@g5.osdl.org>
And Linus Torvalds writes:
-
- What kind of CRAP has Solaris become?
Become? heh. Check mount's output; it's "mountpoint
on device". Always has been. I think there might be
a reason why certain other OSes have eaten their lunch,
and it ain't the price.
- It wasn't about what was "allowed by the standards",
- that was the HP-SUX and AIX's excuses.
No, AIX's excuse is that it's "dictated by these three
standards over here and disallowed by those two, so
clearly we have to support both behaviors depending
on some footnote in our 1e9 page manual." wheee...
- Have Sun people forgotten the difference between "quality" and "crap that
- passes standards tests"?
As far as I've been told, Sun's more interested in
near-perfect backwards compatibility than external
standards tests. It worked for Intel, right? ;)
- Btw, even SuS says:
[...]
- New implementations are discouraged from returning X_OK unless at
- least one execution permission bit is set."
Now there is one possible, cross-OS problem that I
haven't tested. You can chmod a-x and then use
setfacl to grant one person execute access. I'm not
sure if access works in that case, but that might
possibly just say that current ACL systems are crap.
Hmm. Does access handle SELinux or the other systems?
That might be interesting for a public git server, but
I don't know enough about it.
- Somebody hit some Solaris engineers with a 2x4 clue-stick, please.
I think you're targetting the wrong department...
Their hands are tied.
Jason, wondering if you could resist the SUS bait...
^ permalink raw reply
* Re: [PATCH 2/2] cvsimport: cleanup commit function
From: Morten Welinder @ 2006-05-23 17:47 UTC (permalink / raw)
To: Martin Langhoff, Junio C Hamano, Matthias Urlichs, git
In-Reply-To: <20060523070007.GC6180@coredump.intra.peff.net>
Why run "env" and not just muck with %ENV?
M.
> + my $pid = open2(my $commit_read, my $commit_write,
> + 'env',
> + "GIT_AUTHOR_NAME=$author_name",
> + "GIT_AUTHOR_EMAIL=$author_email",
> + "GIT_AUTHOR_DATE=$commit_date",
> + "GIT_COMMITTER_NAME=$author_name",
> + "GIT_COMMITTER_EMAIL=$author_email",
> + "GIT_COMMITTER_DATE=$commit_date",
> + 'git-commit-tree', $tree, @commit_args);
^ permalink raw reply
* Re: [PATCH 2/2] cvsimport: cleanup commit function
From: Linus Torvalds @ 2006-05-23 16:50 UTC (permalink / raw)
To: Martin Langhoff; +Cc: Junio C Hamano, Matthias Urlichs, git
In-Reply-To: <46a038f90605230113x2f6b0e4bq5a2ea97308b495e0@mail.gmail.com>
Hmm. Is it just me, or does the current "git cvsimport" have new problems:
[torvalds@merom git]$ git cvsimport -d ~/CVS gentoo-x86
causes
Committing initial tree 34bd3dcd4bfd79bad35ce3fb08b2e21108195db8
Server has gone away while fetching BUGS-TODO 1.1, retrying...
Retry failed at /home/torvalds/bin/git-cvsimport line 366, <GEN2656> line 9.
and that's it for the import.
I don't see what would have caused it in the changes, but it definitely
worked earlier..
Linus
^ permalink raw reply
* Re: Make more commands builtin
From: Ryan Anderson @ 2006-05-23 16:32 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <e4v1a0$u6c$1@sea.gmane.org>
On Tue, May 23, 2006 at 03:05:12PM +0200, Jakub Narebski wrote:
> Peter Eriksen wrote:
>
> > On Tue, May 23, 2006 at 02:36:54PM +0200, Jakub Narebski wrote:
> >> Peter Eriksen wrote:
> >>
> >> > Btw.
> >> >
> >> > I used these commands to produce the patch series:
> >> >
> >> > git diff --stat -C 24b65a30015aedd..pe/builtin
> >> > git-send-email --no-chain-reply-to --compose \
> >> > --from=s022018@student.dtu.dk --not-signed-off-by-cc \
> >> > --quiet \
> >> > --subject="Make more commands builtin" \
> >> > --to=git@vger.kernel.org Patches/*
> >>
> >> I wonder why the patches themselves are not replies to the main/summary
> >> email, i.e. "Make more commands builtin" email...
> >
> > It seems thay are:
> >
> > Subject: Make more commands builtin
> > Message-Id: <11483865361243-git-send-email-1>
> >
> > Subject: [PATCH 1/8] Builtin git-ls-files.
> > Message-Id: <11483865362613-git-send-email-1>
> > In-Reply-To: <11483865361243-git-send-email-1>
>
> Ahh... I'm reading git mailing list through GMane NNTP interface.
> It would be nice if git-send-email added 'References:' Usenet/news
> header in addition to email one 'In-Reply-To:'.
Things would have worked better if --no-chain-reply-to had *not* been
used, at least for those of us reading via mutt.
I'm still in the process of getting my machines setup after moving, when
I get sorted out, I'll try to look into this.
^ permalink raw reply
* Re: [PATCH] cvsimport: introduce -L<imit> option to workaround memory leaks
From: Linus Torvalds @ 2006-05-23 16:25 UTC (permalink / raw)
To: Theodore Tso
Cc: Martin Langhoff, Git Mailing List, Junio C Hamano,
Johannes.Schindelin, spyderous, smurf
In-Reply-To: <Pine.LNX.4.64.0605230848060.5623@g5.osdl.org>
On Tue, 23 May 2006, Linus Torvalds wrote:
>
> So an unpacked git archive on ext3 (but not ext2, I believe: ext2 should
> use the page cache for directories) ends up being very buffer-cache
> intensive. And the buffer cache is basically deprecated..
A few notes: I'm not 100% sure things really fit in the 2GB (*), so it
really may be IO limited, and the low CPU use is just because that machine
also happens to have a dang fast next-gen Intel CPU that hasn't even been
released yet.
Also, I do realize that hashed directories should actually decrease the
buffer cache pressure too, just because we wouldn't need to read all of
the directory for a lookup.
Linus
(*) cvsps itself grows to 1.6GB of the 2GB and while that memory should be
largely idle, the problem may simply be that we don't swap it out eagerly
enough. Allowing filesystem metadata to swap out processes is something
we've tuned against, because it tends to result in horrible interactive
behaviour after a nightly "updatedb" run. So it's entirely possible that
this is all normal..
^ permalink raw reply
* Re: [PATCH] cvsimport: introduce -L<imit> option to workaround memory leaks
From: Linus Torvalds @ 2006-05-23 16:05 UTC (permalink / raw)
To: Theodore Tso
Cc: Martin Langhoff, Git Mailing List, Junio C Hamano,
Johannes.Schindelin, spyderous, smurf
In-Reply-To: <20060523153636.GA21506@thunk.org>
On Tue, 23 May 2006, Theodore Tso wrote:
> On Mon, May 22, 2006 at 07:28:37PM -0700, Linus Torvalds wrote:
> >
> > I actually think that I found a real ext3 performance bug from trying to
> > determine why git sometimes slows down ridiculously when the tree has been
> > allowed to go too long without a repack.
>
> Do you have dir_index (the hashed btree) feature enabled by any chance?
No, and I know I probably should, since it would hopefully help git usage.
But my problem actually happens even with moderately sized directories:
they were just 40kB or so in size, and the problem isn't high system CPU
usage, but tons of extra IO. I ran things on a machine with 2GB of RAM,
and as far as I could tell, the working set _should_ have fit into memory,
but CPU utilization was consistently in the 1% range.
Now, it's possible that I'm just wrong, and it really didn't fit in
memory, but I I _suspect_ that the issue is that ext3 directory handling
still uses the "buffer_head" thing rather than the page cache, and that we
simply don't LRU the memory appropriately so we don't let the memory
pressure expand the buffer cache.
Now, using buffer cache in this day and age is insane and horrible
(there's a reason I suspect the LRU doesn't work that well: the buffer
heads aren't supposed to be used as a cache, and people are supposed to
use the page cache for it these days), but Andrew tells me that the whole
JBD thing basically requires it. Whatever.
Now, repacking obviously hides it entirely (because then the load becomes
entirely a page-cache load, and the kernel does _that_ beautifully), but
I'm a bit bummed that I think I hit an ext3 braindamage.
So an unpacked git archive on ext3 (but not ext2, I believe: ext2 should
use the page cache for directories) ends up being very buffer-cache
intensive. And the buffer cache is basically deprecated..
Linus
PS. I'll see if I can figure out the problem, and maybe the good news is
that I'll be able to just fix a real kernel performance issue. Still,
there's a _reason_ we tried to get away from the buffer heads as a caching
entity..
^ permalink raw reply
* Re: [PATCH] cvsimport: introduce -L<imit> option to workaround memory leaks
From: Theodore Tso @ 2006-05-23 15:36 UTC (permalink / raw)
To: Linus Torvalds
Cc: Martin Langhoff, Git Mailing List, Junio C Hamano,
Johannes.Schindelin, spyderous, smurf
In-Reply-To: <Pine.LNX.4.64.0605221926270.3697@g5.osdl.org>
On Mon, May 22, 2006 at 07:28:37PM -0700, Linus Torvalds wrote:
>
>
> This stupid patch on top of yours seems to make git happier. It's
> disgusting, I know, but it just repacks things every kilo-commit.
>
> I actually think that I found a real ext3 performance bug from trying to
> determine why git sometimes slows down ridiculously when the tree has been
> allowed to go too long without a repack.
Do you have dir_index (the hashed btree) feature enabled by any chance?
- Ted
^ permalink raw reply
* Re: Git 1.3.2 on Solaris
From: Linus Torvalds @ 2006-05-23 15:31 UTC (permalink / raw)
To: Edgar Toernig; +Cc: Jason Riedy, Stefan Pfetzing, Git Mailing List
In-Reply-To: <20060523172053.60ec1145.froese@gmx.de>
On Tue, 23 May 2006, Edgar Toernig wrote:
>
> But I was hit by this effect on my system which is - surprise surprise -
> Linux :-) Ok, it's a pretty old one with a 2.0 kernel and libc 5.
Yes, we've had that bug too, and yes, I was hit by a clue-stick, and still
have the bruise. That's how you teach people.
[ And how the heck does anybody still run 2.0, btw? ]
Linus
^ permalink raw reply
* Re: Git 1.3.2 on Solaris
From: Edgar Toernig @ 2006-05-23 15:20 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Jason Riedy, Stefan Pfetzing, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0605230744350.5623@g5.osdl.org>
Linus Torvalds wrote:
>
> > - printf ("access: %d\n", access("/etc/motd", X_OK));
> > [...]
> > - will return 0 on solaris - when run as root, even though /etc/motd
> > - is not executeable.
> >
> > This is explicitly allowed by the SUS, even for non-root:
>
> New implementations are discouraged from returning X_OK unless at
> least one execution permission bit is set."
>
> which clearly says "Solaris is CRAP" to me.
Just for the record: firefox's download manager performs exactly this
test to decide whether you can 'open with' a file (pretty silly because
the test is done on the freshly downloaded file in the temp dir which
never has an x-bit set). But I was hit by this effect on my system
which is - surprise surprise - Linux :-) Ok, it's a pretty old one
with a 2.0 kernel and libc 5. But nevertheless, access(2) is not the
right function to portably test the x-bit.
Ciao, ET.
^ permalink raw reply
* Re: Git 1.3.2 on Solaris
From: Linus Torvalds @ 2006-05-23 14:53 UTC (permalink / raw)
To: Jason Riedy; +Cc: Stefan Pfetzing, Git Mailing List
In-Reply-To: <8157.1148359875@lotus.CS.Berkeley.EDU>
On Mon, 22 May 2006, Jason Riedy wrote:
> And "Stefan Pfetzing" writes:
> - printf ("access: %d\n", access("/etc/motd", X_OK));
> [...]
> - will return 0 on solaris - when run as root, even though /etc/motd
> - is not executeable.
>
> This is explicitly allowed by the SUS, even for non-root:
What kind of CRAP has Solaris become?
Yes, it's allowed. That doesn't mean that a quality implementation should
do it.
SunOS used to be the best system around - it was, compared to others,
_nice_ to work with. It wasn't about what was "allowed by the standards",
that was the HP-SUX and AIX's excuses. It was the highest-quality
implementation.
Now, Solaris had some serious problems early on (yes, I was there when
they switched, and yes, I hated it), but I thought they had fixed their
stuff long long ago.
Have Sun people forgotten the difference between "quality" and "crap that
passes standards tests"?
Not doing a reasonable job on "access()" is a joke. It's like VMS being
"posix". Sure, it's the letter of the law, but it's still not _unix_.
Btw, even SuS says:
"The sentence concerning appropriate privileges and execute permission
bits reflects the two possibilities implemented by historical
implementations when checking superuser access for X_OK.
New implementations are discouraged from returning X_OK unless at
least one execution permission bit is set."
which clearly says "Solaris is CRAP" to me.
What the heck is going on? First the totally broken stdio that doesn't
retry on EINTR, now access(). And these people think they can compete?
Somebody hit some Solaris engineers with a 2x4 clue-stick, please.
Linus
^ permalink raw reply
* Re: [PATCH] git status: ignore empty directories (because they cannot be added)
From: Matthias Lederhofer @ 2006-05-23 14:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd5e56yi6.fsf@assigned-by-dhcp.cox.net>
> > Well, anyway, here the reasons for this patch:
> > - Working in a git repository with a lot of empty directories is
> > annoying, because all of them show up in git status even though they
> > cannot be added. With --no-empty-directories they are hidden.
>
> What directories are they? Will they some day have files that
> you might want to keep track of? The reason for this question
> is, "otherwise you could .gitignore them".
I have been trying ruby on rails and there are some empty directories
which will have files later. I would even add those empty directories
if it would be possible.
> > - If there is a directory which may be added because it is quite
> > useful to have the -u option to see what is in there to add (without
> > using ls path/to/directory).
>
> It really depends on how many files there are in such a
> "interesting" directory _and_ other "uninteresting" directories
> full of transitory files. The -u option that disables "skip
> contents and show only the top directory" behaviour globally
> might not be so useful in such a case -- you will see useful
> contents of an otherwise "closed" directory (because it defeats
> the --directory flag), but at the same time you would get tons
> of uninteresting files in another directory as well in such a
> case. You are likely to end up doing ls path/to/dir after
> noticing that there are non-empty foo/ and bar/ directories that
> have no tracked files, and you know which one has files
> interesting to you.
> So I am OK with the change, but I am somewhat still doubtful how
> useful the option would be.
The case I think about is that there are some directories which are
not tracked because they contain no files yet. All other directories
which are uninteresting are added to .gitignore. If an untracked
directory shows up in git-status one could easily check with
git-status -u what can be added.
PS: I had a typo in the git mailing list e-mail address and resent my
last e-mail to the list. Your reply did not make it to the list,
perhaps you can send it again.
^ permalink raw reply
* Re: Make more commands builtin
From: Jakub Narebski @ 2006-05-23 13:05 UTC (permalink / raw)
To: git
In-Reply-To: <20060523125400.GA11128@bohr.gbar.dtu.dk>
Peter Eriksen wrote:
> On Tue, May 23, 2006 at 02:36:54PM +0200, Jakub Narebski wrote:
>> Peter Eriksen wrote:
>>
>> > Btw.
>> >
>> > I used these commands to produce the patch series:
>> >
>> > git diff --stat -C 24b65a30015aedd..pe/builtin
>> > git-send-email --no-chain-reply-to --compose \
>> > --from=s022018@student.dtu.dk --not-signed-off-by-cc \
>> > --quiet \
>> > --subject="Make more commands builtin" \
>> > --to=git@vger.kernel.org Patches/*
>>
>> I wonder why the patches themselves are not replies to the main/summary
>> email, i.e. "Make more commands builtin" email...
>
> It seems thay are:
>
> Subject: Make more commands builtin
> Message-Id: <11483865361243-git-send-email-1>
>
> Subject: [PATCH 1/8] Builtin git-ls-files.
> Message-Id: <11483865362613-git-send-email-1>
> In-Reply-To: <11483865361243-git-send-email-1>
Ahh... I'm reading git mailing list through GMane NNTP interface.
It would be nice if git-send-email added 'References:' Usenet/news
header in addition to email one 'In-Reply-To:'.
--
Jakub Narebski
Warsaw, Poland
^ permalink raw reply
* Re: Make more commands builtin
From: Peter Eriksen @ 2006-05-23 12:54 UTC (permalink / raw)
To: git
In-Reply-To: <e4uvku$o28$1@sea.gmane.org>
On Tue, May 23, 2006 at 02:36:54PM +0200, Jakub Narebski wrote:
> Peter Eriksen wrote:
>
> > Btw.
> >
> > I used these commands to produce the patch series:
> >
> > git diff --stat -C 24b65a30015aedd..pe/builtin
> > git-send-email --no-chain-reply-to --compose \
> > --from=s022018@student.dtu.dk --not-signed-off-by-cc \
> > --quiet \
> > --subject="Make more commands builtin" \
> > --to=git@vger.kernel.org Patches/*
>
> I wonder why the patches themselves are not replies to the main/summary
> email, i.e. "Make more commands builtin" email...
It seems thay are:
Subject: Make more commands builtin
Message-Id: <11483865361243-git-send-email-1>
Subject: [PATCH 1/8] Builtin git-ls-files.
Message-Id: <11483865362613-git-send-email-1>
In-Reply-To: <11483865361243-git-send-email-1>
Subject: [PATCH 2/8] Builtin git-ls-tree.
Message-Id: <11483865362923-git-send-email-1>
In-Reply-To: <11483865361243-git-send-email-1>
Peter
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox