* Usage of isspace and friends
From: Morten Welinder @ 2005-10-12 1:40 UTC (permalink / raw)
To: GIT Mailing List
Someone needs to audit the usage of isspace, tolower, and friends. There are
things like this in the code:
static int is_dev_null(const char *str)
{
return !memcmp("/dev/null", str, 9) && isspace(str[9]);
}
Since str[9] is of type char it should not be used as a argument to
isspace directly,
but rather be cast to unsigned char:
... isspace((unsigned char)str[9]);
Admittedly that is ugly. Blame K&R. (Glibc has a partial workaround for this
kind of coding bug. On the up side you won't get a crash, but on the down
side you can get the wrong result.)
Morten
^ permalink raw reply
* Re: [PATCH] Add git-findtags
From: Junio C Hamano @ 2005-10-13 5:18 UTC (permalink / raw)
To: Martin Langhoff; +Cc: git
In-Reply-To: <46a038f90510122117mb466722n531dc66bac141ea1@mail.gmail.com>
Martin Langhoff <martin.langhoff@gmail.com> writes:
> On 10/12/05, Junio C Hamano <junkio@cox.net> wrote:
>> Martin Langhoff <martin@catalyst.net.nz> writes:
>
> I'm preparing a better patch based on your comments, but File::Find is
> _not_ my friend, really. I really feel stupid after failing for 1hr to
> use it.
Something like this?
use strict;
use File::Find qw(find);
my $git_dir = $ENV{GIT_DIR} || '.git';
my @tagfiles = ();
find({
follow => 1,
wanted => sub {
if (-f _) {
push @tagfiles, $File::Find::name;
}
} }, "$git_dir/refs/tags");
for (@tagfiles) {
print "$_\n";
}
>> BTW, wouldn't it be easier for this particular script, and more
>> useful in general, if something like what 'git-rev-parse' does
>> for commit objects when given "REV^0" is supported for tags?
>
> I don't quite follow...
What I meant is this.
There is an existing notation "^0" which is a postfix
"dereference until you get a commit" operator.
git-rev-parse --verify refs/tags/v0.99^0
git-cat-file -t refs/tags/v0.99^0
does:
1. reads SHA1 from "refs/tags/v0.99", finds the object;
2. if it is a tag object, find the object pointed by it;
if the result is still a tag object, then dereference
it repeatedly;
3. if the resulting object is a commit, let the caller
to use it; otherwise barf.
What _might_ be useful for your application is a similar
operator, say, "refs/tags/junio-gpg-pub%", that does:
1. reads SHA1 from "refs/tags/junio-gpg-pub", finds the
object;
2. if it is a tag object, find the object pointed by it;
if the result is still a tag object, then dereference
it repeatedly;
3. do not worry about the type of the result. Just
output it.
Instead of reserving yet another letter '%', it might be better
to use something like "refs/tags/junio-gpg-pub^{tag}" as a
notation for this. If you had something like this, you would
not have to read tag objects yourself and dereference them by
hand.
^ permalink raw reply
* Re: maybe breakage with latest git-pull and http protocol
From: Junio C Hamano @ 2005-10-13 5:50 UTC (permalink / raw)
To: Randal L. Schwartz; +Cc: git
In-Reply-To: <867jciz18w.fsf@blue.stonehenge.com>
merlyn@stonehenge.com (Randal L. Schwartz) writes:
> I updated git to d06b689a933f6d2130f8afdf1ac0ddb83eeb59ab,
> then compiled and installed.
>
> When I went to "git-pull" on my cogito archive (which I had edited
> to use HTTP instead of RSYNC), I got into trouble. Unfortunately,
> I changed it to rsync to force cogito into a sane state before
> I realized that this would be a good bug report. :)
Indeed I wish we could see the set of refs you had and output
from fsck-objects before the failed git-pull and after.
One thing I am aware of is that cogito repository at kernel.org
is not set up to be HTTP friendly -- it lacks info/refs file
git-clone uses for discovery of the available refs.
Cogito's clone/fetch over HTTP uses recursive wget for
discovery, and I presume that is one of the reasons nobody
noticed this. Another reason may probably be that more people
use rsync transport.
^ permalink raw reply
* Re: Usage of isspace and friends
From: Junio C Hamano @ 2005-10-13 6:49 UTC (permalink / raw)
To: Morten Welinder; +Cc: git
In-Reply-To: <118833cc0510111840k715e1190l54ad65f821c77848@mail.gmail.com>
Morten Welinder <mwelinder@gmail.com> writes:
> Someone needs to audit the usage of isspace, tolower, and
> friends. There are things like this in the code:
>
> static int is_dev_null(const char *str)
> {
> return !memcmp("/dev/null", str, 9) && isspace(str[9]);
> }
>
> Since str[9] is of type char it should not be used as a argument to
> isspace directly,
> but rather be cast to unsigned char:
>
> ... isspace((unsigned char)str[9]);
Huh? isspace is "int isspace(int)". Presumably standard
integral promotion rules applies here whether char is signed or
unsigned, doesn't it?
The snippet you quoted is from apply.c, and I would say what is
more problematic is that we do not force C locale while parsing
the diff (see another thread -- we would want to process diffs
as byte streams).
^ permalink raw reply
* Re: diff_tree_stdin
From: Junio C Hamano @ 2005-10-13 7:11 UTC (permalink / raw)
To: Morten Welinder; +Cc: git
In-Reply-To: <118833cc0510111846q42c5d7e5j162bdacd49dfebbc@mail.gmail.com>
Morten Welinder <mwelinder@gmail.com> writes:
> It looks like diff_tree_stdin can overrun the this_header buffer. Since the
> line length is already calculated, a check would be cheap.
I presume you are talking about "line", not this_header[], since
you are talking about something whose length is already
calculated.
The output buffer this_header[] only ever gets two 40-byte SHA1
and a handful more, so probably 128 bytes would be big enough --
the current 1000 is overkill.
The input line[] is first NUL terminated before getting scanned,
and scanning with get_sha1_hex() stops immediately when we see
NUL, and premature NUL makes it fail, so the first
get_sha1_hex() would not overrun. If the first SHA1 is followed
by garbage then the second get_sha1_hex() would not succeed
without overrunning either. If line[40] is NUL then we do not
even do the second get_sha1_hex() --- in any case I do not see
overrun.
I am getting tired (it _was_ my GIT day, but unfortunately I had
to be worried about another day-job project during the day X-<),
and I might probably be overlooking something fairly obvious to
you. Care to explain?
^ permalink raw reply
* Re: openbsd version?
From: Junio C Hamano @ 2005-10-13 7:47 UTC (permalink / raw)
To: Sven Verdoolaege; +Cc: git, Randal L. Schwartz
In-Reply-To: <7vzmph1225.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
> Sven Verdoolaege <skimo@kotnet.org> writes:
>
>> I think you mean
>>
>> $ git-update-ref refs/heads/mybranch mybranch^
>
> Of course you are right. Thanks.
And I am an idiot. I did that myself again, and ended up
wasting 30 minutes or so, scratching my head.
Maybe there should be a safety measure built into git-update-ref
that says single-level name (i.e. not starting with refs/) gets
warning unless all uppercase or something silly like that to
protect idiots like myself. Hmmm.
^ permalink raw reply
* Re: Usage of isspace and friends
From: Antti-Juhani Kaijanaho @ 2005-10-13 8:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Morten Welinder, git
In-Reply-To: <7vachd6hdx.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> Morten Welinder <mwelinder@gmail.com> writes:
>>Since str[9] is of type char it should not be used as a argument to
>>isspace directly,
>>but rather be cast to unsigned char:
>>
>> ... isspace((unsigned char)str[9]);
>
>
> Huh? isspace is "int isspace(int)". Presumably standard
> integral promotion rules applies here whether char is signed or
> unsigned, doesn't it?
Of course, but that's not the issue. isspace treats its parameter as if
it had been converted from unsigned char to int. If char is signed,
ïsspace will mistreat those characters that have a negative value.
Then again, I don't think a space character, one that the C locale
regards as such, anyway, wiill ever have a negative value, so the issue
is rather academic.
> The snippet you quoted is from apply.c, and I would say what is
> more problematic is that we do not force C locale while parsing
> the diff
Quite true. One reason I tend to avoid the standard is* functions in my
own code.
--
Antti-Juhani
^ permalink raw reply
* Re: openbsd version?
From: Sven Verdoolaege @ 2005-10-13 8:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Randal L. Schwartz
In-Reply-To: <7vpsq9504x.fsf@assigned-by-dhcp.cox.net>
On Thu, Oct 13, 2005 at 12:47:26AM -0700, Junio C Hamano wrote:
> Maybe there should be a safety measure built into git-update-ref
> that says single-level name (i.e. not starting with refs/) gets
> warning unless all uppercase or something silly like that to
> protect idiots like myself. Hmmm.
How about checking whether the name also exists with a certain
prefix instead ?
Otherwise you'll also disallow updating ORIG_HEAD and stuff.
skimo
^ permalink raw reply
* [PATCH] Add findtags - reworked
From: Martin Langhoff @ 2005-10-13 8:56 UTC (permalink / raw)
To: git; +Cc: Martin Langhoff
A short perl script that will walk the tag refs, tag objects, and even commit
objects in its quest to figure out whether the given SHA1 (for a commit or
tree) was ever tagged.
This version is reworked incorporating sanity, feature and style fixes from
Junio.
Usage: git-findtags.perl [ -t ] <commit-or-tree-sha1>
Signed-off-by: Martin Langhoff <martin@catalyst.net.nz>
---
Makefile | 3 +-
git-findtags.perl | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 96 insertions(+), 1 deletions(-)
create mode 100755 git-findtags.perl
applies-to: ef9b5c2cb61cf509adf2f5ef37fa1db517291f48
342d4cd1c66d8e58e1ba2221366c9237f9197b03
diff --git a/Makefile b/Makefile
index 8697d52..a5d9cd4 100644
--- a/Makefile
+++ b/Makefile
@@ -93,7 +93,8 @@ SCRIPT_SH = \
SCRIPT_PERL = \
git-archimport.perl git-cvsimport.perl git-relink.perl \
- git-rename.perl git-shortlog.perl git-fmt-merge-msg.perl
+ git-rename.perl git-shortlog.perl git-fmt-merge-msg.perl \
+ git-findtags.perl
SCRIPT_PYTHON = \
git-merge-recursive.py
diff --git a/git-findtags.perl b/git-findtags.perl
new file mode 100755
index 0000000..745affe
--- /dev/null
+++ b/git-findtags.perl
@@ -0,0 +1,94 @@
+#!/usr/bin/perl -w
+#
+# Copyright (c) 2005 Martin Langhoff
+#
+# Walk the tags and find if they match a commit
+# expects a SHA1 of a commit. Option -t enables
+# searching trees too.
+#
+
+use strict;
+use File::Basename;
+use File::Find;
+use Getopt::Std;
+
+my $git_dir = $ENV{GIT_DIR} || '.git';
+$git_dir =~ s|/$||; # chomp trailing slash
+
+# options
+our $opt_t;
+getopts("t") || usage();
+
+my @tagfiles = `find $git_dir/refs/tags -follow -type f`; # haystack
+my $target = shift @ARGV; # needle
+unless ($target) {
+ usage();
+}
+
+# drive the processing from the find hook
+# slower, safer (?) than the find utility
+find( { wanted => \&process,
+ no_chdir => 1,
+ follow => 1,
+ }, "$git_dir/refs/tags");
+
+
+sub process {
+ my ($dev,$ino,$mode,$nlink,$uid,$gid);
+
+ # process only regular files
+ unless ((($dev,$ino,$mode,$nlink,$uid,$gid) = lstat($_)) && -f _) {
+ return 1; # ignored anyway
+ }
+
+ my $tagfile = $_;
+ chomp $tagfile;
+ my $tagname = substr($tagfile, length($git_dir.'/refs/tags/'));
+
+ my $tagid = quickread($tagfile);
+ chomp $tagid;
+
+ # is it just a soft tag?
+ if ($tagid eq $target) {
+ print "$tagname\n";
+ return 1; # done with this tag
+ }
+
+ # grab the first 2 lines (the whole tag could be large)
+ my $tagobj = `git-cat-file tag $tagid | head -n2 `;
+ if ($tagobj =~ m/^type commit$/m) { # only deal with commits
+
+ if ($tagobj =~ m/^object $target$/m) { # match on the commit
+ print "$tagname\n";
+
+ } elsif ( $opt_t && # follow the commit
+ $tagobj =~ m/^object (\S+)$/m) { # and try to match trees
+ my $commitid = $1;
+ my $commitobj = `git-cat-file commit $commitid | head -n1`;
+ chomp $commitobj;
+ $commitobj =~ m/^tree (\S+)$/;
+ my $treeid = $1;
+ if ($target eq $treeid) {
+ print "$tagname\n";
+ }
+ }
+ }
+}
+
+sub quickread {
+ my $file = shift;
+ local $/; # undef: slurp mode
+ open FILE, "<$file"
+ or die "Cannot open $file : $!";
+ my $content = <FILE>;
+ close FILE;
+ return $content;
+}
+
+sub usage {
+ print STDERR <<END;
+Usage: ${\basename $0} # find tags for a commit or tree
+ [ -t ] <commit-or-tree-sha1>
+END
+ exit(1);
+}
---
0.99.8.GIT
^ permalink raw reply related
* [PATCH] Typo fix
From: Ralf Baechle @ 2005-10-13 1:13 UTC (permalink / raw)
To: git
Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
diff --git a/rsh.c b/rsh.c
--- a/rsh.c
+++ b/rsh.c
@@ -68,7 +68,7 @@ int setup_connection(int *fd_in, int *fd
if (!path) {
return error("Bad URL: %s", url);
}
- /* $GIT_RSH <host> "env GIR_DIR=<path> <remote_prog> <args...>" */
+ /* $GIT_RSH <host> "env GIT_DIR=<path> <remote_prog> <args...>" */
sizen = COMMAND_SIZE;
posn = command;
of = 0;
^ permalink raw reply
* Re: cg-mv
From: Horst von Brand @ 2005-10-12 22:32 UTC (permalink / raw)
To: Petr Baudis; +Cc: Zack Brown, Git Mailing List
In-Reply-To: <20051012100757.GM30889@pasky.or.cz>
Petr Baudis <pasky@suse.cz> wrote:
> Dear diary, on Fri, Oct 07, 2005 at 04:33:33PM CEST, I got a letter
> where Zack Brown <zbrown@tumblerings.org> told me that...
> > IIRC, file renaming is something we only care about at read time, we
> > don't actually need to track it while making the change, because git
> > allows us to track data from file to file without having to tell it
> > that the data is moving.
> > So, just to keep certain people happy, why not have the cg-mv command
> > defined to something like this:
What about git-rename(1)? For completeness, just wrap it into cg-mv.
--
Dr. Horst H. von Brand User #22616 counter.li.org
Departamento de Informatica Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria +56 32 654239
Casilla 110-V, Valparaiso, Chile Fax: +56 32 797513
^ permalink raw reply
* Re: Usage of isspace and friends
From: H. Peter Anvin @ 2005-10-13 13:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Morten Welinder, git
In-Reply-To: <7vachd6hdx.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
>
> Huh? isspace is "int isspace(int)". Presumably standard
> integral promotion rules applies here whether char is signed or
> unsigned, doesn't it?
>
> The snippet you quoted is from apply.c, and I would say what is
> more problematic is that we do not force C locale while parsing
> the diff (see another thread -- we would want to process diffs
> as byte streams).
>
The problem is that isspace() is defined to operate on an integer which
can be an unsigned char value promoted to int or EOF (-1).
-hpa
^ permalink raw reply
* cg-prev and cg-next
From: Zack Brown @ 2005-10-13 14:38 UTC (permalink / raw)
To: Git Mailing List
Hi folks,
Sometimes I just want to surf through a project's history, getting a sense of
where I was over time. So I wrote these short scripts to let me do that easily:
cg-prev:
cg-seek `cg-log | grep "^commit " | head -n 2 | tail -n 1 | cut -d ' ' -f 2`
cg-next:
CURRENT=`cg-log | grep "^commit " | head -n 1`
cg-seek > /dev/null
cg-seek `cg-log | grep "^commit " | grep -B 1 "^$CURRENT" | head -n 1 | cut -d ' ' -f 2`
Even better, I guess would be to be able to do something like this:
$ cg-seek next
$ cg-seek prev
and have it do the right thing.
Be well,
Zack
--
Zack Brown
^ permalink raw reply
* Ignore
From: Andreas Ericsson @ 2005-10-13 14:49 UTC (permalink / raw)
To: git
Just getting an example mail to add some filters for this list.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: diff_tree_stdin
From: Morten Welinder @ 2005-10-13 15:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vu0fl51tg.fsf@assigned-by-dhcp.cox.net>
> I presume you are talking about "line", not this_header[], since
> you are talking about something whose length is already
> calculated.
I was talking about this_header. It gets slightly more than the length
of "line" which is whatever came in from stdin, subject to a 1000 char
limit.
Morten
^ permalink raw reply
* Re: Usage of isspace and friends
From: Linus Torvalds @ 2005-10-13 15:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Morten Welinder, git
In-Reply-To: <7vachd6hdx.fsf@assigned-by-dhcp.cox.net>
On Wed, 12 Oct 2005, Junio C Hamano wrote:
>
> Huh? isspace is "int isspace(int)". Presumably standard
> integral promotion rules applies here whether char is signed or
> unsigned, doesn't it?
No.
The input range for the "isxxxxx()" macros is the same as the range for
the "[f]getc[h]()" family: unsigned char + EOF (the latter usually being
-1).
So Morten is right - if you have a "char *", it should not be dereferenced
and used directly, although I think glibc does the right thing (and, in
fact, I can't understand why the standards haven't been updated to do the
right thing: it's _not_ that hard. In fact, it should be trivial apart
from the special case of "255" that looks undistinguishable from EOF in
signed char representation).
I'm almost goign to suggest that we do our own ctype.h, just to get the
sane semantics: we want locale-independence, _and_ we want the right
signed behaviour. Plus we only use a very small subset of ctype.h anyway
(isspace, isalpha, isdigit and isalnum).
Linus
^ permalink raw reply
* Re: Usage of isspace and friends
From: Junio C Hamano @ 2005-10-13 15:44 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: git
In-Reply-To: <434E60AB.8030607@zytor.com>
"H. Peter Anvin" <hpa@zytor.com> writes:
> The problem is that isspace() is defined to operate on an integer which
> can be an unsigned char value promoted to int or EOF (-1).
Ah, thanks.
^ permalink raw reply
* Re: Usage of isspace and friends
From: H. Peter Anvin @ 2005-10-13 15:45 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Morten Welinder, git
In-Reply-To: <Pine.LNX.4.64.0510130756550.15297@g5.osdl.org>
Linus Torvalds wrote:
>
> So Morten is right - if you have a "char *", it should not be dereferenced
> and used directly, although I think glibc does the right thing (and, in
> fact, I can't understand why the standards haven't been updated to do the
> right thing: it's _not_ that hard. In fact, it should be trivial apart
> from the special case of "255" that looks undistinguishable from EOF in
> signed char representation).
>
Because of the special case of 255 which looks indistinguishable from
EOF, therefore making it required?
The original mistake, of course, was allowing EOF to be passed to the
various isxxx() macros.
-hpa
^ permalink raw reply
* Re: Usage of isspace and friends
From: Linus Torvalds @ 2005-10-13 15:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Morten Welinder, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0510130756550.15297@g5.osdl.org>
On Thu, 13 Oct 2005, Linus Torvalds wrote:
>
> I'm almost goign to suggest that we do our own ctype.h, just to get the
> sane semantics: we want locale-independence, _and_ we want the right
> signed behaviour. Plus we only use a very small subset of ctype.h anyway
> (isspace, isalpha, isdigit and isalnum).
Maybe something like this.
No, I'm not 100% sure we need it. But hey, it's probably less complicated
than trying to de-localize various different targets.
Is there anything else that is locale-dependent that we use in the C
toolchain?
Linus
---
diff-tree f6fea67a590196d81bc4c6a6be1a16dd8bf2815d (from d06b689a933f6d2130f8afdf1ac0ddb83eeb59ab)
Author: Linus Torvalds <torvalds@osdl.org>
Date: Thu Oct 13 08:36:35 2005 -0700
Add locale-independent (and stupid) ctype.
It's also safe for signed chars.
diff --git a/Makefile b/Makefile
index 5e7d055..31e62d4 100644
--- a/Makefile
+++ b/Makefile
@@ -158,7 +158,8 @@ LIB_OBJS = \
object.o pack-check.o patch-delta.o path.o pkt-line.o \
quote.o read-cache.o refs.o run-command.o \
server-info.o setup.o sha1_file.o sha1_name.o strbuf.o \
- tag.o tree.o usage.o config.o environment.o $(DIFF_OBJS)
+ tag.o tree.o usage.o config.o environment.o ctype.o \
+ $(DIFF_OBJS)
LIBS = $(LIB_FILE)
LIBS += -lz
diff --git a/apply.c b/apply.c
index 155fbe8..f4d00f2 100644
--- a/apply.c
+++ b/apply.c
@@ -6,7 +6,6 @@
* This applies patches on top of some (arbitrary) version of the SCM.
*
*/
-#include <ctype.h>
#include <fnmatch.h>
#include "cache.h"
diff --git a/cache.h b/cache.h
index 1a7e047..a465952 100644
--- a/cache.h
+++ b/cache.h
@@ -386,4 +386,30 @@ extern int git_config_bool(const char *,
extern char git_default_email[MAX_GITNAME];
extern char git_default_name[MAX_GITNAME];
+/* Sane ctype - no locale, and works with signed chars */
+#undef isspace
+#undef isdigit
+#undef isalpha
+#undef isalnum
+#undef tolower
+#undef toupper
+extern unsigned char sane_ctype[256];
+#define GIT_SPACE 0x01
+#define GIT_DIGIT 0x02
+#define GIT_ALPHA 0x04
+#define sane_istest(x,mask) ((sane_ctype[(unsigned char)(x)] & (mask)) != 0)
+#define isspace(x) sane_istest(x,GIT_SPACE)
+#define isdigit(x) sane_istest(x,GIT_DIGIT)
+#define isalpha(x) sane_istest(x,GIT_ALPHA)
+#define isalnum(x) sane_istest(x,GIT_ALPHA | GIT_DIGIT)
+#define tolower(x) sane_case((unsigned char)(x), 0x20)
+#define toupper(x) sane_case((unsigned char)(x), 0)
+
+static inline int sane_case(int x, int high)
+{
+ if (sane_istest(x, GIT_ALPHA))
+ x = (x & ~0x20) | high;
+ return x;
+}
+
#endif /* CACHE_H */
diff --git a/commit-tree.c b/commit-tree.c
index 030fb70..ea0fdd4 100644
--- a/commit-tree.c
+++ b/commit-tree.c
@@ -7,7 +7,6 @@
#include <pwd.h>
#include <time.h>
-#include <ctype.h>
#define BLOCKING (1ul << 14)
diff --git a/commit.c b/commit.c
index f735f98..8f40318 100644
--- a/commit.c
+++ b/commit.c
@@ -1,4 +1,3 @@
-#include <ctype.h>
#include "tag.h"
#include "commit.h"
#include "cache.h"
diff --git a/config.c b/config.c
index 9b7c6f2..519fecf 100644
--- a/config.c
+++ b/config.c
@@ -1,4 +1,3 @@
-#include <ctype.h>
#include "cache.h"
diff --git a/convert-objects.c b/convert-objects.c
index 9ad0c77..a892013 100644
--- a/convert-objects.c
+++ b/convert-objects.c
@@ -1,6 +1,5 @@
#define _XOPEN_SOURCE /* glibc2 needs this */
#include <time.h>
-#include <ctype.h>
#include "cache.h"
struct entry {
diff --git a/ctype.c b/ctype.c
new file mode 100644
index 0000000..56bdffa
--- /dev/null
+++ b/ctype.c
@@ -0,0 +1,23 @@
+/*
+ * Sane locale-independent, ASCII ctype.
+ *
+ * No surprises, and works with signed and unsigned chars.
+ */
+#include "cache.h"
+
+#define SS GIT_SPACE
+#define AA GIT_ALPHA
+#define DD GIT_DIGIT
+
+unsigned char sane_ctype[256] = {
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, SS, SS, 0, 0, SS, 0, 0, /* 0-15 */
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, /* 16-15 */
+ SS, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, /* 32-15 */
+ DD, DD, DD, DD, DD, DD, DD, DD, DD, DD, 0, 0, 0, 0, 0, 0, /* 48-15 */
+ 0, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, /* 64-15 */
+ AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, 0, 0, 0, 0, 0, /* 80-15 */
+ 0, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, /* 96-15 */
+ AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, AA, 0, 0, 0, 0, 0, /* 112-15 */
+ /* Nothing in the 128.. range */
+};
+
diff --git a/date.c b/date.c
index b21cadc..63f5a09 100644
--- a/date.c
+++ b/date.c
@@ -4,7 +4,6 @@
* Copyright (C) Linus Torvalds, 2005
*/
-#include <ctype.h>
#include <time.h>
#include "cache.h"
diff --git a/diff-tree.c b/diff-tree.c
index 2203fa5..8517220 100644
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -1,4 +1,3 @@
-#include <ctype.h>
#include "cache.h"
#include "diff.h"
#include "commit.h"
diff --git a/ident.c b/ident.c
index 7a9f567..1bfbc6f 100644
--- a/ident.c
+++ b/ident.c
@@ -9,7 +9,6 @@
#include <pwd.h>
#include <time.h>
-#include <ctype.h>
static char git_default_date[50];
diff --git a/mailsplit.c b/mailsplit.c
index 0f8100d..189f4ed 100644
--- a/mailsplit.c
+++ b/mailsplit.c
@@ -11,7 +11,6 @@
#include <sys/stat.h>
#include <string.h>
#include <stdio.h>
-#include <ctype.h>
#include <assert.h>
#include "cache.h"
diff --git a/pack-objects.c b/pack-objects.c
index 3d62278..83ac22b 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -1,4 +1,3 @@
-#include <ctype.h>
#include "cache.h"
#include "object.h"
#include "delta.h"
diff --git a/patch-id.c b/patch-id.c
index 960e7ce..edbc4aa 100644
--- a/patch-id.c
+++ b/patch-id.c
@@ -1,4 +1,3 @@
-#include <ctype.h>
#include "cache.h"
static void flush_current_id(int patchlen, unsigned char *id, SHA_CTX *c)
diff --git a/refs.c b/refs.c
index 5a8cbd4..42240d2 100644
--- a/refs.c
+++ b/refs.c
@@ -2,7 +2,6 @@
#include "cache.h"
#include <errno.h>
-#include <ctype.h>
/* We allow "recursive" symbolic refs. Only within reason, though */
#define MAXDEPTH 5
diff --git a/update-ref.c b/update-ref.c
index 4a1704c..65dc3d6 100644
--- a/update-ref.c
+++ b/update-ref.c
@@ -1,6 +1,5 @@
#include "cache.h"
#include "refs.h"
-#include <ctype.h>
static const char git_update_ref_usage[] = "git-update-ref <refname> <value> [<oldval>]";
^ permalink raw reply related
* Re: Usage of isspace and friends
From: Linus Torvalds @ 2005-10-13 15:56 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: Junio C Hamano, Morten Welinder, git
In-Reply-To: <434E8117.3090102@zytor.com>
On Thu, 13 Oct 2005, H. Peter Anvin wrote:
>
> Because of the special case of 255 which looks indistinguishable from EOF,
> therefore making it required?
Yeah, and I agree, that was a mistake. It could have been fixed by making
EOF be MIN_INT (or any other value outside the range of either "unsigned
char" or "signed char" - preferably still negative), but there are
probably programs that depend on it being -1.
The stupid thing I just posted doesn't care. It happens to return 0 for
EOF for all cases, but that's a side effect of (a) not doing locales (so
255 is never printable or alpha) and (b) not even implementing iscntrl().
In general, the rule for ctype and EOF _should_ have been that it's part
of an acceptable input range, but that the return value is undefined ;)
(Which would allow you to test EOF later, and not worry about any faults).
Linus
^ permalink raw reply
* [PATCH] git-http-fetch: Remove size limit for objects/info/{packs,alternates}
From: Sergey Vlasov @ 2005-10-13 16:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
git-http-fetch received objects/info/packs into a fixed-size buffer
and started to fail when this file became larger than the buffer.
Change it to grow the buffer dynamically, and do the same thing for
objects/info/alternates. Also add missing free() calls for these
buffers.
Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>
---
The problem currently happens with the linux-2.4 repository:
http://www.kernel.org/pub/scm/linux/kernel/git/marcelo/linux-2.4.git/
For some reason, objects/info/packs in that repository has grown to 662K.
http-fetch.c | 43 ++++++++++++++++++++++++++++++++++++-------
1 files changed, 36 insertions(+), 7 deletions(-)
applies-to: 1970e5869a42fd4095917d861654dc84d60f02b7
341e03b5a2f197b1e60c3d38c0803d552dc6cd4d
diff --git a/http-fetch.c b/http-fetch.c
index 0aba891..c6daf6a 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -110,6 +110,22 @@ static size_t fwrite_buffer(void *ptr, s
return size;
}
+static size_t fwrite_buffer_dynamic(const void *ptr, size_t eltsize,
+ size_t nmemb, struct buffer *buffer)
+{
+ size_t size = eltsize * nmemb;
+ if (size > buffer->size - buffer->posn) {
+ buffer->size = buffer->size * 3 / 2;
+ if (buffer->size < buffer->posn + size)
+ buffer->size = buffer->posn + size;
+ buffer->buffer = xrealloc(buffer->buffer, buffer->size);
+ }
+ memcpy(buffer->buffer + buffer->posn, ptr, size);
+ buffer->posn += size;
+ data_received++;
+ return size;
+}
+
static size_t fwrite_sha1_file(void *ptr, size_t eltsize, size_t nmemb,
void *data)
{
@@ -618,11 +634,12 @@ static int fetch_alternates(char *base)
int i = 0;
int http_specific = 1;
struct alt_base *tail = alt;
+ static const char null_byte = '\0';
struct active_request_slot *slot;
data = xmalloc(4096);
- buffer.size = 4095;
+ buffer.size = 4096;
buffer.posn = 0;
buffer.buffer = data;
@@ -634,7 +651,8 @@ static int fetch_alternates(char *base)
slot = get_active_slot();
curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
- curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
+ curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,
+ fwrite_buffer_dynamic);
curl_easy_setopt(slot->curl, CURLOPT_URL, url);
if (start_active_slot(slot)) {
run_active_slot(slot);
@@ -646,20 +664,24 @@ static int fetch_alternates(char *base)
slot = get_active_slot();
curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,
- fwrite_buffer);
+ fwrite_buffer_dynamic);
curl_easy_setopt(slot->curl, CURLOPT_URL, url);
if (start_active_slot(slot)) {
run_active_slot(slot);
if (slot->curl_result != CURLE_OK) {
+ free(buffer.buffer);
return 0;
}
}
}
} else {
+ free(buffer.buffer);
return 0;
}
- data[buffer.posn] = '\0';
+ fwrite_buffer_dynamic(&null_byte, 1, 1, &buffer);
+ buffer.posn--;
+ data = buffer.buffer;
while (i < buffer.posn) {
int posn = i;
@@ -718,7 +740,8 @@ static int fetch_alternates(char *base)
}
i = posn + 1;
}
-
+
+ free(buffer.buffer);
return ret;
}
@@ -748,17 +771,22 @@ static int fetch_indices(struct alt_base
slot = get_active_slot();
curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
- curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
+ curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION,
+ fwrite_buffer_dynamic);
curl_easy_setopt(slot->curl, CURLOPT_URL, url);
curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);
if (start_active_slot(slot)) {
run_active_slot(slot);
- if (slot->curl_result != CURLE_OK)
+ if (slot->curl_result != CURLE_OK) {
+ free(buffer.buffer);
return error("%s", curl_errorstr);
+ }
} else {
+ free(buffer.buffer);
return error("Unable to start request");
}
+ data = buffer.buffer;
while (i < buffer.posn) {
switch (data[i]) {
case 'P':
@@ -778,6 +806,7 @@ static int fetch_indices(struct alt_base
i++;
}
+ free(buffer.buffer);
repo->got_indices = 1;
return 0;
}
---
0.99.8.GIT
^ permalink raw reply related
* Re: Usage of isspace and friends
From: H. Peter Anvin @ 2005-10-13 16:07 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Morten Welinder, git
In-Reply-To: <Pine.LNX.4.64.0510130847190.15297@g5.osdl.org>
Linus Torvalds wrote:
>
> Yeah, and I agree, that was a mistake. It could have been fixed by making
> EOF be MIN_INT (or any other value outside the range of either "unsigned
> char" or "signed char" - preferably still negative), but there are
> probably programs that depend on it being -1.
>
> The stupid thing I just posted doesn't care. It happens to return 0 for
> EOF for all cases, but that's a side effect of (a) not doing locales (so
> 255 is never printable or alpha) and (b) not even implementing iscntrl().
>
Given that for isprint() et al are useless for non-ASCII UTF-8 anyway,
might as well (they are not defined to be able to take wide character
values.)
-hpa
^ permalink raw reply
* [PATCH] Revert "Also use 'track_object_refs = 0' in update-server-info."
From: Junio C Hamano @ 2005-10-13 17:20 UTC (permalink / raw)
To: git; +Cc: Sergey Vlasov
In-Reply-To: <20051013161010.GC12092@master.mivlgu.local>
Sergey spotted a grave bug, already in "master", which shows my
lack of testing. I just pushed out his fix, which is to revert
d119e3de13ea1493107bd57381d0ce9c9dd90976. It should propagate
to the mirrors soon.
If you used git-update-server-info with the bug on your
repository, you will find a huge objects/info/packs file.
Usually it should list just a handful edge tags and commits, but
with the bug it practically lists everything.
After rebuilding the fixed git-update-server-info, please run
'git-update-server-info -f' there to fix this problem.
Sorry for the stupid bug, and many thanks to Sergey.
------------
This reverts d119e3de13ea1493107bd57381d0ce9c9dd90976 commit.
Object references are used in server-info.c:find_pack_info_one() to
find out which objects in the pack are heads, therefore tracking of
references cannot be disabled.
Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
Looks like this bug has caused objects/info/packs in the linux-2.4
repository to grow huge and hit the limit, which I then needed to remove
in the previous patch.
update-server-info.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
applies-to: 1970e5869a42fd4095917d861654dc84d60f02b7
985738719bfb5284911871f15180a2aee6512d53
diff --git a/update-server-info.c b/update-server-info.c
index b708563..e824f62 100644
--- a/update-server-info.c
+++ b/update-server-info.c
@@ -1,5 +1,4 @@
#include "cache.h"
-#include "object.h"
static const char update_server_info_usage[] =
"git-update-server-info [--force]";
@@ -8,8 +7,6 @@ int main(int ac, char **av)
{
int i;
int force = 0;
- track_object_refs = 0;
-
for (i = 1; i < ac; i++) {
if (av[i][0] == '-') {
if (!strcmp("--force", av[i]) ||
---
0.99.8.GIT
^ permalink raw reply related
* [PATCH] Sparse fixes for http-fetch
From: Peter Hagervall @ 2005-10-13 17:42 UTC (permalink / raw)
To: junkio; +Cc: git
This patch cleans out all sparse warnings from http-fetch.c
I'm a bit uncomfortable with adding extra #ifdefs to avoid either
'mixing declaration with code' or 'unused variable' warnings, but I
figured that since those functions are already littered with #ifdefs I
might just get away with it. Comments?
---
* ANSI:fy a few function definitions
* Make needlessly global functions static
* Move variable declarations to beginning of enclosing block
Signed-off-by: Peter Hagervall <hager@cs.umu.se>
---
diff --git a/http-fetch.c b/http-fetch.c
index 0aba891..f2d0e0a 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -143,7 +143,7 @@ void process_curl_messages();
void process_request_queue();
#endif
-struct active_request_slot *get_active_slot()
+static struct active_request_slot *get_active_slot(void)
{
struct active_request_slot *slot = active_queue_head;
struct active_request_slot *newslot;
@@ -192,7 +192,7 @@ struct active_request_slot *get_active_s
return slot;
}
-int start_active_slot(struct active_request_slot *slot)
+static int start_active_slot(struct active_request_slot *slot)
{
#ifdef USE_CURL_MULTI
CURLMcode curlm_result = curl_multi_add_handle(curlm, slot->curl);
@@ -207,7 +207,7 @@ int start_active_slot(struct active_requ
return 1;
}
-void run_active_slot(struct active_request_slot *slot)
+static void run_active_slot(struct active_request_slot *slot)
{
#ifdef USE_CURL_MULTI
int num_transfers;
@@ -255,7 +255,7 @@ void run_active_slot(struct active_reque
#endif
}
-void start_request(struct transfer_request *request)
+static void start_request(struct transfer_request *request)
{
char *hex = sha1_to_hex(request->sha1);
char prevfile[PATH_MAX];
@@ -381,7 +381,7 @@ void start_request(struct transfer_reque
request->state = ACTIVE;
}
-void finish_request(struct transfer_request *request)
+static void finish_request(struct transfer_request *request)
{
fchmod(request->local, 0444);
close(request->local);
@@ -409,7 +409,7 @@ void finish_request(struct transfer_requ
pull_say("got %s\n", sha1_to_hex(request->sha1));
}
-void release_request(struct transfer_request *request)
+static void release_request(struct transfer_request *request)
{
struct transfer_request *entry = request_queue_head;
@@ -427,7 +427,7 @@ void release_request(struct transfer_req
}
#ifdef USE_CURL_MULTI
-void process_curl_messages()
+void process_curl_messages(void)
{
int num_messages;
struct active_request_slot *slot;
@@ -479,7 +479,7 @@ void process_curl_messages()
}
}
-void process_request_queue()
+void process_request_queue(void)
{
struct transfer_request *request = request_queue_head;
int num_transfers;
@@ -875,6 +875,9 @@ static int fetch_object(struct alt_base
char *hex = sha1_to_hex(sha1);
int ret;
struct transfer_request *request = request_queue_head;
+#ifdef USE_CURL_MULTI
+ int num_transfers;
+#endif
while (request != NULL && memcmp(request->sha1, sha1, 20))
request = request->next;
@@ -887,7 +890,6 @@ static int fetch_object(struct alt_base
}
#ifdef USE_CURL_MULTI
- int num_transfers;
while (request->state == WAITING) {
curl_multi_perform(curlm, &num_transfers);
if (num_transfers < active_requests) {
@@ -1052,6 +1054,9 @@ int main(int argc, char **argv)
char *url;
int arg = 1;
struct active_request_slot *slot;
+#ifdef USE_CURL_MULTI
+ char *http_max_requests;
+#endif
while (arg < argc && argv[arg][0] == '-') {
if (argv[arg][1] == 't') {
@@ -1082,7 +1087,7 @@ int main(int argc, char **argv)
curl_global_init(CURL_GLOBAL_ALL);
#ifdef USE_CURL_MULTI
- char *http_max_requests = getenv("GIT_HTTP_MAX_REQUESTS");
+ http_max_requests = getenv("GIT_HTTP_MAX_REQUESTS");
if (http_max_requests != NULL)
max_requests = atoi(http_max_requests);
if (max_requests < 1)
^ permalink raw reply related
* Cogito RFE: cg-commit -q
From: H. Peter Anvin @ 2005-10-13 17:45 UTC (permalink / raw)
To: Git Mailing List, Petr Baudis
I would find it very useful if cg-commit had a "-q" option, meaning
"silently skip this commit if there is nothing to commit." There are
some automatic release scripts that I have which enforces consistency
before release, but if the repository is already correctly set up for
release, there is nothing to do.
This is the opposite of -f, which would create a commit object pointing
to the same tree.
-hpa
^ 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