[PATCH v2] compat: Fix read() of 2GB and more on Mac OS X

[Steffen Prohaska]

Previously, filtering 2GB or more through an external filter (see test)
failed on Mac OS X 10.8.4 (12E55) for a 64-bit executable with:

error: read from external filter cat failed
error: cannot feed the input to external filter cat
error: cat died of signal 13
error: external filter cat failed 141
error: external filter cat failed

The reason was that read() immediately returns with EINVAL if nbyte =
2GB.  According to POSIX [1], if the value of nbyte passed to read() is
greater than SSIZE_MAX, the result is implementation-defined.  The write
function has the same restriction [2].  Since OS X still supports
running 32-bit executables, the 32-bit limit (SSIZE_MAX = INT_MAX
= 2GB - 1) seems to be also imposed on 64-bit executables under certain
conditions.  For write, the problem has been addressed in a earlier
commit [6c642a].

The problem for read() is addressed in a similar way by introducing
a wrapper function in compat that always reads less than 2GB.
Unfortunately, '#undef read' is needed at a few places to avoid
expanding the compat macro in constructs like 'vtbl-read(...)'.

Note that 'git add' exits with 0 even if it prints filtering errors to
stderr.  The test, therefore, checks stderr.  'git add' should probably
be changed (sometime in another commit) to exit with nonzero if
filtering fails.  The test could then be changed to use test_must_fail.

Thanks to the following people for their suggestions:

Johannes Sixt j...@kdbg.org
John Keeping j...@keeping.me.uk
Jonathan Nieder jrnie...@gmail.com
Kyle J. McKay mack...@gmail.com
Torsten Bögershausen tbo...@web.de

[1] http://pubs.opengroup.org/onlinepubs/009695399/functions/read.html
[2] http://pubs.opengroup.org/onlinepubs/009695399/functions/write.html

[6c642a] 6c642a878688adf46b226903858b53e2d31ac5c3
compate/clipped-write.c: large write(2) fails on Mac OS X/XNU

Signed-off-by: Steffen Prohaska proha...@zib.de
---
 Makefile  |  8 
 builtin/var.c |  1 +
 config.mak.uname  |  1 +
 git-compat-util.h |  5 +
 streaming.c   |  1 +
 t/t0021-conversion.sh | 14 ++
 6 files changed, 30 insertions(+)

diff --git a/Makefile b/Makefile
index 3588ca1..0f69e24 100644
--- a/Makefile
+++ b/Makefile
@@ -69,6 +69,9 @@ all::
 # Define NO_MSGFMT_EXTENDED_OPTIONS if your implementation of msgfmt
 # doesn't support GNU extensions like --check and --statistics
 #
+# Define NEEDS_CLIPPED_READ if your read(2) cannot read more than
+# INT_MAX bytes at once (e.g. MacOS X).
+#
 # Define NEEDS_CLIPPED_WRITE if your write(2) cannot write more than
 # INT_MAX bytes at once (e.g. MacOS X).
 #
@@ -1493,6 +1496,11 @@ ifndef NO_MSGFMT_EXTENDED_OPTIONS
MSGFMT += --check --statistics
 endif
 
+ifdef NEEDS_CLIPPED_READ
+   BASIC_CFLAGS += -DNEEDS_CLIPPED_READ
+   COMPAT_OBJS += compat/clipped-read.o
+endif
+
 ifdef NEEDS_CLIPPED_WRITE
BASIC_CFLAGS += -DNEEDS_CLIPPED_WRITE
COMPAT_OBJS += compat/clipped-write.o
diff --git a/builtin/var.c b/builtin/var.c
index aedbb53..e59f5ba 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -38,6 +38,7 @@ static struct git_var git_vars[] = {
{ , NULL },
 };
 
+#undef read
 static void list_vars(void)
 {
struct git_var *ptr;
diff --git a/config.mak.uname b/config.mak.uname
index b27f51d..5c10726 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -95,6 +95,7 @@ ifeq ($(uname_S),Darwin)
NO_MEMMEM = YesPlease
USE_ST_TIMESPEC = YesPlease
HAVE_DEV_TTY = YesPlease
+   NEEDS_CLIPPED_READ = YesPlease
NEEDS_CLIPPED_WRITE = YesPlease
COMPAT_OBJS += compat/precompose_utf8.o
BASIC_CFLAGS += -DPRECOMPOSE_UNICODE
diff --git a/git-compat-util.h b/git-compat-util.h
index 115cb1d..a227127 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -185,6 +185,11 @@ typedef unsigned long uintptr_t;
 #define probe_utf8_pathname_composition(a,b)
 #endif
 
+#ifdef NEEDS_CLIPPED_READ
+ssize_t clipped_read(int fd, void *buf, size_t nbyte);
+#define read(x,y,z) clipped_read((x),(y),(z))
+#endif
+
 #ifdef NEEDS_CLIPPED_WRITE
 ssize_t clipped_write(int fildes, const void *buf, size_t nbyte);
 #define write(x,y,z) clipped_write((x),(y),(z))
diff --git a/streaming.c b/streaming.c
index debe904..c1fe34a 100644
--- a/streaming.c
+++ b/streaming.c
@@ -99,6 +99,7 @@ int close_istream(struct git_istream *st)
return r;
 }
 
+#undef read
 ssize_t read_istream(struct git_istream *st, void *buf, size_t sz)
 {
return st-vtbl-read(st, buf, sz);
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index e50f0f7..b92e6cb 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -190,4 +190,18 @@ test_expect_success 'required filter clean failure' '
test_must_fail git add test.fc
 '
 
+test -n $GIT_TEST_LONG  test_set_prereq EXPENSIVE
+
+test_expect_success EXPENSIVE 'filter large file' '
+   git config filter.largefile.smudge cat 
+   git config 

Re: [PATCH] submodule: prevent warning in summary output

[Chris Packham]

Hi Brian,
On 19/08/13 05:31, brian m. carlson wrote:
 When git submodule summary is run and there is a deleted submodule, there is 
 an
 warning from git rev-parse:
 
   fatal: Not a git repository: '.vim/pathogen/.git'
 
 Silence this warning, since it is fully expected that a deleted submodule will
 not be a git repository.
 
 Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
 ---
  git-submodule.sh | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/git-submodule.sh b/git-submodule.sh
 index 2979197..66ee621 100755
 --- a/git-submodule.sh
 +++ b/git-submodule.sh
 @@ -1071,7 +1071,7 @@ cmd_summary() {
   missing_dst=
  
   test $mod_src = 16 
 - ! GIT_DIR=$name/.git git-rev-parse -q --verify $sha1_src^0 
 /dev/null 
 + ! GIT_DIR=$name/.git git-rev-parse -q --verify $sha1_src^0 
 /dev/null 21 
   missing_src=t
  
   test $mod_dst = 16 
 

I wonder if there are other useful errors this will silence
unintentionally. Perhaps this would be better (untested)

 test $mod_src = 16 
 test -e $name/.git 
 ! GIT_DIR=$name/.git git-rev-parse -q --verify $sha1_src^0 /dev/null 
  missing_src=t

Having said that there are precedents for both in git-submodule.sh. If
there aren't any errors worth catching here then your way is probably
cleaner than mine.

- C

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Notes and submodules

[Francis Moreau]

Hello,

Is it possible to keep submodules notes in the super project  ?

Thanks
-- 
Francis
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] compat: Fix read() of 2GB and more on Mac OS X

[Johannes Sixt]

Am 19.08.2013 08:38, schrieb Steffen Prohaska:

+test_expect_success EXPENSIVE 'filter large file' '
+   git config filter.largefile.smudge cat 
+   git config filter.largefile.clean cat 
+   for i in $(test_seq 1 2048); do printf %1048576d 1; done 2GB 


Shouldn't you count to 2049 to get a file that is over 2GB?


+   echo 2GB filter=largefile .gitattributes 
+   git add 2GB 2err 
+   ! test -s err 
+   rm -f 2GB 
+   git checkout -- 2GB 2err 
+   ! test -s err
+'


-- Hannes

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] compat: Fix read() of 2GB and more on Mac OS X

[Steffen Prohaska]

On Aug 19, 2013, at 9:54 AM, John Keeping j...@keeping.me.uk wrote:

 You've created compat/clipped-read.c, but...
 
 Makefile  |  8 
 builtin/var.c |  1 +
 config.mak.uname  |  1 +
 git-compat-util.h |  5 +
 streaming.c   |  1 +
 t/t0021-conversion.sh | 14 ++
 6 files changed, 30 insertions(+)
 
 ... it's not included here.  Did you forget to git add?

Indeed.  How embarrassing.  Thanks for spotting this.  I'll send v3 in a minute.

Stefffen--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] compat: Fix read() of 2GB and more on Mac OS X

[Steffen Prohaska]

Previously, filtering 2GB or more through an external filter (see test)
failed on Mac OS X 10.8.4 (12E55) for a 64-bit executable with:

error: read from external filter cat failed
error: cannot feed the input to external filter cat
error: cat died of signal 13
error: external filter cat failed 141
error: external filter cat failed

The reason was that read() immediately returns with EINVAL if nbyte =
2GB.  According to POSIX [1], if the value of nbyte passed to read() is
greater than SSIZE_MAX, the result is implementation-defined.  The write
function has the same restriction [2].  Since OS X still supports
running 32-bit executables, the 32-bit limit (SSIZE_MAX = INT_MAX
= 2GB - 1) seems to be also imposed on 64-bit executables under certain
conditions.  For write, the problem has been addressed in a earlier
commit [6c642a].

The problem for read() is addressed in a similar way by introducing
a wrapper function in compat that always reads less than 2GB.
Unfortunately, '#undef read' is needed at a few places to avoid
expanding the compat macro in constructs like 'vtbl-read(...)'.

Note that 'git add' exits with 0 even if it prints filtering errors to
stderr.  The test, therefore, checks stderr.  'git add' should probably
be changed (sometime in another commit) to exit with nonzero if
filtering fails.  The test could then be changed to use test_must_fail.

Thanks to the following people for their suggestions:

Johannes Sixt j...@kdbg.org
John Keeping j...@keeping.me.uk
Jonathan Nieder jrnie...@gmail.com
Kyle J. McKay mack...@gmail.com
Torsten Bögershausen tbo...@web.de

[1] http://pubs.opengroup.org/onlinepubs/009695399/functions/read.html
[2] http://pubs.opengroup.org/onlinepubs/009695399/functions/write.html

[6c642a] 6c642a878688adf46b226903858b53e2d31ac5c3
compate/clipped-write.c: large write(2) fails on Mac OS X/XNU

Signed-off-by: Steffen Prohaska proha...@zib.de
---
 Makefile  |  8 
 builtin/var.c |  1 +
 compat/clipped-read.c | 13 +
 config.mak.uname  |  1 +
 git-compat-util.h |  5 +
 streaming.c   |  1 +
 t/t0021-conversion.sh | 14 ++
 7 files changed, 43 insertions(+)
 create mode 100644 compat/clipped-read.c

diff --git a/Makefile b/Makefile
index 3588ca1..0f69e24 100644
--- a/Makefile
+++ b/Makefile
@@ -69,6 +69,9 @@ all::
 # Define NO_MSGFMT_EXTENDED_OPTIONS if your implementation of msgfmt
 # doesn't support GNU extensions like --check and --statistics
 #
+# Define NEEDS_CLIPPED_READ if your read(2) cannot read more than
+# INT_MAX bytes at once (e.g. MacOS X).
+#
 # Define NEEDS_CLIPPED_WRITE if your write(2) cannot write more than
 # INT_MAX bytes at once (e.g. MacOS X).
 #
@@ -1493,6 +1496,11 @@ ifndef NO_MSGFMT_EXTENDED_OPTIONS
MSGFMT += --check --statistics
 endif
 
+ifdef NEEDS_CLIPPED_READ
+   BASIC_CFLAGS += -DNEEDS_CLIPPED_READ
+   COMPAT_OBJS += compat/clipped-read.o
+endif
+
 ifdef NEEDS_CLIPPED_WRITE
BASIC_CFLAGS += -DNEEDS_CLIPPED_WRITE
COMPAT_OBJS += compat/clipped-write.o
diff --git a/builtin/var.c b/builtin/var.c
index aedbb53..e59f5ba 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -38,6 +38,7 @@ static struct git_var git_vars[] = {
{ , NULL },
 };
 
+#undef read
 static void list_vars(void)
 {
struct git_var *ptr;
diff --git a/compat/clipped-read.c b/compat/clipped-read.c
new file mode 100644
index 000..6962f67
--- /dev/null
+++ b/compat/clipped-read.c
@@ -0,0 +1,13 @@
+#include ../git-compat-util.h
+#undef read
+
+/*
+ * Version of read that will write at most INT_MAX bytes.
+ * Workaround a xnu bug on Mac OS X
+ */
+ssize_t clipped_read(int fd, void *buf, size_t nbyte)
+{
+   if (nbyte  INT_MAX)
+   nbyte = INT_MAX;
+   return read(fd, buf, nbyte);
+}
diff --git a/config.mak.uname b/config.mak.uname
index b27f51d..5c10726 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -95,6 +95,7 @@ ifeq ($(uname_S),Darwin)
NO_MEMMEM = YesPlease
USE_ST_TIMESPEC = YesPlease
HAVE_DEV_TTY = YesPlease
+   NEEDS_CLIPPED_READ = YesPlease
NEEDS_CLIPPED_WRITE = YesPlease
COMPAT_OBJS += compat/precompose_utf8.o
BASIC_CFLAGS += -DPRECOMPOSE_UNICODE
diff --git a/git-compat-util.h b/git-compat-util.h
index 115cb1d..a227127 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -185,6 +185,11 @@ typedef unsigned long uintptr_t;
 #define probe_utf8_pathname_composition(a,b)
 #endif
 
+#ifdef NEEDS_CLIPPED_READ
+ssize_t clipped_read(int fd, void *buf, size_t nbyte);
+#define read(x,y,z) clipped_read((x),(y),(z))
+#endif
+
 #ifdef NEEDS_CLIPPED_WRITE
 ssize_t clipped_write(int fildes, const void *buf, size_t nbyte);
 #define write(x,y,z) clipped_write((x),(y),(z))
diff --git a/streaming.c b/streaming.c
index debe904..c1fe34a 100644
--- a/streaming.c
+++ b/streaming.c
@@ -99,6 +99,7 @@ int close_istream(struct git_istream *st)
return r;
 }
 

Re: [PATCH gitk 0/4] gitk support for git log -L

[Thomas Rast]

Paul Mackerras pau...@samba.org writes:

 Hi Thomas,

 On Wed, Jul 31, 2013 at 03:17:41PM +0200, Thomas Rast wrote:
 Jens Lehmann jens.lehm...@web.de writes:
 
  Am 29.07.2013 21:37, schrieb Thomas Rast:
  Thomas Rast tr...@inf.ethz.ch writes:
  
  Thomas Rast tr...@inf.ethz.ch writes:
 
  Now that git log -L has hit master, I figure it's time to discuss the
  corresponding change to gitk.
[...]

 One thing I worry about is having gitk storing in memory not just the
 history graph but also all the diffs (assuming I have understood
 correctly what you're doing).  Gitk's memory consumption is already
 pretty large.  However, I can't see an alternative at this point.

I don't think there is one.  log -L is pretty much an all or nothing
thing at this point.  I suppose if we really found that the diffs are
regularly too big to be manageable for gitk, we could invent a porcelain
mode where 'log -L' just prints the detected commits and corresponding
line ranges, and then have a new option to diff-tree to let it again
filter that range.

But note that ordinary 'git log -L' also buffers the entire set of diffs
within less.  The memory consumption of gitk to hold the same diffs in
memory should be only a small factor of what less uses in the same
scenario.  Furthermore, users will typically ask for a small region of
code (one function, or some such), so the diffs themselves are usually
quite small, nowhere near the size of the full commit diffs.

 Unfortunately it's turning out to be harder than I hoped.  gitk runs the
 arguments through git-rev-parse, which only knows that -n gets an
 unstuck argument.  Consequently, gitk accepts an unstuck -n but only
 stuck forms of -S and -G.

 Excuse my ignorance, but what do you mean by stuck vs. unstuck?

Whether the option value is a separate argument in argv, or directly
stuck to the option.

stuck:   gitk -L:foo:main.c
unstuck: gitk -L :foo:main.c

Existing gitk chokes on 'gitk -S foo', but works with 'git -Sfoo'.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] compat: Fix read() of 2GB and more on Mac OS X

[Stefan Beller]

On 08/19/2013 10:20 AM, Johannes Sixt wrote:
 Am 19.08.2013 08:38, schrieb Steffen Prohaska:
 +test_expect_success EXPENSIVE 'filter large file' '
 +git config filter.largefile.smudge cat 
 +git config filter.largefile.clean cat 
 +for i in $(test_seq 1 2048); do printf %1048576d 1; done 2GB 
 
 Shouldn't you count to 2049 to get a file that is over 2GB?

Would it be possible to offload the looping from shell to a real
program? So for example
truncate -s 2049M filename
should do the job. That would create a file reading all bytes as zeros  
being larger as 2G. If truncate is not available, what about dd?

Stefan



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v2] compat: Fix read() of 2GB and more on Mac OS X

[Johannes Sixt]

Am 19.08.2013 08:38, schrieb Steffen Prohaska:

Note that 'git add' exits with 0 even if it prints filtering errors to
stderr.  The test, therefore, checks stderr.  'git add' should probably
be changed (sometime in another commit) to exit with nonzero if
filtering fails.  The test could then be changed to use test_must_fail.


Thanks for this hint. I was not aware of this behavior.

Of course, we do *not* want to use test_must_fail because git add 
generally must not fail for files with more than 2GB. (Architectures with 
a 32bit size_t are a different matter, of course.)


-- Hannes

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] compat: Fix read() of 2GB and more on Mac OS X

[Steffen Prohaska]

On Aug 19, 2013, at 10:20 AM, Johannes Sixt j...@kdbg.org wrote:

 Am 19.08.2013 08:38, schrieb Steffen Prohaska:
 +test_expect_success EXPENSIVE 'filter large file' '
 +git config filter.largefile.smudge cat 
 +git config filter.largefile.clean cat 
 +for i in $(test_seq 1 2048); do printf %1048576d 1; done 2GB 
 
 Shouldn't you count to 2049 to get a file that is over 2GB?

No.  INT_MAX = 2GB - 1 works.  INT_MAX + 1 = 2GB fails.  It tests exactly at 
the boundary.

Steffen

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Does Git now have any C struct version history tracking mechanism?

[Thomas Rast]

Nazri Ramliy ayieh...@gmail.com writes:

 On Sun, Aug 18, 2013 at 6:33 PM, Zhan Jianyu nasa4...@gmail.com wrote:
   Such a requirement came into my mind when I am tracking a gloomy C
 struct , with lengthy list of elements which are either elaborated or
 opaque. So I use git blame to track it down and found that its
 original version is quite simple and intuitive. So I think I could
 just slice out every snapshot of this struct, reading every changelog
 , to get a better knowledge of what it is and why it should be like
 this.

 It seems quite helpful but the process is quite cumbersome. So I
 wonder if there is already some mechanism fulfilling my requirement in
 Git.  If it doesn't,  would it be worthy adding one ?

 It's already merged to git.git's master quite recently in
 ed73fe56428eecd2b635473f6a517a183c4713a3 (back in June).

 You'd invoke git log like this:

 $ git log -L :struct_or_function_name:filename.c

 and it will show you the commits and the specific hunks that affect
 the struct or function name.

 It's still a bit rough on the edges, for example, doing the following
 in git.git:

 $ git log -L :rev_cmdline_info:revision.h

 Shows three commits (a765499, ca92e59 and 281eee4) where the second
 one does not touch the struct at all (if you do git show ca92e59 you
 might gain an insight as to how -L works).

Hmm, IIUC that's actually not a bug or even a roughness; it's an
artifact of how the :pattern:file syntax is defined.  It takes the first
_funcname line_ matching 'pattern', up to (but excluding) the next
funcname line.

The default funcname rule (in the absence of any patterns) does not
match '#define...' on account of the '#'.  The default funcname pattern
for 'cpp' (if you manually configured your git.git repo to set this
attribute; it doesn't by default) never matches a leading '#´ either.
So it's no surprise that the tracked range extends a few more lines
after the struct.

Or is there an issue that I'm missing?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] compat: Fix read() of 2GB and more on Mac OS X

[Johannes Sixt]

Am 19.08.2013 10:25, schrieb Stefan Beller:

On 08/19/2013 10:20 AM, Johannes Sixt wrote:

Am 19.08.2013 08:38, schrieb Steffen Prohaska:

+test_expect_success EXPENSIVE 'filter large file' '
+git config filter.largefile.smudge cat 
+git config filter.largefile.clean cat 
+for i in $(test_seq 1 2048); do printf %1048576d 1; done 2GB 


Shouldn't you count to 2049 to get a file that is over 2GB?


Would it be possible to offload the looping from shell to a real
program? So for example
truncate -s 2049M filename
should do the job. That would create a file reading all bytes as zeros  
being larger as 2G. If truncate is not available, what about dd?


The point is exactly to avoid external dependencies. Our dd on Windows 
doesn't do the right thing with seek=2GB (it makes the file twice as large 
as expected).


-- Hannes

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply.

[Cham Tao Soon]

I have a project for you in the tune of One Hundred  Five Million EUR,Please
reply for specifics.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] compat: Fix read() of 2GB and more on Mac OS X

[Eric Sunshine]

On Mon, Aug 19, 2013 at 4:21 AM, Steffen Prohaska proha...@zib.de wrote:
 Previously, filtering 2GB or more through an external filter (see test)
 failed on Mac OS X 10.8.4 (12E55) for a 64-bit executable with:

 error: read from external filter cat failed
 error: cannot feed the input to external filter cat
 error: cat died of signal 13
 error: external filter cat failed 141
 error: external filter cat failed


 Signed-off-by: Steffen Prohaska proha...@zib.de
 ---
  Makefile  |  8 
  builtin/var.c |  1 +
  compat/clipped-read.c | 13 +
  config.mak.uname  |  1 +
  git-compat-util.h |  5 +
  streaming.c   |  1 +
  t/t0021-conversion.sh | 14 ++
  7 files changed, 43 insertions(+)
  create mode 100644 compat/clipped-read.c

 diff --git a/Makefile b/Makefile
 index 3588ca1..0f69e24 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -69,6 +69,9 @@ all::
  # Define NO_MSGFMT_EXTENDED_OPTIONS if your implementation of msgfmt
  # doesn't support GNU extensions like --check and --statistics
  #
 +# Define NEEDS_CLIPPED_READ if your read(2) cannot read more than
 +# INT_MAX bytes at once (e.g. MacOS X).
 +#
  # Define NEEDS_CLIPPED_WRITE if your write(2) cannot write more than
  # INT_MAX bytes at once (e.g. MacOS X).

Is it likely that we would see a platform requiring only one or the
other CLIPPED? Would it make sense to combine these into a single
NEEDS_CLIPPED_IO?

  #
 @@ -1493,6 +1496,11 @@ ifndef NO_MSGFMT_EXTENDED_OPTIONS
 MSGFMT += --check --statistics
  endif

 +ifdef NEEDS_CLIPPED_READ
 +   BASIC_CFLAGS += -DNEEDS_CLIPPED_READ
 +   COMPAT_OBJS += compat/clipped-read.o
 +endif
 +
  ifdef NEEDS_CLIPPED_WRITE
 BASIC_CFLAGS += -DNEEDS_CLIPPED_WRITE
 COMPAT_OBJS += compat/clipped-write.o
 diff --git a/builtin/var.c b/builtin/var.c
 index aedbb53..e59f5ba 100644
 --- a/builtin/var.c
 +++ b/builtin/var.c
 @@ -38,6 +38,7 @@ static struct git_var git_vars[] = {
 { , NULL },
  };
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Notes and submodules

[Johan Herland]

On Mon, Aug 19, 2013 at 10:13 AM, Francis Moreau francis.m...@gmail.com wrote:
 Hello,

 Is it possible to keep submodules notes in the super project  ?

Not easily. I guess it depends on what you want to use the notes for.
In order for notes to be generally useful (i.e. show up in logs,
surviving a notes prune, etc.) they really must reside in the same
repo as the annotated objects [1]. Now, if all your interaction with
notes happens through scripts that you control, then I guess it would
be possible to hack this in some sort of semi-workable way, but you
would still have to make sure never to run git notes prune in the
super project. I guess the real question here is: Why would you want
to do this? and is there maybe some other way your use case can be
accomodated?

...Johan

[1]: If you were to annotate objects in a submodule, but then store
the notes objects in the super project, it would be impossible for
git log in the submodule to find the notes objects, and your log
would show no notes. Similarly, a git log in the super project would
see a lot of notes objects pointing to non-existing objects (because
those objects live in the submodule), hence the notes objects would be
removed when running git notes prune in the super project.

-- 
Johan Herland, jo...@herland.net
www.herland.net
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] compat: Fix read() of 2GB and more on Mac OS X

[Torsten Bögershausen]

On 2013-08-19 08.38, Steffen Prohaska wrote:
[snip]

 diff --git a/builtin/var.c b/builtin/var.c
 index aedbb53..e59f5ba 100644
 --- a/builtin/var.c
 +++ b/builtin/var.c
 @@ -38,6 +38,7 @@ static struct git_var git_vars[] = {
   { , NULL },
  };
  
 +#undef read
This is techically right for this very version of the  code,
but not really future proof, if someone uses read() further down in the code
(in a later version)

I think the problem comes from further up:
--
struct git_var {
const char *name;
const char *(*read)(int);
};
-
could the read be replaced by readfn ?

===
 diff --git a/streaming.c b/streaming.c
 index debe904..c1fe34a 100644
 --- a/streaming.c
 +++ b/streaming.c
 @@ -99,6 +99,7 @@ int close_istream(struct git_istream *st)
   return r;
  }
  
 +#undef read
Same possible future problem as above.
When later someone uses read, the original (buggy) read() will be
used, and not the re-defined clipped_read() from git-compat-util.h

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4] compat: Fix read() of 2GB and more on Mac OS X

[Steffen Prohaska]

Previously, filtering 2GB or more through an external filter (see test)
failed on Mac OS X 10.8.4 (12E55) for a 64-bit executable with:

error: read from external filter cat failed
error: cannot feed the input to external filter cat
error: cat died of signal 13
error: external filter cat failed 141
error: external filter cat failed

The reason was that read() immediately returns with EINVAL if nbyte =
2GB.  According to POSIX [1], if the value of nbyte passed to read() is
greater than SSIZE_MAX, the result is implementation-defined.  The write
function has the same restriction [2].  Since OS X still supports
running 32-bit executables, the 32-bit limit (SSIZE_MAX = INT_MAX
= 2GB - 1) seems to be also imposed on 64-bit executables under certain
conditions.  For write, the problem has been addressed in a earlier
commit [6c642a].

The problem for read() is addressed in a similar way by introducing
a wrapper function in compat that always reads less than 2GB.  It is
very likely that the read() and write() wrappers are always used
together.  To avoid introducing another option, NEEDS_CLIPPED_WRITE is
changed to NEEDS_CLIPPED_IO and used to activate both wrappers.

To avoid expanding the read compat macro in constructs like
'vtbl-read(...)', 'read' is renamed to 'readfn' in two cases.  The
solution seems more robust than using '#undef read'.

Note that 'git add' exits with 0 even if it prints filtering errors to
stderr.  The test, therefore, checks stderr.  'git add' should probably
be changed (sometime in another commit) to exit with nonzero if
filtering fails.  The test could then be changed to use test_must_fail.

Thanks to the following people for their suggestions:

Johannes Sixt j...@kdbg.org
John Keeping j...@keeping.me.uk
Jonathan Nieder jrnie...@gmail.com
Kyle J. McKay mack...@gmail.com
Torsten Bögershausen tbo...@web.de
Eric Sunshine sunsh...@sunshineco.com

[1] http://pubs.opengroup.org/onlinepubs/009695399/functions/read.html
[2] http://pubs.opengroup.org/onlinepubs/009695399/functions/write.html

[6c642a] 6c642a878688adf46b226903858b53e2d31ac5c3
compate/clipped-write.c: large write(2) fails on Mac OS X/XNU

Signed-off-by: Steffen Prohaska proha...@zib.de
---
 Makefile | 10 +-
 builtin/var.c| 10 +-
 compat/{clipped-write.c = clipped-io.c} | 11 ++-
 config.mak.uname |  2 +-
 git-compat-util.h|  5 -
 streaming.c  |  4 ++--
 t/t0021-conversion.sh| 14 ++
 7 files changed, 41 insertions(+), 15 deletions(-)
 rename compat/{clipped-write.c = clipped-io.c} (53%)

diff --git a/Makefile b/Makefile
index 3588ca1..f54134d 100644
--- a/Makefile
+++ b/Makefile
@@ -69,8 +69,8 @@ all::
 # Define NO_MSGFMT_EXTENDED_OPTIONS if your implementation of msgfmt
 # doesn't support GNU extensions like --check and --statistics
 #
-# Define NEEDS_CLIPPED_WRITE if your write(2) cannot write more than
-# INT_MAX bytes at once (e.g. MacOS X).
+# Define NEEDS_CLIPPED_IO if your read(2) and/or write(2) cannot handle more
+# than INT_MAX bytes at once (e.g. Mac OS X).
 #
 # Define HAVE_PATHS_H if you have paths.h and want to use the default PATH
 # it specifies.
@@ -1493,9 +1493,9 @@ ifndef NO_MSGFMT_EXTENDED_OPTIONS
MSGFMT += --check --statistics
 endif
 
-ifdef NEEDS_CLIPPED_WRITE
-   BASIC_CFLAGS += -DNEEDS_CLIPPED_WRITE
-   COMPAT_OBJS += compat/clipped-write.o
+ifdef NEEDS_CLIPPED_IO
+   BASIC_CFLAGS += -DNEEDS_CLIPPED_IO
+   COMPAT_OBJS += compat/clipped-io.o
 endif
 
 ifneq (,$(XDL_FAST_HASH))
diff --git a/builtin/var.c b/builtin/var.c
index aedbb53..06f8459 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -28,7 +28,7 @@ static const char *pager(int flag)
 
 struct git_var {
const char *name;
-   const char *(*read)(int);
+   const char *(*readfn)(int);
 };
 static struct git_var git_vars[] = {
{ GIT_COMMITTER_IDENT, git_committer_info },
@@ -43,8 +43,8 @@ static void list_vars(void)
struct git_var *ptr;
const char *val;
 
-   for (ptr = git_vars; ptr-read; ptr++)
-   if ((val = ptr-read(0)))
+   for (ptr = git_vars; ptr-readfn; ptr++)
+   if ((val = ptr-readfn(0)))
printf(%s=%s\n, ptr-name, val);
 }
 
@@ -53,9 +53,9 @@ static const char *read_var(const char *var)
struct git_var *ptr;
const char *val;
val = NULL;
-   for (ptr = git_vars; ptr-read; ptr++) {
+   for (ptr = git_vars; ptr-readfn; ptr++) {
if (strcmp(var, ptr-name) == 0) {
-   val = ptr-read(IDENT_STRICT);
+   val = ptr-readfn(IDENT_STRICT);
break;
}
}
diff --git a/compat/clipped-write.c b/compat/clipped-io.c
similarity index 53%
rename from compat/clipped-write.c
rename 

Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X

[Linus Torvalds]

On Mon, Aug 19, 2013 at 8:41 AM, Steffen Prohaska proha...@zib.de wrote:

 The reason was that read() immediately returns with EINVAL if nbyte =
 2GB.  According to POSIX [1], if the value of nbyte passed to read() is
 greater than SSIZE_MAX, the result is implementation-defined.

Yeah, the OS X filesystem layer is an incredible piece of shit. Not
only doesn't it follow POSIX, it fails *badly*. Because OS X kernel
engineers apparently have the mental capacity of a retarded rodent on
crack.

Linux also refuses to actually read more than a maximum value in one
go (because quite frankly, doing more than 2GB at a time is just not
reasonable, especially in unkillable disk wait), but at least Linux
gives you the partial read, so that the usual read until you're
happy works (which you have to do anyway with sockets, pipes, NFS
intr mounts, etc etc). Returning EINVAL is a sign of a diseased mind.

I hate your patch for other reasons, though:

 The problem for read() is addressed in a similar way by introducing
 a wrapper function in compat that always reads less than 2GB.

Why do you do that? We already _have_ wrapper functions for read(),
namely xread().  Exactly because you basically have to, in order to
handle signals on interruptible filesystems (which aren't POSIX
either, but at least sanely so) or from other random sources. And to
handle the you can't do reads that big issue.

So why isn't the patch much more straightforward? Like the attached
totally untested one that just limits the read/write size to 8MB
(which is totally arbitrary, but small enough to not have any latency
issues even on slow disks, and big enough that any reasonable IO
subsystem will still get good throughput).

And by totally untested I mean that it actually passes the git test
suite, but since I didn't apply your patch nor do I have OS X
anywhere, I can't actually test that it fixes *your* problem. But it
should.


   Linus


patch.diff
Description: Binary data

Re: [PATCH] Git segmentation faults if submodule path is empty.

[Jharrod LaFon]

Updated the patch and the patch submission.

 -- 8 --

Git segmentation faults if submodule path is empty.

Git fails due to a segmentation fault if a submodule path is empty.
Here is an example .gitmodules that will cause a segmentation fault:
[submodule foo-module]
  path
  url = http://host/repo.git
$ git status
Segmentation fault (core dumped)

This occurs because in the function parse_submodule_config_option, the
variable 'value' is assumed to be null, and when passed as an argument
to xstrdup a segmentation fault occurs if it is indeed null.
This is the case when using the .gitmodules example above.

This patch addresses the issue by checking to make sure 'value' is not
null before using it as an argument to xstrdup.  For some configuration
options, such as fetchRecurseSubmodules, an empty value is valid.  If
the option being read is 'path', an empty value is not valid, and so
an error message is printed.

Signed-off-by: Jharrod LaFon jlafon at eyesopen.com
---
 submodule.c|6 ++
 t/t1307-null-submodule-path.sh |   14 ++
 2 files changed, 20 insertions(+)
 create mode 100755 t/t1307-null-submodule-path.sh

diff --git a/submodule.c b/submodule.c
index 1821a5b..1a2cf30 100644
--- a/submodule.c
+++ b/submodule.c
@@ -134,6 +134,9 @@ int parse_submodule_config_option(const char *var, const 
char *value)
return 0;
 
if (!strcmp(key, path)) {
+   if (!value)
+   return config_error_nonbool(var);
+
config = unsorted_string_list_lookup(config_name_for_path, 
value);
if (config)
free(config-util);
@@ -151,6 +154,9 @@ int parse_submodule_config_option(const char *var, const 
char *value)
} else if (!strcmp(key, ignore)) {
char *name_cstr;
 
+   if (!value)
+   return config_error_nonbool(var);
+
if (strcmp(value, untracked)  strcmp(value, dirty) 
strcmp(value, all)  strcmp(value, none)) {
warning(Invalid parameter \%s\ for config option 
\submodule.%s.ignore\, value, var);
diff --git a/t/t1307-null-submodule-path.sh b/t/t1307-null-submodule-path.sh
new file mode 100755
index 000..a4470a8
--- /dev/null
+++ b/t/t1307-null-submodule-path.sh
@@ -0,0 +1,14 @@
+#!/bin/sh
+
+test_description='test empty submodule path'
+. ./test-lib.sh
+
+setup() {
+echo '[submodule test] path'  .gitmodules
+}
+
+test_expect_success 'git status with empty submodule path should not seg 
fault' '
+setup 
+test_must_fail git status
+'
+test_done
-- 
1.7.9.5

 On Aug 16, 2013, at 2:52 PM, Jeff King p...@peff.net wrote:

 On Fri, Aug 16, 2013 at 10:59:35AM -0700, Jharrod LaFon wrote:
 
 Here is an updated patch with a test.
 
 Bits like this that should not be part of the commit message should
 either go after the --- lines near the diffstat, or should come before
 a scissors line, like:
 
  Here is my new patch.
 
  -- 8 --
  Git segmentation faults etc...
 
 That way git-am can do the right thing, and the maintainer does not have
 to fix up your patch by hand.
 
 diff --git a/submodule.c b/submodule.c
 index 1821a5b..1e4acfd 100644
 --- a/submodule.c
 +++ b/submodule.c
 @@ -134,6 +134,9 @@ int parse_submodule_config_option(const char *var, const 
 char *value)
  return 0;
 
  if (!strcmp(key, path)) {
 +if (!value)
 +return config_error_nonbool(var);
 
 Your indentation looks like two spaces here, which does not match the
 rest of the block. It should be one tab.
 
 @@ -151,6 +154,9 @@ int parse_submodule_config_option(const char *var, const 
 char *value)
  } else if (!strcmp(key, ignore)) {
  char *name_cstr;
 
 +if (!value)
 +return config_error_nonbool(var);
 +
 
 Ditto here.
 
 diff --git a/t/t1307-null-submodule-path.sh b/t/t1307-null-submodule-path.sh
 new file mode 100644
 index 000..eeda2cb
 --- /dev/null
 +++ b/t/t1307-null-submodule-path.sh
 @@ -0,0 +1,16 @@
 +#!/bin/sh
 +
 +test_description='test empty submodule path'
 +. ./test-lib.sh
 +
 +setup() {
 +(printf [submodule \test\]\n  
 +printf   path\n 
 +printf   url) .gitmodules
 +}
 
 You can use single-quotes to avoid having to backslash the embedded
 double-quotes. And I do not see any reason to use printf rather than the
 more readable echo, which can drop the \n.
 
 And is there any point in having the url field?  The presence of a
 valueless bool path should be enough, no? It may be easier to see what
 it is we are testing without the extraneous parameter.
 
 With those changes, you could even put it all on one line:
 
  echo '[submodule test] path' .gitmodules
 
 -Peff

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

Re: [PATCH v3] compat: Fix read() of 2GB and more on Mac OS X

[Junio C Hamano]

Eric Sunshine sunsh...@sunshineco.com writes:

 +# Define NEEDS_CLIPPED_READ if your read(2) cannot read more than
 +# INT_MAX bytes at once (e.g. MacOS X).
 +#
  # Define NEEDS_CLIPPED_WRITE if your write(2) cannot write more than
  # INT_MAX bytes at once (e.g. MacOS X).

 Is it likely that we would see a platform requiring only one or the
 other CLIPPED? Would it make sense to combine these into a single
 NEEDS_CLIPPED_IO?

I am slightly negative to that suggestion for two reasons.

 - Does MacOS X clip other IO operations?  Do we need to invent yet
   another NEEDS_CLIPPED, e.g. NEEDS_CLIPPED_LSEEK?

   A single NEEDS_CLIPPED_IO may sound attractive for its simplicity
   (e.g. on a system that only needs NEEDS_CLIPPED_WRITE, we will
   unnecessarily chop a big read into multiple reads, but that does
   not affect the correctness of the operation, only performance but
   the actual IO cost will dominate it anyway).  If we know there
   are 47 different IO operations that might need clipping, that
   simplicity is certainly a good thing to have.  I somehow do not
   think the set of operations will grow that large, though.

 - NEEDS_CLIPPED_IO essentially says only those who clip their
   writes would clip their reads (and vice versa), which is not all
   that different from saying only Apple would clip their IO,
   which in turn defeats the notion of let's use a generic
   NEEDS_CLIPPED without limiting the workaround to specific
   platforms somewhat.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X

[Steffen Prohaska]

On Aug 19, 2013, at 6:04 PM, Linus Torvalds torva...@linux-foundation.org 
wrote:

 I hate your patch for other reasons, though:
 
 The problem for read() is addressed in a similar way by introducing
 a wrapper function in compat that always reads less than 2GB.
 
 Why do you do that? We already _have_ wrapper functions for read(),
 namely xread().  Exactly because you basically have to, in order to
 handle signals on interruptible filesystems (which aren't POSIX
 either, but at least sanely so) or from other random sources. And to
 handle the you can't do reads that big issue.
 
 So why isn't the patch much more straightforward? 

The first version was more straightforward [1].  But reviewers suggested
that the compat wrappers would be the right way to do it and showed me
that it has been done like this before [2].

I haven't submitted anything in a while, so I tried to be a kind person
and followed the suggestions.  I started to hate the patch a bit (maybe less
than you), but I wasn't brave enough to reject the suggestions.  This is
why the patch became what it is.

I'm happy to rework it again towards your suggestion.  I would also remove
the compat wrapper for write().  But I got a bit tired.  I'd appreciate if
I received more indication whether a version without compat wrappers would
be accepted.

Steffen

[1] http://article.gmane.org/gmane.comp.version-control.git/232455
[2] 6c642a8 compate/clipped-write.c: large write(2) fails on Mac OS X/XNU--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (Aug 2013, #04; Thu, 15)

[Junio C Hamano]

Johannes Sixt j...@kdbg.org writes:

 Am 16.08.2013 00:36, schrieb Junio C Hamano:
 Due to unfortunate regressions, two topics had to be reverted:

   * An attempted fix to git stash save, to detect that going back
 to the state of the HEAD needs to lose killed files, and/or
 untracked files in a killed directory, to prevent the command
 from proceeding without --force.

 This used ls-files -k that was unusably slow.

   * An attempted enhancement to allow @ to be used to name HEAD.

 This rewrote @ in a ref where it shouldn't have,
 e.g. refs/@/foo.

 You still need to remove corresponding paragraphs from the release notes.

Indeed.  Thanks for reminding.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

CPPCheck found 24 high risk bugs in Git v.1.8.3.4

[Koch, Rick (Subcontractor)]

I'm directing to this e-mail, as it seems to be the approved forum for posting 
Git bugs. We ran CPPCheck against Git v.1.8.3.4 and found 24 high risk bugs.  
Please see the attachment xlsx. 

Is there a method to post to the Git community to allow the community to review 
and debunk as faults positive or develop patches to fix lists code files?

v/r

Roderick (Rick) Koch
Information Assurance
rick.k...@tbe.com



CPPCheck_on_Git.v1.8.3.4.xlsx
Description: CPPCheck_on_Git.v1.8.3.4.xlsx

Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X

[Junio C Hamano]

Linus Torvalds torva...@linux-foundation.org writes:

 I hate your patch for other reasons, though:

 The problem for read() is addressed in a similar way by introducing
 a wrapper function in compat that always reads less than 2GB.

 Why do you do that? We already _have_ wrapper functions for read(),
 namely xread().  Exactly because you basically have to, in order to
 handle signals on interruptible filesystems (which aren't POSIX
 either, but at least sanely so) or from other random sources. And to
 handle the you can't do reads that big issue.

The same argument applies to xwrite(), but currently we explicitly
catch EINTR and EAGAIN knowing that on sane systems these are the
signs that we got interrupted.

Do we catch EINVAL unconditionally in the same codepath?  Could
EINVAL on saner systems mean completely different thing (like our
caller is passing bogus parameters to underlying read/write, which
is a program bug we would want to catch)?

 So why isn't the patch much more straightforward? Like the attached
 totally untested one that just limits the read/write size to 8MB
 (which is totally arbitrary, but small enough to not have any latency
 issues even on slow disks, and big enough that any reasonable IO
 subsystem will still get good throughput).

Ahh.  OK, not noticing EINVAL unconditionally, but always feed IOs
in chunks that are big enough for sane systems but small enough for
broken ones.

That makes sense.  Could somebody on MacOS X test this?

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X

[Junio C Hamano]

Steffen Prohaska proha...@zib.de writes:

 I'm happy to rework it again towards your suggestion.  I would also remove
 the compat wrapper for write().  But I got a bit tired.  I'd appreciate if
 I received more indication whether a version without compat wrappers would
 be accepted.

I think it is a reasonable way forward to remove the writer side
wrapper and doing large IO in reasonably big (but small enough not to
trigger MacOS X limitations) chunks in both read/write direction.

Linus, thanks for a dose of sanity.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH gitk 0/4] gitk support for git log -L

[Junio C Hamano]

Thomas Rast tr...@inf.ethz.ch writes:

 Whether the option value is a separate argument in argv, or directly
 stuck to the option.

 stuck:   gitk -L:foo:main.c
 unstuck: gitk -L :foo:main.c

 Existing gitk chokes on 'gitk -S foo', but works with 'git -Sfoo'.

I somehow thought that we encourage the stuck/sticked form, to
reduce things the users need to remember to cope better with options
with optional value.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X

[Linus Torvalds]

On Mon, Aug 19, 2013 at 10:16 AM, Junio C Hamano gits...@pobox.com wrote:
 Linus Torvalds torva...@linux-foundation.org writes:

 The same argument applies to xwrite(), but currently we explicitly
 catch EINTR and EAGAIN knowing that on sane systems these are the
 signs that we got interrupted.

 Do we catch EINVAL unconditionally in the same codepath?

No, and we shouldn't. If EINVAL happens, it will keep happening.

But with the size limiter, it doesn't matter, since we won't hit the
OS X braindamage.

 Could
 EINVAL on saner systems mean completely different thing (like our
 caller is passing bogus parameters to underlying read/write, which
 is a program bug we would want to catch)?

Yes. Even on OS X, it means that - it's just that OS X notion of what
is bogus is pure crap. But the thing is, looping on EINVAL would be
wrong even on OS X, since unless you change the size, it will keep
happening forever.

But with the limit IO to 8MB (or whatever) patch, the issue is moot.
If you get an EINVAL, it will be due to something else being horribly
horribly wrong.

 Linus
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Git segmentation faults if submodule path is empty.

[Junio C Hamano]

Jharrod LaFon jla...@eyesopen.com writes:

 Updated the patch and the patch submission.


Getting a lot warmer ;-).

  -- 8 --

 Git segmentation faults if submodule path is empty.

If this is meant to replace the MUA's Subject: line, then please add
Subject:  prefix, like the example at the end of this message.

Our commit titles by convention omit the final full-stop.

 Git fails due to a segmentation fault if a submodule path is empty.

Please do not indent the body of the commit log message.  Flush them
to the left.

 Here is an example .gitmodules that will cause a segmentation fault:

Please have a blank line before an example added for illustration in
the log message.

 [submodule foo-module]
   path
   url = http://host/repo.git
 $ git status
 Segmentation fault (core dumped)

Indenting such an illustration, and having a blank line after it,
are good things. Please continue doing so.

 This occurs because in the function parse_submodule_config_option, the

Again, please do not indent the body text of the log message.

 variable 'value' is assumed to be null, and when passed as an argument
 to xstrdup a segmentation fault occurs if it is indeed null.
 This is the case when using the .gitmodules example above.

It is not assumed to be null, is it?

 This patch addresses the issue by checking to make sure 'value' is not
 null before using it as an argument to xstrdup.  For some configuration
 options, such as fetchRecurseSubmodules, an empty value is valid.  If
 the option being read is 'path', an empty value is not valid, and so
 an error message is printed.

We like to write the log message in the imperative mood, as if we
are ordering the codebase to make it so.

 Signed-off-by: Jharrod LaFon jlafon at eyesopen.com

Please do not do cute  at  here.  With a Signed-off-by, you
are already agreeing to Developer's Certificate of Origin 1.1,
cluase (d):

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

 ---
  submodule.c|6 ++
  t/t1307-null-submodule-path.sh |   14 ++

Can we have the test not in a brand new test script file, but at the
end of an existing one?

 diff --git a/t/t1307-null-submodule-path.sh b/t/t1307-null-submodule-path.sh
 new file mode 100755
 index 000..a4470a8
 --- /dev/null
 +++ b/t/t1307-null-submodule-path.sh
 @@ -0,0 +1,14 @@
 +#!/bin/sh
 +
 +test_description='test empty submodule path'
 +. ./test-lib.sh
 +
 +setup() {
 +echo '[submodule test] path'  .gitmodules
 +}

No SP after redirection operator, i.e.

echo '[submodule test] path' .gitmodules

Also it does not make much sense to have a helper script that is
used only once (for that matter, it does not make much sense to add
a new script file to add a single two-liner test).

Here is to illustrate all the points mentioned above.

-- 8 --
From: Jharrod LaFon jla...@eyesopen.com
Subject: avoid segfault on submodule.*.path set to an empty true
Date: Mon, 19 Aug 2013 09:26:56 -0700

Git fails due to a segmentation fault if a submodule path is empty.
Here is an example .gitmodules that will cause a segmentation fault:

[submodule foo-module]
  path
  url = http://host/repo.git
$ git status
Segmentation fault (core dumped)

This is because the parsing of submodule.*.path is not prepared to
see a value-less true and assumes that the value is always
non-NULL (parsing of ignore has the same problem).

Fix it by checking the NULL-ness of value and complain with
config_error_nonbool().

Signed-off-by: Jharrod LaFon jla...@eyesopen.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 submodule.c|  6 ++
 t/t7400-submodule-basic.sh | 10 ++
 2 files changed, 16 insertions(+)

diff --git a/submodule.c b/submodule.c
index 3f0a3f9..c0f93c2 100644
--- a/submodule.c
+++ b/submodule.c
@@ -134,6 +134,9 @@ int parse_submodule_config_option(const char *var, const 
char *value)
return 0;
 
if (!strcmp(key, path)) {
+   if (!value)
+   return config_error_nonbool(var);
+
config = unsorted_string_list_lookup(config_name_for_path, 
value);
if (config)
free(config-util);
@@ -151,6 +154,9 @@ int parse_submodule_config_option(const char *var, const 
char *value)
} else if (!strcmp(key, ignore)) {
char *name_cstr;
 
+   if (!value)
+   return config_error_nonbool(var);
+
if (strcmp(value, untracked)  strcmp(value, dirty) 
strcmp(value, all)  strcmp(value, none)) {
   

[ANNOUNCE] Git v1.8.4-rc4

[Junio C Hamano]

A release candidate Git v1.8.4-rc4 is now available for testing
at the usual places.

The only changes since -rc3 are reversion of two topics that
introduced regressions.  Hopefully the final at the end of this week
and then we will start the next cycle, most likely to be for 1.8.5.

This is a bit delayed than I planned early last week, but sometimes
real life happens.

The release tarballs are found at:

http://code.google.com/p/git-core/downloads/list

and their SHA-1 checksums are:

f390da8ea032c92c673d4cc6c4d8818379db344d  git-1.8.4.rc4.tar.gz
90a9df6b2ada9a0ab9c8711e03d77244e7310c1e  git-htmldocs-1.8.4.rc4.tar.gz
4e6ed2c0307ba538257bdc9f233dd574b419f411  git-manpages-1.8.4.rc4.tar.gz

The following public repositories all have a copy of the v1.8.4-rc4
tag and the master branch that the tag points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = https://code.google.com/p/git-core/
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git

Git v1.8.4 Release Notes (draft)


Backward compatibility notes (for Git 2.0)
--

When git push [$there] does not say what to push, we have used the
traditional matching semantics so far (all your branches were sent
to the remote as long as there already are branches of the same name
over there).  In Git 2.0, the default will change to the simple
semantics that pushes:

 - only the current branch to the branch with the same name, and only
   when the current branch is set to integrate with that remote
   branch, if you are pushing to the same remote as you fetch from; or

 - only the current branch to the branch with the same name, if you
   are pushing to a remote that is not where you usually fetch from.

Use the user preference configuration variable push.default to
change this.  If you are an old-timer who is used to the matching
semantics, you can set the variable to matching to keep the
traditional behaviour.  If you want to live in the future early, you
can set it to simple today without waiting for Git 2.0.

When git add -u (and git add -A) is run inside a subdirectory and
does not specify which paths to add on the command line, it
will operate on the entire tree in Git 2.0 for consistency
with git commit -a and other commands.  There will be no
mechanism to make plain git add -u behave like git add -u ..
Current users of git add -u (without a pathspec) should start
training their fingers to explicitly say git add -u .
before Git 2.0 comes.  A warning is issued when these commands are
run without a pathspec and when you have local changes outside the
current directory, because the behaviour in Git 2.0 will be different
from today's version in such a situation.

In Git 2.0, git add path will behave as git add -A path, so
that git add dir/ will notice paths you removed from the directory
and record the removal.  Versions before Git 2.0, including this
release, will keep ignoring removals, but the users who rely on this
behaviour are encouraged to start using git add --ignore-removal path
now before 2.0 is released.


Updates since v1.8.3


Foreign interfaces, subsystems and ports.

 * Cygwin port has been updated for more recent Cygwin 1.7.

 * git rebase -i now honors --strategy and -X options.

 * Git-gui has been updated to its 0.18.0 version.

 * MediaWiki remote helper (in contrib/) has been updated to use the
   credential helper interface from Git.pm.

 * Update build for Cygwin 1.[57].  Torsten Bögershausen reports that
   this is fine with Cygwin 1.7 ($gmane/225824) so let's try moving it
   ahead.

 * The credential helper to talk to keychain on OS X (in contrib/) has
   been updated to kick in not just when talking http/https but also
   imap(s) and smtp.

 * Remote transport helper has been updated to report errors and
   maintain ref hierarchy used to keep track of its own state better.

 * With export remote-helper protocol, (1) a push that tries to
   update a remote ref whose name is different from the pushing side
   does not work yet, and (2) the helper may not know how to do
   --dry-run; these problematic cases are disabled for now.

 * git-remote-hg/bzr (in contrib/) updates.

 * git-remote-mw (in contrib/) hints users to check the certificate,
   when https:// connection failed.

 * git-remote-mw (in contrib/) adds a command to allow previewing the
   contents locally before pushing it out, when working with a
   MediaWiki remote.


UI, Workflows  Features

 * Sample post-receive-email hook script got an enhanced replacement
   multimail (in contrib/).

 * Also in contrib/ is a new contacts script that runs git blame
   to find out the people who may be interested in a set of changes.

 * git clean command learned an interactive mode.

 * The --head option to git show-ref was only to add 

Re: CPPCheck found 24 high risk bugs in Git v.1.8.3.4

[Philip Oakley]

From: Koch, Rick (Subcontractor) rick.k...@tbe.com
Sent: Monday, August 19, 2013 6:09 PM

I'm directing to this e-mail, as it seems to be the approved forum
for posting Git bugs. We ran CPPCheck against Git v.1.8.3.4
and found 24 high risk bugs. Please see the attachment xlsx.



Is there a method to post to the Git community to allow the
community to review and debunk as faults positive or develop
patches to fix lists code files?



v/r



Roderick (Rick) Koch
Information Assurance
rick.k...@tbe.com


What OS version / CPPCheck version was this checked on?

In case other readers don't have a .xlsx reader here is Rick's list in 
plain text (may be white space damaged).


I expect some will be false positives, and some will just be being too 
cautious.


Philip

description resourceFilePath fileName lineNumber
 nullPointer(CppCheck) \git-master\builtin\add.c add.c 286
 wrongPrintfScanfArgNum(CppCheck) \git-master\builtin\fetch.c
fetch.c 588
 nullPointer(CppCheck) \git-master\builtin\ls-files.c ls-files.c
144
 nullPointer(CppCheck) \git-master\builtin\merge.c merge.c 1208
 doubleFree(CppCheck) \git-master\builtin\notes.c notes.c 275
 nullPointer(CppCheck) \git-master\builtin\reflog.c reflog.c 437
 uninitvar(CppCheck) \git-master\builtin\rev-list.c rev-list.c 342
 uninitvar(CppCheck) \git-master\builtin\rev-list.c rev-list.c 342
 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c
2803
 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c
2802
 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c
2805
 memleakOnRealloc(CppCheck) \git-master\compat\win32\syslog.c
syslog.c 46
 uninitvar(CppCheck)
\git-master\contrib\examples\builtin-fetch--tool.c builtin-fetch--tool.c
419
 uninitvar(CppCheck) \git-master\fast-import.c fast-import.c 2917
 nullPointer(CppCheck) \git-master\line-log.c line-log.c 638
 nullPointer(CppCheck) \git-master\mailmap.c mailmap.c 156
 uninitvar(CppCheck) \git-master\merge-recursive.c
merge-recursive.c 1887
 uninitvar(CppCheck) \git-master\notes.c notes.c 805
 uninitvar(CppCheck) \git-master\notes.c notes.c 805
 deallocret(CppCheck) \git-master\pretty.c pretty.c 677
 resourceLeak(CppCheck) \git-master\refs.c refs.c 3041
 doubleFree(CppCheck) \git-master\sequencer.c sequencer.c 924
 nullPointer(CppCheck) \git-master\sha1_file.c sha1_file.c 125
 doubleFree(CppCheck) \git-master\shell.c shell.c 130

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: CPPCheck found 24 high risk bugs in Git v.1.8.3.4

[Jeff King]

On Mon, Aug 19, 2013 at 09:03:21PM +0100, Philip Oakley wrote:

 In case other readers don't have a .xlsx reader here is Rick's list
 in plain text (may be white space damaged).
 
 I expect some will be false positives, and some will just be being
 too cautious.

 [...]
 
 description resourceFilePath fileName lineNumber
  nullPointer(CppCheck) \git-master\builtin\add.c add.c 286

Hm. That code in v1.8.3.4 reads:

if (pathspec)
while (pathspec[pc])
pc++;

What's the problem? If pathspec is not properly terminated, we can run
off the end, but I do see anything to indicate that is the case. What
does the nullPointer check mean here?

  wrongPrintfScanfArgNum(CppCheck) \git-master\builtin\fetch.c
 fetch.c 588

Line 588 does not have formatted I/O at all. Are these line numbers
somehow not matching what I have in v1.8.3.4?

  nullPointer(CppCheck) \git-master\builtin\ls-files.c ls-files.c
 144

This one looks like:

   if (tag  *tag  show_valid_bit 
(ce-ce_flags  CE_VALID)) {
static char alttag[4];
memcpy(alttag, tag, 3);
if (isalpha(tag[0]))

where the final line is 144. But we have explicitly checked that tag is not
NULL...

  doubleFree(CppCheck) \git-master\builtin\notes.c notes.c 275

This one looks like:

  if (...) {
free(buf);
die(...);
  }
  ...
  free(buf);

which might look like a double free if you do not know that die() will
never return (it is properly annotated for gcc, but I don't know whether
CppCheck understands such things).

So out of the 4 entries I investigated, none of them looks like an
actual problem. But I'm not even sure I am looking at the right place;
these don't even seem like things that would cause a false positive in a
static analyzer.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ANNOUNCE] Git v1.8.4-rc4

[Kyle J. McKay]

On Aug 19, 2013, at 12:59, Junio C Hamano wrote:


Performance, Internal Implementation, etc.

* On Cygwin, we used to use our own lstat(2) emulation that is
  allegedly faster than the platform one in codepaths where some of
  the information it returns did not matter, but it started to bite
  us in a few codepaths where the trick it uses to cheat does show
  breakages. This emulation has been removed and we use the native
  lstat(2) emulation supplied by Cygwin now.

* The function attributes extensions are used to catch mistakes in
  use of our own variadic functions that use NULL sentinel at the end
  (i.e. like execl(3)) and format strings (i.e. like printf(3)).

* The code to allow configuration data to be read from in-tree blob
  objects is in.  This may help working in a bare repository and
  submodule updates.

* Fetching between repositories with many refs employed O(n^2)
  algorithm to match up the common objects, which has been corrected.

* The original way to specify remote repository using .git/branches/
  used to have a nifty feature.  The code to support the feature was
  still in a function but the caller was changed not to call it 5
  years ago, breaking that feature and leaving the supporting code
  unreachable.  The dead code has been removed.

* git pack-refs that races with new ref creation or deletion have
  been susceptible to lossage of refs under right conditions, which
  has been tightened up.

* We read loose and packed rerferences in two steps, but after

^^

  deciding to read a loose ref but before actually opening it to read


s/rerferences/references/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: CPPCheck found 24 high risk bugs in Git v.1.8.3.4

[Junio C Hamano]

Jeff King p...@peff.net writes:

 On Mon, Aug 19, 2013 at 09:03:21PM +0100, Philip Oakley wrote:

 In case other readers don't have a .xlsx reader here is Rick's list
 in plain text (may be white space damaged).
 
 I expect some will be false positives, and some will just be being
 too cautious.

 [...]
 
 description resourceFilePath fileName lineNumber
  nullPointer(CppCheck) \git-master\builtin\add.c add.c 286

 Hm. That code in v1.8.3.4 reads:

 if (pathspec)
 while (pathspec[pc])
 pc++;

 What's the problem? If pathspec is not properly terminated, we can run
 off the end, but I do see anything to indicate that is the case. What
 does the nullPointer check mean here?

  wrongPrintfScanfArgNum(CppCheck) \git-master\builtin\fetch.c
 fetch.c 588

 Line 588 does not have formatted I/O at all. Are these line numbers
 somehow not matching what I have in v1.8.3.4?

  nullPointer(CppCheck) \git-master\builtin\ls-files.c ls-files.c
 144

 This one looks like:

if (tag  *tag  show_valid_bit 
 (ce-ce_flags  CE_VALID)) {
 static char alttag[4];
 memcpy(alttag, tag, 3);
 if (isalpha(tag[0]))

 where the final line is 144. But we have explicitly checked that tag is not
 NULL...

  doubleFree(CppCheck) \git-master\builtin\notes.c notes.c 275

 This one looks like:

   if (...) {
 free(buf);
 die(...);
   }
   ...
   free(buf);

 which might look like a double free if you do not know that die() will
 never return (it is properly annotated for gcc, but I don't know whether
 CppCheck understands such things).

 So out of the 4 entries I investigated, none of them looks like an
 actual problem. But I'm not even sure I am looking at the right place;
 these don't even seem like things that would cause a false positive in a
 static analyzer.

And the ones I picked at random looks totally bogus, too.

 uninitvar(CppCheck) \git-master\notes.c notes.c 805
 uninitvar(CppCheck) \git-master\notes.c notes.c 805

That is

int combine_notes_concatenate(unsigned char *cur_sha1,
const unsigned char *new_sha1)
{
char *cur_msg = NULL, *new_msg = NULL, *buf;
unsigned long cur_len, new_len, buf_len;
enum object_type cur_type, new_type;
int ret;

/* read in both note blob objects */
if (!is_null_sha1(new_sha1))
new_msg = read_sha1_file(new_sha1, new_type, new_len);
805:if (!new_msg || !new_len || new_type != OBJ_BLOB) {
free(new_msg);
return 0;
}


new_msg starts out to be NULL, if we did not run read_sha1_file(),
it will still be NULL and if() will not look at new_len/new_type
so their being uninitialized does not matter.  If we did run
read_sha1_file(), and if it returns a non-NULL new_msg, both of
these are filled.  If read_sha1_file() returns a NULL new_msg, again
the other two does not matter.

In short, the analyzer seems to be giving useless noise for this
one.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Git segmentation faults if submodule path is empty.

[Jeff King]

On Mon, Aug 19, 2013 at 11:56:17AM -0700, Junio C Hamano wrote:

 Jharrod LaFon jla...@eyesopen.com writes:
 
  Updated the patch and the patch submission.
 
 
 Getting a lot warmer ;-).

Thanks, I agree with all of the stuff you said. The end result that you
included looks good to me.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Git segmentation faults if submodule path is empty.

[Junio C Hamano]

Jharrod LaFon jla...@eyesopen.com writes:

 I will keep trying this until it's perfect, and I thank you for
 the help.  When I resubmit this, would you like me to include your
 sign-off line as well?

If the one I attached at the end of the message you are responding
to looks fine to you, I'd just apply it without having you to
reroll.

 Also, the end of the test script was not included in your message.  

Sorry, but I am not sure what you meant by this.

I reworked your example to make it the second test in an existing
test script.  There are many existing tests after that so it is
natural that we wouldn't see the end of the file in the patch
context.

 diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
 index 5ee97b0..a39d074 100755
 --- a/t/t7400-submodule-basic.sh
 +++ b/t/t7400-submodule-basic.sh
 @@ -18,6 +18,16 @@ test_expect_success 'setup - initial commit' '
  git branch initial
 '
 
 +test_expect_success 'configuration parsing' '
 +test_when_finished rm -f .gitmodules 
 +cat .gitmodules -\EOF 
 +[submodule s]
 +path
 +ignore
 +EOF
 +test_must_fail git status
 +'
 +
 test_expect_success 'setup - repository in init subdirectory' '
  mkdir init 
  (
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] submodule: fix confusing variable name

[Jens Lehmann]

Am 17.08.2013 19:25, schrieb brian m. carlson:
 cmd_summary reads the output of git diff, but reads in the submodule path 
 into a
 variable called name.  Since this variable does not contain the name of the
 submodule, but the path, rename it to be clearer what data it actually holds.
 
 Signed-off-by: brian m. carlson sand...@crustytoothpaste.net

Thanks, this one is looking good to me.

Acked-by: Jens Lehmann jens.lehm...@web.de

 ---
  git-submodule.sh | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)
 
 diff --git a/git-submodule.sh b/git-submodule.sh
 index 2979197..38520db 100755
 --- a/git-submodule.sh
 +++ b/git-submodule.sh
 @@ -1032,13 +1032,13 @@ cmd_summary() {
   # Get modified modules cared by user
   modules=$(git $diff_cmd $cached --ignore-submodules=dirty --raw $head 
 -- $@ |
   sane_egrep '^:([0-7]* )?16' |
 - while read mod_src mod_dst sha1_src sha1_dst status name
 + while read mod_src mod_dst sha1_src sha1_dst status sm_path
   do
   # Always show modules deleted or type-changed 
 (blob-module)
 - test $status = D -o $status = T  echo $name  
 continue
 + test $status = D -o $status = T  echo $sm_path  
 continue
   # Also show added or modified modules which are checked 
 out
 - GIT_DIR=$name/.git git-rev-parse --git-dir /dev/null 
 21 
 - echo $name
 + GIT_DIR=$sm_path/.git git-rev-parse --git-dir 
 /dev/null 21 
 + echo $sm_path
   done
   )
  
 

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] submodule: don't print status output with ignore=all

[Jens Lehmann]

Am 17.08.2013 19:25, schrieb brian m. carlson:
 git status prints information for submodules, but it should ignore the status 
 of
 those which have submodule.name.ignore set to all.  Fix it so that it does
 properly ignore those which have that setting either in .git/config or in
 .gitmodules.
 
 Not ignored are submodules that are added, deleted, or moved (which is
 essentially a combination of the first two) because it is not easily possible 
 to
 determine the old path once a move has occurred, nor is it easily possible to
 detect which adds and deletions are moves and which are not.  This also
 preserves the previous behavior of always listing modules which are to be
 deleted.

Sounds sane. Even though we should be able to follow renames by inspecting
the old .gitmodules content with the new --blob option of git config, I doubt
it'll be worth the hassle. Thanks for fixing two known test breakages.

Acked-by: Jens Lehmann jens.lehm...@web.de

 Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
 ---
  git-submodule.sh  | 7 +++
  t/t7508-status.sh | 4 ++--
  2 files changed, 9 insertions(+), 2 deletions(-)
 
 diff --git a/git-submodule.sh b/git-submodule.sh
 index 38520db..c1ba0f8 100755
 --- a/git-submodule.sh
 +++ b/git-submodule.sh
 @@ -1036,6 +1036,13 @@ cmd_summary() {
   do
   # Always show modules deleted or type-changed 
 (blob-module)
   test $status = D -o $status = T  echo $sm_path  
 continue
 + # Respect the ignore setting for --for-status.
 + if test -n $for_status
 + then
 + name=$(module_name $sm_path)
 + ignore_config=$(get_submodule_config $name 
 ignore none)
 + test $status != A -a $ignore_config = all  
 continue
 + fi
   # Also show added or modified modules which are checked 
 out
   GIT_DIR=$sm_path/.git git-rev-parse --git-dir 
 /dev/null 21 
   echo $sm_path
 diff --git a/t/t7508-status.sh b/t/t7508-status.sh
 index ac3d0fe..fb89fb9 100755
 --- a/t/t7508-status.sh
 +++ b/t/t7508-status.sh
 @@ -1316,7 +1316,7 @@ test_expect_success --ignore-submodules=all suppresses 
 submodule summary '
   test_i18ncmp expect output
  '
  
 -test_expect_failure '.gitmodules ignore=all suppresses submodule summary' '
 +test_expect_success '.gitmodules ignore=all suppresses submodule summary' '
   git config --add -f .gitmodules submodule.subname.ignore all 
   git config --add -f .gitmodules submodule.subname.path sm 
   git status  output 
 @@ -1324,7 +1324,7 @@ test_expect_failure '.gitmodules ignore=all suppresses 
 submodule summary' '
   git config -f .gitmodules  --remove-section submodule.subname
  '
  
 -test_expect_failure '.git/config ignore=all suppresses submodule summary' '
 +test_expect_success '.git/config ignore=all suppresses submodule summary' '
   git config --add -f .gitmodules submodule.subname.ignore none 
   git config --add -f .gitmodules submodule.subname.path sm 
   git config --add submodule.subname.ignore all 
 

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] submodule: prevent warning in summary output

[Jens Lehmann]

Am 19.08.2013 10:13, schrieb Chris Packham:
 Hi Brian,
 On 19/08/13 05:31, brian m. carlson wrote:
 When git submodule summary is run and there is a deleted submodule, there is 
 an
 warning from git rev-parse:

   fatal: Not a git repository: '.vim/pathogen/.git'

 Silence this warning, since it is fully expected that a deleted submodule 
 will
 not be a git repository.

 Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
 ---
  git-submodule.sh | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/git-submodule.sh b/git-submodule.sh
 index 2979197..66ee621 100755
 --- a/git-submodule.sh
 +++ b/git-submodule.sh
 @@ -1071,7 +1071,7 @@ cmd_summary() {
  missing_dst=
  
  test $mod_src = 16 
 -! GIT_DIR=$name/.git git-rev-parse -q --verify $sha1_src^0 
 /dev/null 
 +! GIT_DIR=$name/.git git-rev-parse -q --verify $sha1_src^0 
 /dev/null 21 
  missing_src=t
  
  test $mod_dst = 16 

 
 I wonder if there are other useful errors this will silence
 unintentionally. Perhaps this would be better (untested)
 
  test $mod_src = 16 
  test -e $name/.git 
  ! GIT_DIR=$name/.git git-rev-parse -q --verify $sha1_src^0 /dev/null 
   missing_src=t
 
 Having said that there are precedents for both in git-submodule.sh. If
 there aren't any errors worth catching here then your way is probably
 cleaner than mine.

I'd prefer a way to not drop all errors too. We should be able to use
the status variable to see if the submodule is deleted and then skip
the rev-parse all together, or am I missing something here?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git-send-email: kill $prompting variable

[Jeff King]

On Fri, Aug 16, 2013 at 05:34:04PM +, Rasmus Villemoes wrote:

 The variable $prompting is weird. It is only read in one place (when
 deciding whether to prompt for a Message-ID to use in In-Reply-To),
 and it will be false unless we've taken the completely unrelated
 branch filling in @initial_to.
 
 Prompting should be done if the info is needed, not if some unrelated
 item had to be prompted for. So kill $prompting.

The prompting flag dates back to 1f038a0 from late 2005. I _think_ the
intent was that you could use certain command lines to specify the
required information (like initial compose subject line, sender, etc),
and then send-email would skip prompting for the optional information
(like in-reply-to). That makes it easier to use in a batch mode in
which the user does not want to be prompted (they do not have to give a
blank --in-reply-to to prevent the prompt).

Over the years, the set of items which triggered prompting (and which
depended on previous prompts) has grown and shrunk, and most prompts do
not respect the $prompting system at all. So I kind of doubt that
anybody will care if it goes away; it does not make much sense at this
point.

However, your patch will make the default be to ask about the initial
message-id. Which is likely going to annoy people, as it is not
necessary (and people who care can specify it on the command line).
Would we want to get rid of it entirely?

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: CPPCheck found 24 high risk bugs in Git v.1.8.3.4

[Stefan Beller]

On 08/19/2013 07:09 PM, Koch, Rick (Subcontractor) wrote:
 I'm directing to this e-mail, as it seems to be the approved forum for 
 posting Git bugs. We ran CPPCheck against Git v.1.8.3.4 and found 24 high 
 risk bugs.  Please see the attachment xlsx. 
 
 Is there a method to post to the Git community to allow the community to 
 review and debunk as faults positive or develop patches to fix lists code 
 files?
 

Hi,

if you're using cppcheck as found at https://github.com/danmar/cppcheck 
or http://sourceforge.net/apps/trac/cppcheck/ you really need to review 
the results, as there are many false positives.

I used that tool for my contributions so far (bug fixes as reported by 
cppcheck).
However you *really* need to manually review any message cppcheck generates.
This is because git is using a C, asm-like coding style for many routines,
whereas that cppcheck is rather optimized to find typical C++ errors.
And the styles vary wildy! (cppcheck tries to become no false positives, 
but it's hard I guess)

I am running that cppcheck tool on git regulary (cppcheck master branch on
git master branch), and review for real findings, you're welcome to do so
as well. :) 

There are other static code analyzers, which have slightly different 
goals, such as http://css.csail.mit.edu/stack/ which has an incredibly 
low false positive rate (I found none as of now).
However I think having different tools is a great thing, but you'd need
to know your tools. ;)

Stefan



signature.asc
Description: OpenPGP digital signature

Re: CPPCheck found 24 high risk bugs in Git v.1.8.3.4

[Philip Oakley]

From: Koch, Rick (Subcontractor) rick.k...@tbe.com


Ran CPPCheck 1.5.6 on Windows-XP.


Hi Rick,
Thank you for the clarification.
Normal practice on the list is to use Reply All, so everyone can 
participate in the discussion.


It looks like most of the reports are false positives. My bikeshedding 
thought would be that it is common in Git to inspect all the call sites 
such that they don't create the various problems, rather than protect 
against the problems within the various functions, which may be a cause 
of the reports (i.e. different philosophical approach to checking).


regards

Philip
---

v/r

Roderick (Rick) Koch
OSF - Information Assurance
Team Teledyne / Sentar Inc.
Work: 256-726-1253
rick.k...@tbe.com


-Original Message-
From: Philip Oakley [mailto:philipoak...@iee.org]
Sent: Monday, August 19, 2013 3:03 PM
To: Koch, Rick (Subcontractor); Git List
Subject: Re: CPPCheck found 24 high risk bugs in Git v.1.8.3.4

From: Koch, Rick (Subcontractor) rick.k...@tbe.com
Sent: Monday, August 19, 2013 6:09 PM

I'm directing to this e-mail, as it seems to be the approved forum for
posting Git bugs. We ran CPPCheck against Git v.1.8.3.4 and found 24
high risk bugs. Please see the attachment xlsx.



Is there a method to post to the Git community to allow the community
to review and debunk as faults positive or develop patches to fix lists
code files?



v/r



Roderick (Rick) Koch
Information Assurance
rick.k...@tbe.com


What OS version / CPPCheck version was this checked on?

In case other readers don't have a .xlsx reader here is Rick's list in 
plain text (may be white space damaged).


I expect some will be false positives, and some will just be being too 
cautious.


Philip

description resourceFilePath fileName lineNumber
 nullPointer(CppCheck) \git-master\builtin\add.c add.c 286
 wrongPrintfScanfArgNum(CppCheck) \git-master\builtin\fetch.c 
fetch.c 588

 nullPointer(CppCheck) \git-master\builtin\ls-files.c ls-files.c
144
 nullPointer(CppCheck) \git-master\builtin\merge.c merge.c 1208
 doubleFree(CppCheck) \git-master\builtin\notes.c notes.c 275
 nullPointer(CppCheck) \git-master\builtin\reflog.c reflog.c 437
 uninitvar(CppCheck) \git-master\builtin\rev-list.c rev-list.c 342
 uninitvar(CppCheck) \git-master\builtin\rev-list.c rev-list.c 342
 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c
2803
 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c
2802
 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c
2805
 memleakOnRealloc(CppCheck) \git-master\compat\win32\syslog.c 
syslog.c 46

 uninitvar(CppCheck)
\git-master\contrib\examples\builtin-fetch--tool.c builtin-fetch--tool.c
419
 uninitvar(CppCheck) \git-master\fast-import.c fast-import.c 2917
 nullPointer(CppCheck) \git-master\line-log.c line-log.c 638
 nullPointer(CppCheck) \git-master\mailmap.c mailmap.c 156
 uninitvar(CppCheck) \git-master\merge-recursive.c 
merge-recursive.c 1887

 uninitvar(CppCheck) \git-master\notes.c notes.c 805
 uninitvar(CppCheck) \git-master\notes.c notes.c 805
 deallocret(CppCheck) \git-master\pretty.c pretty.c 677
 resourceLeak(CppCheck) \git-master\refs.c refs.c 3041
 doubleFree(CppCheck) \git-master\sequencer.c sequencer.c 924
 nullPointer(CppCheck) \git-master\sha1_file.c sha1_file.c 125
 doubleFree(CppCheck) \git-master\shell.c shell.c 130


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X

[Kyle J. McKay]

On Aug 19, 2013, at 10:16, Junio C Hamano wrote:


Linus Torvalds torva...@linux-foundation.org writes:


So why isn't the patch much more straightforward? Like the attached
totally untested one that just limits the read/write size to 8MB
(which is totally arbitrary, but small enough to not have any latency
issues even on slow disks, and big enough that any reasonable IO
subsystem will still get good throughput).


Ahh.  OK, not noticing EINVAL unconditionally, but always feed IOs
in chunks that are big enough for sane systems but small enough for
broken ones.

That makes sense.  Could somebody on MacOS X test this?


I tested this on both i386 (OS X 32-bit intel) and x86_64 (OS X 64-bit  
intel).


What I tested:

1. I started with branch pu:
   (965adb10 Merge branch 'sg/bash-prompt-lf-in-cwd-test' into pu)

2. I added Steffen's additional test (modified to always run) to t0021:

diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index e50f0f7..b92e6cb 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -190,4 +190,16 @@ test_expect_success 'required filter clean  
failure' '

test_must_fail git add test.fc
'

+test_expect_success 'filter large file' '
+   git config filter.largefile.smudge cat 
+   git config filter.largefile.clean cat 
+   for i in $(test_seq 1 2048); do printf %1048576d 1; done 2GB 
+   echo 2GB filter=largefile .gitattributes 
+   git add 2GB 2err 
+   ! test -s err 
+   rm -f 2GB 
+   git checkout -- 2GB 2err 
+   ! test -s err
+'
+
test_done

3. I verified that the test fails with an unpatched build on both 32- 
bit and 64-bit.


4. I applied Linus's unmodified patch to wrapper.c.

5. I tested again.  The t0021 test now passes on 64-bit.  It still  
fails on 32-bit for another reason unrelated to Linus's patch.


It fails when attempting the git add 2GB line from the new 'filter  
large file' part of the test.  The failure with backtrace:


git(16806,0xa095c720) malloc: *** mmap(size=2147487744) failed (error  
code=12)

*** error: can't allocate region
*** set a breakpoint in malloc_error_break to debug

# NOTE: error code 12 is ENOMEM on OS X

Breakpoint 1, 0x97f634b1 in malloc_error_break ()
(gdb) bt
#0  0x97f634b1 in malloc_error_break ()
#1  0x97f5e49f in szone_error ()
#2  0x97e8b876 in allocate_pages ()
#3  0x97e8c062 in large_and_huge_malloc ()
#4  0x97e831c8 in szone_malloc ()
#5  0x97e82fb8 in malloc_zone_malloc ()
#6  0x97e8c7b2 in realloc ()
#7  0x00128abe in xrealloc (ptr=0x0, size=2147483649) at wrapper.c:100
#8  0x00111a8c in strbuf_grow (sb=0xbfffe634, extra=2147483648) at  
strbuf.c:74
#9  0x00112bb9 in strbuf_read (sb=0xbfffe634, fd=6, hint=2548572518)  
at strbuf.c:349
#10 0x0009b899 in apply_filter (path=value temporarily unavailable,  
due to optimizations, src=0x100 ' ' repeats 200 times...,  
len=2147483648, dst=0xbfffe774, cmd=0x402980 cat) at convert.c:407
#11 0x0009c6f6 in convert_to_git (path=0x4028b4 2GB, src=0x100 '  
' repeats 200 times..., len=2147483648, dst=0xbfffe774,  
checksafe=SAFE_CRLF_WARN) at convert.c:764
#12 0x0010bb38 in index_mem (sha1=0x402330 , buf=0x100,  
size=2147483648, type=OBJ_BLOB, path=0x4028b4 2GB, flags=1) at  
sha1_file.c:3044
#13 0x0010bf57 in index_core [inlined] () at /private/var/tmp/src/git/ 
sha1_file.c:3101
#14 0x0010bf57 in index_fd (sha1=0x402330 , fd=5, st=0xbfffe900,  
type=OBJ_BLOB, path=0x4028b4 2GB, flags=1) at sha1_file.c:3139
#15 0x0010c05e in index_path (sha1=0x402330 , path=0x4028b4 2GB,  
st=0xbfffe900, flags=1) at sha1_file.c:3157
#16 0x000e82f4 in add_to_index (istate=0x1a8820, path=0x4028b4 2GB,  
st=0xbfffe900, flags=0) at read-cache.c:665
#17 0x000e87c8 in add_file_to_index (istate=0x1a8820, path=0x4028b4  
2GB, flags=0) at read-cache.c:694
#18 0x440a in cmd_add (argc=value temporarily unavailable, due to  
optimizations, argv=0xb584, prefix=0x0) at builtin/add.c:299
#19 0x2e1f in run_builtin [inlined] () at /private/var/tmp/src/git/ 
git.c:303
#20 0x2e1f in handle_internal_command (argc=2, argv=0xb584) at  
git.c:466
#21 0x32d4 in run_argv [inlined] () at /private/var/tmp/src/git/ 
git.c:512

#22 0x32d4 in main (argc=2, av=0xbfffe28c) at git.c:595

The size 2147487744 is 2GB + 4096 bytes.  Apparently git does not  
support a filter for a file unless the file can fit entirely into  
git's memory space.  Normally a single 2GB + 4096 byte allocation  
works in an OS X 32-bit process, but something else is apparently  
eating up a large portion of the memory space in this case (perhaps an  
mmap'd copy?).  In any case, if the file being filtered was closer to  
4GB in size it would always fail on 32-bit regardless.


The fact that the entire file is read into memory when applying the  
filter does not seem like a good thing (see #7-#10 above).


--Kyle
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

What's cooking in git.git (Aug 2013, #05; Mon, 19)

[Junio C Hamano]

What's cooking in git.git (Aug 2013, #05; Mon, 19)
--

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

An extra release candidate -rc4 has been tagged and pushed out.
Hopefully this will be the last one before the final release of
1.8.4.  As I expect we will have two more cycles of 1.8.x by the end
of the year and then 2.0 early next year, we may want to merge these
for 2.0 topics to 'next' for real, starting the next cycle.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[New Topics]

* es/rebase-i-respect-core-commentchar (2013-08-18) 1 commit
 - rebase -i: fix cases ignoring core.commentchar

 Will merge to and cook in 'next'.


* jx/branch-vv-always-compare-with-upstream (2013-08-18) 3 commits
 - status: always show tracking branch even no change
 - branch: mark missing tracking branch as gone
 - branch: not report invalid tracking branch


* nd/fetch-into-shallow (2013-08-18) 6 commits
 - list-objects: mark more commits as edges in mark_edges_uninteresting
 - list-objects: reduce one argument in mark_edges_uninteresting
 - upload-pack: delegate rev walking in shallow fetch to pack-objects
 - shallow: add setup_temporary_shallow()
 - shallow: only add shallow graft points to new shallow file
 - move setup_alternate_shallow and write_shallow_commits to shallow.c


* sb/diff-delta-remove-needless-comparison (2013-08-18) 1 commit
 - create_delta_index: simplify condition always evaluating to true

 Will merge to and cook in 'next'.


* sg/bash-prompt-lf-in-cwd-test (2013-08-18) 1 commit
 - bash prompt: test the prompt with newline in repository path

 Will merge to and cook in 'next'.


* jl/some-submodule-config-are-not-boolean (2013-08-19) 1 commit
 - avoid segfault on submodule.*.path set to an empty true

 Will merge to and cook in 'next'.

--
[Stalled]

* tf/gitweb-ss-tweak (2013-07-15) 4 commits
 - gitweb: make search help link less ugly
 - gitweb: omit the repository owner when it is unset
 - gitweb: vertically centre contents of page footer
 - gitweb: ensure OPML text fits inside its box

 Comments?


* rj/read-default-config-in-show-ref-pack-refs (2013-06-17) 3 commits
 - ### DONTMERGE: needs better explanation on what config they need
 - pack-refs.c: Add missing call to git_config()
 - show-ref.c: Add missing call to git_config()

 The changes themselves are probably good, but it is unclear what
 basic setting needs to be read for which exact operation.

 Waiting for clarification.
 $gmane/228294


* jh/shorten-refname (2013-05-07) 4 commits
 - t1514: refname shortening is done after dereferencing symbolic refs
 - shorten_unambiguous_ref(): Fix shortening refs/remotes/origin/HEAD to origin
 - t1514: Demonstrate failure to correctly shorten refs/remotes/origin/HEAD
 - t1514: Add tests of shortening refnames in strict/loose mode

 When remotes/origin/HEAD is not a symbolic ref, rev-parse
 --abbrev-ref remotes/origin/HEAD ought to show origin, not
 origin/HEAD, which is fixed with this series (if it is a symbolic
 ref that points at remotes/origin/something, then it should show
 origin/something and it already does).

 Expecting a reroll, as an early part of a larger series.
 $gmane/225137


* jk/list-objects-sans-blobs (2013-06-06) 4 commits
 . archive: ignore blob objects when checking reachability
 . list-objects: optimize revs-blob_objects = 0 case
 . upload-archive: restrict remote objects with reachability check
 . clear parsed flag when we free tree buffers

 Attempt to allow archive --remote=$there $arbitrary_sha1 while
 keeping the reachability safety.

 Seems to break some tests in a trivial and obvious way.


* mg/more-textconv (2013-05-10) 7 commits
 - grep: honor --textconv for the case rev:path
 - grep: allow to use textconv filters
 - t7008: demonstrate behavior of grep with textconv
 - cat-file: do not die on --textconv without textconv filters
 - show: honor --textconv for blobs
 - diff_opt: track whether flags have been set explicitly
 - t4030: demonstrate behavior of show with textconv

 Make git grep and git show pay attention to --textconv when
 dealing with blob objects.

 I thought this was pretty well designed and executed, but it seems
 there are some doubts on the list; kicked back to 'pu'.


* jc/format-patch (2013-04-22) 2 commits
 - format-patch: --inline-single
 - format-patch: rename no_inline field

 A new option to send a single patch to the standard output to be
 appended at the bottom of a message.  I personally have no need for
 this, but it was easy enough to cobble together.  Tests, docs and
 stripping out more MIMEy stuff are left as exercises to interested
 parties.

 Not ready for inclusion.

Re: [PATCH v4] compat: Fix read() of 2GB and more on Mac OS X

[Linus Torvalds]

On Mon, Aug 19, 2013 at 2:56 PM, Kyle J. McKay mack...@gmail.com wrote:

 The fact that the entire file is read into memory when applying the filter
 does not seem like a good thing (see #7-#10 above).

Yeah, that's horrible. Its likely bad for performance too, because
even if you have enough memory, it blows everything out of the L2/L3
caches, and if you don't have enough memory it obviously causes other
problems.

So it would probably be a great idea to make the filtering code able
to do things in smaller chunks, but I suspect that the patch to chunk
up xread/xwrite is the right thing to do anyway.

  Linus
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: CPPCheck found 24 high risk bugs in Git v.1.8.3.4

[Philip Oakley]

- Original Message - 
From: Philip Oakley philipoak...@iee.org

From: Koch, Rick (Subcontractor) rick.k...@tbe.com
Sent: Monday, August 19, 2013 6:09 PM

I'm directing to this e-mail, as it seems to be the approved forum
for posting Git bugs. We ran CPPCheck against Git v.1.8.3.4
and found 24 high risk bugs. Please see the attachment xlsx.



Is there a method to post to the Git community to allow the
community to review and debunk as faults positive or develop
patches to fix lists code files?



v/r



Roderick (Rick) Koch
Information Assurance
rick.k...@tbe.com


What OS version / CPPCheck version was this checked on?

In case other readers don't have a .xlsx reader here is Rick's list in 
plain text (may be white space damaged).


I expect some will be false positives, and some will just be being too 
cautious.


Philip

description resourceFilePath fileName lineNumber
 nullPointer(CppCheck) \git-master\builtin\add.c add.c 286
 wrongPrintfScanfArgNum(CppCheck) \git-master\builtin\fetch.c
fetch.c 588
 nullPointer(CppCheck) \git-master\builtin\ls-files.c ls-files.c
144
 nullPointer(CppCheck) \git-master\builtin\merge.c merge.c 1208
 doubleFree(CppCheck) \git-master\builtin\notes.c notes.c 275
 nullPointer(CppCheck) \git-master\builtin\reflog.c reflog.c 437
 uninitvar(CppCheck) \git-master\builtin\rev-list.c rev-list.c 342
 uninitvar(CppCheck) \git-master\builtin\rev-list.c rev-list.c 342
 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c
2803
 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c
2802
 uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c
2805
 memleakOnRealloc(CppCheck) \git-master\compat\win32\syslog.c
syslog.c 46


This looks like a possible, based on 
http://bytes.com/topic/c/answers/215084-can-realloc-potentially-cause-memory-leak 
(Mac's reply, with tweaks)


Misuse of realloc CAN cause a memory leak, but only when allocation 
fails
if realloc fails, the memory previously pointed to by 'str = 
realloc(str, ++str_len + 1)' will still be claimed, but you will have 
lost your only pointer to it, because realloc returns NULL on failure. 
This is a memory leak.


We (those using the compat function) then only provide a warning, so it 
could repeat endlessly.


Eric (cc'd) may be able to clarify if this is a possibility.


 uninitvar(CppCheck)
\git-master\contrib\examples\builtin-fetch--tool.c 
builtin-fetch--tool.c

419
 uninitvar(CppCheck) \git-master\fast-import.c fast-import.c 2917
 nullPointer(CppCheck) \git-master\line-log.c line-log.c 638
 nullPointer(CppCheck) \git-master\mailmap.c mailmap.c 156
 uninitvar(CppCheck) \git-master\merge-recursive.c
merge-recursive.c 1887
 uninitvar(CppCheck) \git-master\notes.c notes.c 805
 uninitvar(CppCheck) \git-master\notes.c notes.c 805
 deallocret(CppCheck) \git-master\pretty.c pretty.c 677
 resourceLeak(CppCheck) \git-master\refs.c refs.c 3041
 doubleFree(CppCheck) \git-master\sequencer.c sequencer.c 924
 nullPointer(CppCheck) \git-master\sha1_file.c sha1_file.c 125
 doubleFree(CppCheck) \git-master\shell.c shell.c 130

--
Philip 


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: CPPCheck found 24 high risk bugs in Git v.1.8.3.4

[Erik Faye-Lund]

This one seems real, although it's quite theoretical. It should only happen
in cases where the log-message contains %1, the initial malloc passed and
reallocing two more bytes failed.

However, what's much more of a disaster: pos is used after the call to
realloc might have moved the memory!

I guess something like this should fix both issues. Sorry about the
lack of indentation, it seems Gmail has regressed, and the old compose
mode is somehow gone... (also sorry for triple-posting to some of you,
Gmail seems particularly broken today)

diff --git a/compat/win32/syslog.c b/compat/win32/syslog.c
index d015e43..0641f4e 100644
--- a/compat/win32/syslog.c
+++ b/compat/win32/syslog.c
@@ -43,11 +43,14 @@ void syslog(int priority, const char *fmt, ...)
  va_end(ap);

  while ((pos = strstr(str, %1)) != NULL) {
- str = realloc(str, ++str_len + 1);
- if (!str) {
+ char *tmp = realloc(str, ++str_len + 1);
+ if (!tmp) {
  warning(realloc failed: '%s', strerror(errno));
+ free(str);
  return;
  }
+ pos = tmp + (pos - str);
+ str = tmp;
  memmove(pos + 2, pos + 1, strlen(pos));
  pos[1] = ' ';
  }



On Tue, Aug 20, 2013 at 12:55 AM, Philip Oakley philipoak...@iee.org wrote:
 - Original Message - From: Philip Oakley philipoak...@iee.org

 From: Koch, Rick (Subcontractor) rick.k...@tbe.com
 Sent: Monday, August 19, 2013 6:09 PM

 I'm directing to this e-mail, as it seems to be the approved forum
 for posting Git bugs. We ran CPPCheck against Git v.1.8.3.4
 and found 24 high risk bugs. Please see the attachment xlsx.


 Is there a method to post to the Git community to allow the
 community to review and debunk as faults positive or develop
 patches to fix lists code files?


 v/r


 Roderick (Rick) Koch
 Information Assurance
 rick.k...@tbe.com


 What OS version / CPPCheck version was this checked on?

 In case other readers don't have a .xlsx reader here is Rick's list in
 plain text (may be white space damaged).

 I expect some will be false positives, and some will just be being too
 cautious.

 Philip

 description resourceFilePath fileName lineNumber
  nullPointer(CppCheck) \git-master\builtin\add.c add.c 286
  wrongPrintfScanfArgNum(CppCheck) \git-master\builtin\fetch.c
 fetch.c 588
  nullPointer(CppCheck) \git-master\builtin\ls-files.c ls-files.c
 144
  nullPointer(CppCheck) \git-master\builtin\merge.c merge.c 1208
  doubleFree(CppCheck) \git-master\builtin\notes.c notes.c 275
  nullPointer(CppCheck) \git-master\builtin\reflog.c reflog.c 437
  uninitvar(CppCheck) \git-master\builtin\rev-list.c rev-list.c 342
  uninitvar(CppCheck) \git-master\builtin\rev-list.c rev-list.c 342
  uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c
 2803
  uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c
 2802
  uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c
 2805
  memleakOnRealloc(CppCheck) \git-master\compat\win32\syslog.c
 syslog.c 46


 This looks like a possible, based on
 http://bytes.com/topic/c/answers/215084-can-realloc-potentially-cause-memory-leak
 (Mac's reply, with tweaks)

 Misuse of realloc CAN cause a memory leak, but only when allocation fails
 if realloc fails, the memory previously pointed to by 'str = realloc(str,
 ++str_len + 1)' will still be claimed, but you will have lost your only
 pointer to it, because realloc returns NULL on failure. This is a memory
 leak.

 We (those using the compat function) then only provide a warning, so it
 could repeat endlessly.

 Eric (cc'd) may be able to clarify if this is a possibility.


  uninitvar(CppCheck)
 \git-master\contrib\examples\builtin-fetch--tool.c builtin-fetch--tool.c
 419
  uninitvar(CppCheck) \git-master\fast-import.c fast-import.c 2917
  nullPointer(CppCheck) \git-master\line-log.c line-log.c 638
  nullPointer(CppCheck) \git-master\mailmap.c mailmap.c 156
  uninitvar(CppCheck) \git-master\merge-recursive.c
 merge-recursive.c 1887
  uninitvar(CppCheck) \git-master\notes.c notes.c 805
  uninitvar(CppCheck) \git-master\notes.c notes.c 805
  deallocret(CppCheck) \git-master\pretty.c pretty.c 677
  resourceLeak(CppCheck) \git-master\refs.c refs.c 3041
  doubleFree(CppCheck) \git-master\sequencer.c sequencer.c 924
  nullPointer(CppCheck) \git-master\sha1_file.c sha1_file.c 125
  doubleFree(CppCheck) \git-master\shell.c shell.c 130

 --

 Philip
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCHv4] repack: rewrite the shell script in C.

[Stefan Beller]

Hi,

so today I compared the argument lists of the repack shell script with the
C rewrite passed on to the pack-objects command and fixed some corner 
cases (-A -d --unpack-unreachable=%s should only pass 
--unpack-unreachable=%s once to pack-objects)
Also I fixed some missing smaller options (--quiet, --no-reuse-delta, 
--no-reuse-object).

I have run the test suite several times successfully now,
trying to find a pattern for the non-deterministically bug, which appears to
only occur in 1 out of 4 test suite runs.

The test suite has around 40 calls to repack and 35 calls to gc, which calls
repack internally. That alone covers quite a lot of the repack options,
but the debugging for the test cases is no fun, as the the calls to repack
are usually not the main concern of the respective tests.

There are however 
  t7700-repack.sh 
  t7701-repack-unpack-unreachable.sh
  
It was suggested earlier, and I think it's a good idea to enhance those
tests.

Anyway, here is an updated version of the repack rewrite.

Stefan

--8--
From f6da16ac3ca71aa746fe6d9224b06e6cc4e7a104 Mon Sep 17 00:00:00 2001
From: Stefan Beller stefanbel...@googlemail.com
Date: Fri, 16 Aug 2013 02:08:47 +0200
Subject: [RFC PATCHv4] repack: rewrite the shell script in C.

This is the beginning of the rewrite of the repacking.

All tests have been positive at least once now.
However there is still a non-deterministic error occuring in
about 1 out of 4 test suite runs (usually in 7701 or 9301,
but could also occur in 5501 or 3306 iirc)

Signed-off-by: Stefan Beller stefanbel...@googlemail.com
---
 Makefile|   2 +-
 builtin.h   |   1 +
 builtin/repack.c| 363 
 git-repack.sh = contrib/examples/git-repack.sh |   0
 git.c   |   1 +
 5 files changed, 366 insertions(+), 1 deletion(-)
 create mode 100644 builtin/repack.c
 rename git-repack.sh = contrib/examples/git-repack.sh (100%)

diff --git a/Makefile b/Makefile
index ef442eb..4ec5bbe 100644
--- a/Makefile
+++ b/Makefile
@@ -464,7 +464,6 @@ SCRIPT_SH += git-pull.sh
 SCRIPT_SH += git-quiltimport.sh
 SCRIPT_SH += git-rebase.sh
 SCRIPT_SH += git-remote-testgit.sh
-SCRIPT_SH += git-repack.sh
 SCRIPT_SH += git-request-pull.sh
 SCRIPT_SH += git-stash.sh
 SCRIPT_SH += git-submodule.sh
@@ -972,6 +971,7 @@ BUILTIN_OBJS += builtin/reflog.o
 BUILTIN_OBJS += builtin/remote.o
 BUILTIN_OBJS += builtin/remote-ext.o
 BUILTIN_OBJS += builtin/remote-fd.o
+BUILTIN_OBJS += builtin/repack.o
 BUILTIN_OBJS += builtin/replace.o
 BUILTIN_OBJS += builtin/rerere.o
 BUILTIN_OBJS += builtin/reset.o
diff --git a/builtin.h b/builtin.h
index 8afa2de..b56cf07 100644
--- a/builtin.h
+++ b/builtin.h
@@ -102,6 +102,7 @@ extern int cmd_reflog(int argc, const char **argv, const 
char *prefix);
 extern int cmd_remote(int argc, const char **argv, const char *prefix);
 extern int cmd_remote_ext(int argc, const char **argv, const char *prefix);
 extern int cmd_remote_fd(int argc, const char **argv, const char *prefix);
+extern int cmd_repack(int argc, const char **argv, const char *prefix);
 extern int cmd_repo_config(int argc, const char **argv, const char *prefix);
 extern int cmd_rerere(int argc, const char **argv, const char *prefix);
 extern int cmd_reset(int argc, const char **argv, const char *prefix);
diff --git a/builtin/repack.c b/builtin/repack.c
new file mode 100644
index 000..a87900e
--- /dev/null
+++ b/builtin/repack.c
@@ -0,0 +1,363 @@
+/*
+ * The shell version was written by Linus Torvalds (2005) and many others.
+ * This is a translation into C by Stefan Beller (2013)
+ */
+
+#include builtin.h
+#include cache.h
+#include dir.h
+#include parse-options.h
+#include run-command.h
+#include sigchain.h
+#include strbuf.h
+#include string-list.h
+#include argv-array.h
+
+static int delta_base_offset = 0;
+
+static const char *const git_repack_usage[] = {
+   N_(git repack [options]),
+   NULL
+};
+
+static int repack_config(const char *var, const char *value, void *cb)
+{
+   if (!strcmp(var, repack.usedeltabaseoffset)) {
+   delta_base_offset = git_config_bool(var, value);
+   return 0;
+   }
+   return git_default_config(var, value, cb);
+}
+
+static void remove_temporary_files() {
+   DIR *dir;
+   struct dirent *e;
+   char *prefix, *path;
+
+   prefix = mkpathdup(.tmp-%d-pack, getpid());
+   path = mkpathdup(%s/pack, get_object_directory());
+
+   dir = opendir(path);
+   while ((e = readdir(dir)) != NULL) {
+   if (!prefixcmp(e-d_name, prefix)) {
+   struct strbuf fname = STRBUF_INIT;
+   strbuf_addf(fname, %s/%s, path, e-d_name);
+   unlink(strbuf_detach(fname, NULL));
+   }
+   }
+   free(prefix);
+   free(path);
+   closedir(dir);
+}
+
+static void remove_pack_on_signal(int 

Re: [PATCH] Git segmentation faults if submodule path is empty.

[Jharrod LaFon]

It looks great to me.

Thanks,

--
Jharrod LaFon
OpenEye Scientific Software

On Aug 19, 2013, at 2:54 PM, Junio C Hamano gits...@pobox.com wrote:

 Jharrod LaFon jla...@eyesopen.com writes:
 
 I will keep trying this until it's perfect, and I thank you for
 the help.  When I resubmit this, would you like me to include your
 sign-off line as well?
 
 If the one I attached at the end of the message you are responding
 to looks fine to you, I'd just apply it without having you to
 reroll.
 
 Also, the end of the test script was not included in your message.  
 
 Sorry, but I am not sure what you meant by this.
 
 I reworked your example to make it the second test in an existing
 test script.  There are many existing tests after that so it is
 natural that we wouldn't see the end of the file in the patch
 context.
 
 diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
 index 5ee97b0..a39d074 100755
 --- a/t/t7400-submodule-basic.sh
 +++ b/t/t7400-submodule-basic.sh
 @@ -18,6 +18,16 @@ test_expect_success 'setup - initial commit' '
 git branch initial
 '
 
 +test_expect_success 'configuration parsing' '
 +   test_when_finished rm -f .gitmodules 
 +   cat .gitmodules -\EOF 
 +   [submodule s]
 +   path
 +   ignore
 +   EOF
 +   test_must_fail git status
 +'
 +
 test_expect_success 'setup - repository in init subdirectory' '
 mkdir init 
 (

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] submodule: prevent warning in summary output

[brian m. carlson]

When git submodule summary is run and there is a deleted submodule, there is an
warning from git rev-parse:

  fatal: Not a git repository: '.vim/pathogen/.git'

Silence this warning, since it is fully expected that a deleted submodule will
not be a git repository.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---

I hesitated to add the test for $status because it will end up having no effect
since we exclude that case later.  However, for correctness, I included it.

 git-submodule.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/git-submodule.sh b/git-submodule.sh
index 2979197..eec3135 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1070,7 +1070,10 @@ cmd_summary() {
missing_src=
missing_dst=
 
+   test $status = D  missing_src=t
+
test $mod_src = 16 
+   test -e $name/.git 
! GIT_DIR=$name/.git git-rev-parse -q --verify $sha1_src^0 
/dev/null 
missing_src=t
 
-- 
1.8.4.rc3

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Does Git now have any C struct version history tracking mechanism?

[Nazri Ramliy]

On Mon, Aug 19, 2013 at 4:37 PM, Thomas Rast tr...@inf.ethz.ch wrote:
 Hmm, IIUC that's actually not a bug or even a roughness; it's an
 artifact of how the :pattern:file syntax is defined.  It takes the first
 _funcname line_ matching 'pattern', up to (but excluding) the next
 funcname line.

...
 Or is there an issue that I'm missing?

No you're correct. I should have used the following instead for the demo:

$ git log -L /struct\ rev_cmdline_info/,/^}/:revision.h

Moral: You'd hit your target better if you work on your aim.

nazri
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Document smart http

[Nguyễn Thái Ngọc Duy]

This may provide some clues for those who want to modify smart http
code as smart http is pretty much undocumented. Smart http document
so far is a few commit messages and the source code.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/git-fetch-pack.txt  | 11 +++--
 Documentation/git-receive-pack.txt| 12 +-
 Documentation/git-send-pack.txt   |  9 +++-
 Documentation/git-upload-pack.txt |  9 +++-
 Documentation/technical/pack-protocol.txt | 69 +++
 5 files changed, 103 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-fetch-pack.txt b/Documentation/git-fetch-pack.txt
index 1e71754..85a9437 100644
--- a/Documentation/git-fetch-pack.txt
+++ b/Documentation/git-fetch-pack.txt
@@ -9,10 +9,7 @@ git-fetch-pack - Receive missing objects from another 
repository
 SYNOPSIS
 
 [verse]
-'git fetch-pack' [--all] [--quiet|-q] [--keep|-k] [--thin] [--include-tag]
-   [--upload-pack=git-upload-pack]
-   [--depth=n] [--no-progress]
-   [-v] [host:]directory [refs...]
+'git fetch-pack' [options] [host:]directory [refs...]
 
 DESCRIPTION
 ---
@@ -90,6 +87,12 @@ be in a separate packet, and the list must end with a flush 
packet.
 --no-progress::
Do not show the progress.
 
+--stateless-rpc::
+   Smart HTTP mode.
+
+--lock-pack::
+   Issue lock command to the remote helper via stdout.
+
 -v::
Run verbosely.
 
diff --git a/Documentation/git-receive-pack.txt 
b/Documentation/git-receive-pack.txt
index b1f7dc6..0b2029c 100644
--- a/Documentation/git-receive-pack.txt
+++ b/Documentation/git-receive-pack.txt
@@ -9,7 +9,7 @@ git-receive-pack - Receive what is pushed into the repository
 SYNOPSIS
 
 [verse]
-'git-receive-pack' directory
+'git-receive-pack' [options] directory
 
 DESCRIPTION
 ---
@@ -35,6 +35,16 @@ are not fast-forwards.
 
 OPTIONS
 ---
+--stateless-rpc::
+   Smart HTTP mode.
+
+--advertise-refs::
+   Only the initial ref advertisement is output then exits
+   immediately.
+
+--quiet::
+   Make unpack-objects at the receive-pack end quiet.
+
 directory::
The repository to sync into.
 
diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
index dc3a568..6cee3d4 100644
--- a/Documentation/git-send-pack.txt
+++ b/Documentation/git-send-pack.txt
@@ -9,7 +9,7 @@ git-send-pack - Push objects over Git protocol to another 
repository
 SYNOPSIS
 
 [verse]
-'git send-pack' [--all] [--dry-run] [--force] 
[--receive-pack=git-receive-pack] [--verbose] [--thin] [host:]directory 
[ref...]
+'git send-pack' [options] [host:]directory [ref...]
 
 DESCRIPTION
 ---
@@ -52,6 +52,13 @@ OPTIONS
Send a thin pack, which records objects in deltified form based
on objects not included in the pack to reduce network traffic.
 
+--stateless-rpc::
+   Smart HTTP mode.
+
+--helper-status:
+   Issue status commands (e.g. ok or error) to the remote
+   helper via stdout.
+
 host::
A remote host to house the repository.  When this
part is specified, 'git-receive-pack' is invoked via
diff --git a/Documentation/git-upload-pack.txt 
b/Documentation/git-upload-pack.txt
index 0abc806..ce9a455 100644
--- a/Documentation/git-upload-pack.txt
+++ b/Documentation/git-upload-pack.txt
@@ -9,7 +9,7 @@ git-upload-pack - Send objects packed back to git-fetch-pack
 SYNOPSIS
 
 [verse]
-'git-upload-pack' [--strict] [--timeout=n] directory
+'git-upload-pack' [options] directory
 
 DESCRIPTION
 ---
@@ -31,6 +31,13 @@ OPTIONS
 --timeout=n::
Interrupt transfer after n seconds of inactivity.
 
+--stateless-rpc::
+   Smart HTTP mode.
+
+--advertise-refs::
+   Only the initial ref advertisement is output then exits
+   immediately.
+
 directory::
The repository to sync from.
 
diff --git a/Documentation/technical/pack-protocol.txt 
b/Documentation/technical/pack-protocol.txt
index b898e97..7590394 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -546,3 +546,72 @@ An example client/server communication might look like 
this:
S: 0018ok refs/heads/debug\n
S: 002ang refs/heads/master non-fast-forward\n
 
+
+Smart HTTP Transport
+
+
+Smart HTTP protocol is basically git protocol on top of http. The
+base protocol is modified slightly to fit HTTP processing model: no
+bidirectional full-duplex connections, the program may read the
+request, write a response and must exit. Any negotiation is broken
+down into many separate GET or POST requests. The server is
+stateless. The client must send enough information so that the server
+can recreate the previous state.
+
+Reference Discovery
+---
+
+The server end always sends the list of references in both push and
+fetch cases. This ref list is retrieved by the client's sending HTTP
+GET request to a smart http