Re: [PATCH v4 1/2] refs.c: optimize check_refname_component()

2014-06-01 Thread Andreas Schwab
David Turner dtur...@twopensource.com writes:

 +static unsigned char refname_disposition[] = {
 + 1, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9,
 + 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9,
 + 9, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, 0, 0, 0, 2, 1,
 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, 0, 0, 0, 0, 9,
 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, 9, 0, 9, 0,
 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 9, 9
 +};

 + unsigned char ch = (unsigned char) *cp;
 + char disp = refname_disposition[ch];

Undefined behaviour.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.
--
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: Reset by checkout?

2014-06-01 Thread Kevin Bracey

On 01/06/2014 07:26, Atsushi Nakagawa wrote:

Kevin Bracey ke...@bracey.fi wrote:

The original git reset --hard used to be a pretty top-level command.
It was used for aborting merges in particular. But I think it now
stands out as being one of the only really dangerous porcelain
commands, and I can't think of any real workflow it's still useful
for.

My thoughts exactly.  I think the 'reset --soft/--mixed/--hard' pattern
is so ingrained, that many people just don't realize there's a safer
alternative.  (I've heard work mates on more than one occasion
recommending 'reset --hard' as the go-to command for discarding commits.)

I believe this is likely because many third party GUI tools just don't
support 'reset --keep', and these tools present a Reset... dialog with
the de facto Soft/Mixed/Hard options.  (Even 'gitk' does this.)

True on the GUI - hard really needs demotion.

It would help if the documentation explained better straight off what 
the different reset modes are intended /for/ in a more practical way, 
rather than the technical jargon.


There is the EXAMPLES section, but I think the problem is that it's 
not clearly laid out by mode, meaning people checking to see what git 
reset can do are inclined to go first to the --xxx mode list in 
DESCRIPTION, and stop there, baffled, probably not finding any example 
for that mode. Maybe the examples should have clearer --option 
subheadings? (And all the existing examples for --hard should really 
suggest --keep instead).


But given that the DISCUSSION section now has the full internal 
details on what exactly each mode does in every state, and now that we 
have more than the simple soft/mixed/hard to deal with, I think the 
main DESCRIPTION could be simplified for end users.


Most useful for visualisation, I feel, would just showing what git 
status will look like afterwards, primarily from the point of view of a 
backwards reset to HEAD~n. In particular, normal users don't think in 
terms of the absolute contents of the index, but rather in terms of diffs.


Maybe something like this:

All modes move the current branch pointer so that HEAD now points to 
the specified commit. ORIG_HEAD is set to the original HEAD location. 
The modes differ in what happens to the contents of ORIG_HEAD, that are 
no longer on the reset branch; and also what happens to your 
not-yet-committed changes.


--soft
 Retains the contents of ORIG_HEAD in the index+work area, leaving 
the difference as changes to be committed. git reset --soft HEAD~1 
would be the first step if you want to remove the last commit, but 
intend to recommit most or all of its changes.


git status after reset --soft shows:

  To be committed:
   Changes in ORIG_HEAD relative to HEAD
   (+Any initial staged changes)

  Not staged:
   (Any initial unstaged changes)

--mixed (default)
Retains the contents of ORIG_HEAD in the work area, leaving the 
difference as unstaged changes. git reset HEAD~1 would be the first 
step if you want to remove the last commit, and think again from scratch 
about which of its changes should be committed.


git status after reset --mixed shows:

   Not staged:
   Changes in ORIG_HEAD relative to HEAD
   (+Any initial staged changes)
   (+Any initial unstaged changes)

--keep
   The contents of ORIG_HEAD are dropped, leaving the work area and 
index containing the new HEAD; your uncommitted changes to unaffected 
files are retained. If you have uncommitted changes to any files that 
differ in the proposed new HEAD, the operation is refused; you would 
need to git stash first. git reset --keep HEAD~1 can be used to 
totally remove the last commit. (This removal can itself be undone with 
another git reset --keep ORIG_HEAD, or git reset --keep 
branch@{n} - see git-reflog(1)). git reset --keep is a safe 
alternative to --hard, and is roughly equivalent to git checkout -B 
current-branch-name.


git status after reset --keep shows:

   Not staged
   (Any initial staged changes)  [should these be left staged, as 
per git checkout?]

   (+Any initial unstaged changes)

--hard
   All other changes are dropped, and the work area and index are 
forcibly reset to the new HEAD. Note that this is dangerous if used 
carelessly: ALL uncommitted changes to ALL tracked files will be lost, 
even if you were only trying to drop an unrelated commit that didn't 
touch those files. Older documentation often recommends git reset 
--hard to undo commits; the newer --keep option is a much better 
alternative in almost all cases.


git status after reset --hard shows:

   Work area clean (or untracked files present)

--merge
   Performs the operation of git merge --abort, intended for use 
during a merge resolution - see git-merge(1) for more information. This 
form is not normally used directly. [Not really true? Still the best 
command to abort git checkout --merge/git stash pop|apply? Do those 
need --abort?]




If people just forgot about 

[PATCH] mailinfo: use strcmp() for string comparison

2014-06-01 Thread René Scharfe
The array header is defined as:

static const char *header[MAX_HDR_PARSED] = {
 From,Subject,Date,
};

When looking for the index of a specfic string in that array, simply
use strcmp() instead of memcmp().  This avoids running over the end of
the string (e.g. with memcmp(Subject, From, 7)) and gets rid of
magic string length constants.

Signed-off-by: Rene Scharfe l@web.de
---
This is a minimal fix.  A good question, however, would be: Why do we
keep on looking up constant strings in a (short) constant string array
anyway?

 builtin/mailinfo.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 2c3cd8e..cf11c8d 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -334,7 +334,7 @@ static int check_header(const struct strbuf *line,
}
if (starts_with(line-buf, [PATCH])  isspace(line-buf[7])) {
for (i = 0; header[i]; i++) {
-   if (!memcmp(Subject, header[i], 7)) {
+   if (!strcmp(Subject, header[i])) {
handle_header(hdr_data[i], line);
ret = 1;
goto check_header_out;
@@ -929,13 +929,13 @@ static void handle_info(void)
else
continue;
 
-   if (!memcmp(header[i], Subject, 7)) {
+   if (!strcmp(header[i], Subject)) {
if (!keep_subject) {
cleanup_subject(hdr);
cleanup_space(hdr);
}
output_header_lines(fout, Subject, hdr);
-   } else if (!memcmp(header[i], From, 4)) {
+   } else if (!strcmp(header[i], From)) {
cleanup_space(hdr);
handle_from(hdr);
fprintf(fout, Author: %s\n, name.buf);
-- 
2.0.0

--
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] pack-objects: use free()+xcalloc() instead of xrealloc()+memset()

2014-06-01 Thread René Scharfe
Whenever the hash table becomes too small then its size is increased,
the original part (and the added space) is zerod out using memset(),
and the table is rebuilt from scratch.

Simplify this proceess by returning the old memory using free() and
allocating the new buffer using xcalloc(), which already clears the
buffer for us.  That way we avoid copying the old hash table contents
needlessly inside xrealloc().

While at it, use the first array member with sizeof instead of a
specific type.  The old code used uint32_t and int, while index is
actually an array of int32_t.  Their sizes are the same basically
everywhere, so it's not actually a problem, but the new code is
cleaner and doesn't have to be touched should the type be changed.

Signed-off-by: Rene Scharfe l@web.de
---
 pack-objects.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pack-objects.c b/pack-objects.c
index d01d851..4f36c32 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -47,8 +47,8 @@ static void rehash_objects(struct packing_data *pdata)
if (pdata-index_size  1024)
pdata-index_size = 1024;
 
-   pdata-index = xrealloc(pdata-index, sizeof(uint32_t) * 
pdata-index_size);
-   memset(pdata-index, 0, sizeof(int) * pdata-index_size);
+   free(pdata-index);
+   pdata-index = xcalloc(pdata-index_size, sizeof(*pdata-index));
 
entry = pdata-objects;
 
-- 
2.0.0

--
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 2/4] replace: add test for --graft

2014-06-01 Thread Christian Couder
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 12 
 1 file changed, 12 insertions(+)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 68b3cb2..ca45a84 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -351,4 +351,16 @@ test_expect_success 'replace ref cleanup' '
test -z $(git replace)
 '
 
+test_expect_success '--graft with and without already replaced object' '
+   test $(git log --oneline | wc -l) = 7 
+   git replace --graft $HASH5 
+   test $(git log --oneline | wc -l) = 3 
+   git cat-file -p $HASH5 | test_must_fail grep parent 
+   test_must_fail git replace --graft $HASH5 $HASH4 $HASH3 
+   git replace --force -g $HASH5 $HASH4 $HASH3 
+   git cat-file -p $HASH5 | grep parent $HASH4 
+   git cat-file -p $HASH5 | grep parent $HASH3 
+   git replace -d $HASH5
+'
+
 test_done
-- 
2.0.0.rc0.40.gd30ccc4


--
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 0/4] Add --graft option to git replace

2014-06-01 Thread Christian Couder
Here is a small patch series to implement:

git replace [-f] --graft commit [parent...]

This patch series goes on top of the patch series that
implements --edit.

The changes since v1 are the following thanks to Eric,
Junio and Peff:

- change commit message of patch 1/4
- added patch 4/4 that was previously sent separately
- in patch 4/4 rename the existing grafts file
  into grafts.bak

Christian Couder (4):
  replace: add --graft option
  replace: add test for --graft
  Documentation: replace: add --graft option
  contrib: add convert-grafts-to-replace-refs.sh

 Documentation/git-replace.txt | 10 
 builtin/replace.c | 84 ++-
 contrib/convert-grafts-to-replace-refs.sh | 29 +++
 t/t6050-replace.sh| 12 +
 4 files changed, 134 insertions(+), 1 deletion(-)
 create mode 100755 contrib/convert-grafts-to-replace-refs.sh

-- 
2.0.0.rc0.40.gd30ccc4

--
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 1/4] replace: add --graft option

2014-06-01 Thread Christian Couder
The usage string for this option is:

git replace [-f] --graft commit [parent...]

First we create a new commit that is the same as commit
except that its parents are [parents...]

Then we create a replace ref that replace commit with
the commit we just created.

With this new option, it should be straightforward to
convert grafts to replace refs.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 builtin/replace.c | 84 ++-
 1 file changed, 83 insertions(+), 1 deletion(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index 1bb491d..9d1e82f 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -17,6 +17,7 @@
 static const char * const git_replace_usage[] = {
N_(git replace [-f] object replacement),
N_(git replace [-f] --edit object),
+   N_(git replace [-f] --graft commit [parent...]),
N_(git replace -d object...),
N_(git replace [--format=format] [-l [pattern]]),
NULL
@@ -294,6 +295,76 @@ static int edit_and_replace(const char *object_ref, int 
force)
return replace_object_sha1(object_ref, old, replacement, new, force);
 }
 
+static int read_sha1_commit(const unsigned char *sha1, struct strbuf *dst)
+{
+   void *buf;
+   enum object_type type;
+   unsigned long size;
+
+   buf = read_sha1_file(sha1, type, size);
+   if (!buf)
+   return error(_(cannot read object %s), sha1_to_hex(sha1));
+   if (type != OBJ_COMMIT) {
+   free(buf);
+   return error(_(object %s is not a commit), sha1_to_hex(sha1));
+   }
+   strbuf_attach(dst, buf, size, size + 1);
+   return 0;
+}
+
+static int create_graft(int argc, const char **argv, int force)
+{
+   unsigned char old[20], new[20];
+   const char *old_ref = argv[0];
+   struct commit *commit;
+   struct strbuf buf = STRBUF_INIT;
+   struct strbuf new_parents = STRBUF_INIT;
+   const char *parent_start, *parent_end;
+   int i;
+
+   if (get_sha1(old_ref, old)  0)
+   die(_(Not a valid object name: '%s'), old_ref);
+   commit = lookup_commit_or_die(old, old_ref);
+   if (read_sha1_commit(old, buf))
+   die(_(Invalid commit: '%s'), old_ref);
+
+   /* make sure the commit is not corrupt */
+   if (parse_commit_buffer(commit, buf.buf, buf.len))
+   die(_(Could not parse commit: '%s'), old_ref);
+
+   /* find existing parents */
+   parent_start = buf.buf;
+   parent_start += 46; /* tree  + hex sha1 + \n */
+   parent_end = parent_start;
+
+   while (starts_with(parent_end, parent ))
+   parent_end += 48; /* parent  + hex sha1 + \n */
+
+   /* prepare new parents */
+   for (i = 1; i  argc; i++) {
+   unsigned char sha1[20];
+   if (get_sha1(argv[i], sha1)  0)
+   die(_(Not a valid object name: '%s'), argv[i]);
+   lookup_commit_or_die(sha1, argv[i]);
+   strbuf_addf(new_parents, parent %s\n, sha1_to_hex(sha1));
+   }
+
+   /* replace existing parents with new ones */
+   strbuf_splice(buf, parent_start - buf.buf, parent_end - parent_start,
+ new_parents.buf, new_parents.len);
+
+   if (write_sha1_file(buf.buf, buf.len, commit_type, new))
+   die(_(could not write replacement commit for: '%s'), old_ref);
+
+   strbuf_release(new_parents);
+   strbuf_release(buf);
+
+   if (!hashcmp(old, new))
+   return error(new commit is the same as the old one: '%s', 
sha1_to_hex(old));
+
+   return replace_object_sha1(old_ref, old, replacement, new, force);
+}
+
 int cmd_replace(int argc, const char **argv, const char *prefix)
 {
int force = 0;
@@ -303,12 +374,14 @@ int cmd_replace(int argc, const char **argv, const char 
*prefix)
MODE_LIST,
MODE_DELETE,
MODE_EDIT,
+   MODE_GRAFT,
MODE_REPLACE
} cmdmode = MODE_UNSPECIFIED;
struct option options[] = {
OPT_CMDMODE('l', list, cmdmode, N_(list replace refs), 
MODE_LIST),
OPT_CMDMODE('d', delete, cmdmode, N_(delete replace refs), 
MODE_DELETE),
OPT_CMDMODE('e', edit, cmdmode, N_(edit existing object), 
MODE_EDIT),
+   OPT_CMDMODE('g', graft, cmdmode, N_(change a commit's 
parents), MODE_GRAFT),
OPT_BOOL('f', force, force, N_(replace the ref if it 
exists)),
OPT_STRING(0, format, format, N_(format), N_(use this 
format)),
OPT_END()
@@ -325,7 +398,10 @@ int cmd_replace(int argc, const char **argv, const char 
*prefix)
usage_msg_opt(--format cannot be used when not listing,
  git_replace_usage, options);
 
-   if (force  cmdmode != MODE_REPLACE  cmdmode != MODE_EDIT)
+   if (force 
+   cmdmode != MODE_REPLACE 
+   

[PATCH v2 3/4] Documentation: replace: add --graft option

2014-06-01 Thread Christian Couder
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt | 8 
 1 file changed, 8 insertions(+)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index 61461b9..491875e 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -10,6 +10,7 @@ SYNOPSIS
 [verse]
 'git replace' [-f] object replacement
 'git replace' [-f] --edit object
+'git replace' [-f] --graft commit [parent...]
 'git replace' -d object...
 'git replace' [--format=format] [-l [pattern]]
 
@@ -73,6 +74,13 @@ OPTIONS
newly created object. See linkgit:git-var[1] for details about
how the editor will be chosen.
 
+--graft commit [parent...]::
+   Create a graft commit. A new commit is created with the same
+   content as commit except that its parents will be
+   [parent...] instead of commit's parents. A replacement ref
+   is then created to replace commit with the newly created
+   commit.
+
 -l pattern::
 --list pattern::
List replace refs for objects that match the given pattern (or
-- 
2.0.0.rc0.40.gd30ccc4


--
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 4/4] contrib: add convert-grafts-to-replace-refs.sh

2014-06-01 Thread Christian Couder
This patch adds into contrib/ an example script to convert
grafts from an existing grafts file into replace refs using
the new --graft option of git replace.

While at it let's mention this new script in the
git replace documentation for the --graft option.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt |  4 +++-
 contrib/convert-grafts-to-replace-refs.sh | 29 +
 2 files changed, 32 insertions(+), 1 deletion(-)
 create mode 100755 contrib/convert-grafts-to-replace-refs.sh

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index 491875e..e1be2e6 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -79,7 +79,9 @@ OPTIONS
content as commit except that its parents will be
[parent...] instead of commit's parents. A replacement ref
is then created to replace commit with the newly created
-   commit.
+   commit. See contrib/convert-grafts-to-replace-refs.sh for an
+   example script based on this option that can convert grafts to
+   replace refs.
 
 -l pattern::
 --list pattern::
diff --git a/contrib/convert-grafts-to-replace-refs.sh 
b/contrib/convert-grafts-to-replace-refs.sh
new file mode 100755
index 000..7718a53
--- /dev/null
+++ b/contrib/convert-grafts-to-replace-refs.sh
@@ -0,0 +1,29 @@
+#!/bin/sh
+
+# You should execute this script in the repository where you
+# want to convert grafts to replace refs.
+
+die () {
+   echo 2 $@
+   exit 1
+}
+
+GRAFTS_FILE=${GIT_DIR:-.git}/info/grafts
+
+test -f $GRAFTS_FILE || die Could not find graft file: '$GRAFTS_FILE'
+
+grep '^[^# ]' $GRAFTS_FILE | while read definition
+do
+   test -n $definition  {
+   echo Converting: $definition
+   git replace --graft $definition ||
+   die Convertion failed for: $definition
+   }
+done
+
+mv $GRAFTS_FILE $GRAFTS_FILE.bak ||
+   die Could not mv '$GRAFTS_FILE' to '$GRAFTS_FILE.bak'
+
+echo Success!
+echo All the grafts in '$GRAFTS_FILE' have been converted to replace refs!
+echo The grafts file '$GRAFTS_FILE' has been renamed: '$GRAFTS_FILE.bak'
-- 
2.0.0.rc0.40.gd30ccc4

--
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 v1 1/3] replace: add --graft option

2014-06-01 Thread Christian Couder
From: Jeff King p...@peff.net

 On Thu, May 22, 2014 at 11:33:04PM +0200, Christian Couder wrote:
 
 The usage string for this option is:
 
 git replace [-f] --graft commit [parent...]
 
 First we create a new commit that is the same as commit
 except that its parents are [parents...]
 
 Then we create a replace ref that replace commit with
 the commit we just created.
 
 With this new option, it should be straightforward to
 convert grafts to replace refs, with something like:
 
 cat .git/info/grafts | while read line
 do git replace --graft $line; done
 
 I think this script at the end should end up in the documentation or a
 contrib script (probably the former, as it is short enough that somebody
 might just cut-and-paste).
 
 The graft file ignores comments and blank lines, so maybe grep '^[^#]'
 would be in order.
 
 And maybe it's just me, but I think spacing it like:
 
   grep '^[^#]' .git/info/grafts |
   while read line; do
   git replace --graft $line
   done
 
 is more readable (I think some would even argue for putting the do on
 a separate line).

Thanks, I used something like that in the contrib script.
 
 +/* make sure the commit is not corrupt */
 +if (parse_commit_buffer(commit, buf.buf, buf.len))
 +die(_(Could not parse commit: '%s'), old_ref);
 
 I guess the checks here are sufficient to make...
 
 +/* find existing parents */
 +parent_start = buf.buf;
 +parent_start += 46; /* tree  + hex sha1 + \n */
 +parent_end = parent_start;
 +
 +while (starts_with(parent_end, parent ))
 +parent_end += 48; /* parent  + hex sha1 + \n */
 
 ...this number-based parsing safe, though it would miss removing a stray
 parent line elsewhere in the commit.

Yeah, but I don't think that it is a problem.

Those parent lines are not standard in the first place, because they
are not parsed by parse_commit_buffer(). And I don't think this option
should mess with non standard stuff.

 It still feels rather magical to
 me, as we are depending on unspoken format guarantees defined inside
 parse_commit_buffer. 

My opinion is that we are depending on the standard way to parse
headers, and that's good. I think it would be black magic to mess
with non standard stuff.

 I'd prefer something like the line-based parser I
 showed in the other thread, but I suppose it may just be a matter of
 preference.

Yeah probably.

Thanks,
Christian.
--
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 2/9] connect.c: replace signal() with sigaction()

2014-06-01 Thread Jeremiah Mahler
From the signal(2) man page:

  The behavior of signal() varies across UNIX versions, and has also var‐
  ied historically across different versions of Linux.   Avoid  its  use:
  use sigaction(2) instead.

Replaced signal() with sigaction() in connect.c

Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
 connect.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/connect.c b/connect.c
index a983d06..1dc51b2 100644
--- a/connect.c
+++ b/connect.c
@@ -665,11 +665,14 @@ struct child_process *git_connect(int fd[2], const char 
*url,
enum protocol protocol;
const char **arg;
struct strbuf cmd = STRBUF_INIT;
+   struct sigaction sa;
 
/* Without this we cannot rely on waitpid() to tell
 * what happened to our children.
 */
-   signal(SIGCHLD, SIG_DFL);
+   memset(sa, 0, sizeof(sa));
+   sa.sa_handler = SIG_DFL;
+   sigaction(SIGCHLD, sa, NULL);
 
protocol = parse_connect_url(url, hostandport, path);
if (flags  CONNECT_DIAG_URL) {
-- 
2.0.0.8.g7bf6e1f.dirty

--
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 1/9] compat/mingw.c: expand MinGW support for sigaction

2014-06-01 Thread Jeremiah Mahler
Due to portability issues across UNIX versions sigaction(2) should be used
instead of signal(2).

From the signal(2) man page:

  The behavior of signal() varies across UNIX versions, and has also var‐
  ied historically across different versions of Linux.   Avoid  its  use:
  use sigaction(2) instead.

Unfortunately MinGW under Windows has limited support for signal and no
support for sigaction.  And this prevents sigaction from being used across
the entire Git project.

In compat/mingw.c there is a faux sigaction function but it only supports
SIGALARM.  Hence the need for continuing to use signal() in other cases.

This patch expands the faux sigaction function so that it calls signal in
cases other than SIGALRM.  Now sigaction can be used across the entire Git
project and MinGW will still work with signal as it did before.

Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
 compat/mingw.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index e9892f8..e504cef 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1651,14 +1651,15 @@ int setitimer(int type, struct itimerval *in, struct 
itimerval *out)
 
 int sigaction(int sig, struct sigaction *in, struct sigaction *out)
 {
-   if (sig != SIGALRM)
-   return errno = EINVAL,
-   error(sigaction only implemented for SIGALRM);
if (out != NULL)
return errno = EINVAL,
error(sigaction: param 3 != NULL not implemented);
 
-   timer_fn = in-sa_handler;
+   if (sig == SIGALRM)
+   timer_fn = in-sa_handler;
+   else
+   signal(sig, in-sa_handler);
+
return 0;
 }
 
-- 
2.0.0.8.g7bf6e1f.dirty

--
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 0/9] replace signal() with sigaction()

2014-06-01 Thread Jeremiah Mahler
This is version 3 of the patch set to convert signal(2) to sigaction(2)
(previous discussion [1]).

[1]: http://marc.info/?l=gitm=140148352416926w=2

Changes in this revision include:

  - Using NULL pointers instead of 0 as per the
Documentation/CodingGuidlines pointed out by Chris Packham.

sigaction(SIGCHLD, sa, NULL);

  - Conversion of all remaining files which used signal().

  - sigchain.c required the most changes.  Both the old signal handler
was used and the return value from signal() was being checked.
signal() would return the previous error handler which would be
SIG_ERR if an error occurred.  sigaction() just returns -1 in this
case.

Jeremiah Mahler (9):
  compat/mingw.c: expand MinGW support for sigaction
  connect.c: replace signal() with sigaction()
  progress.c: replace signal() with sigaction()
  write_or_die.c: replace signal() with sigaction()
  daemon.c: replace signal() with sigaction()
  builtin/log.c: replace signal() with sigaction()
  builtin/merge-index.c: replace signal() with sigaction()
  builtin/verify-tag.c: replace signal() with sigaction()
  sigchain.c: replace signal() with sigaction()

 builtin/log.c |  6 +-
 builtin/merge-index.c |  5 -
 builtin/verify-tag.c  |  5 -
 compat/mingw.c|  9 +
 connect.c |  5 -
 daemon.c  | 16 +---
 progress.c|  6 +-
 sigchain.c| 14 +++---
 write_or_die.c|  6 +-
 9 files changed, 56 insertions(+), 16 deletions(-)

-- 
2.0.0.8.g7bf6e1f.dirty

--
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 4/9] write_or_die.c: replace signal() with sigaction()

2014-06-01 Thread Jeremiah Mahler
From the signal(2) man page:

  The behavior of signal() varies across UNIX versions, and has also var‐
  ied historically across different versions of Linux.   Avoid  its  use:
  use sigaction(2) instead.

Replaced signal() with sigaction() in write_or_die.c

Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
 write_or_die.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/write_or_die.c b/write_or_die.c
index b50f99a..f5fdec8 100644
--- a/write_or_die.c
+++ b/write_or_die.c
@@ -2,8 +2,12 @@
 
 static void check_pipe(int err)
 {
+   struct sigaction sa;
+
if (err == EPIPE) {
-   signal(SIGPIPE, SIG_DFL);
+   memset(sa, 0, sizeof(sa));
+   sa.sa_handler = SIG_DFL;
+   sigaction(SIGPIPE, sa, NULL);
raise(SIGPIPE);
/* Should never happen, but just in case... */
exit(141);
-- 
2.0.0.8.g7bf6e1f.dirty

--
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 3/9] progress.c: replace signal() with sigaction()

2014-06-01 Thread Jeremiah Mahler
From the signal(2) man page:

  The behavior of signal() varies across UNIX versions, and has also var‐
  ied historically across different versions of Linux.   Avoid  its  use:
  use sigaction(2) instead.

Replaced signal() with sigaction() in progress.c

Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
 progress.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/progress.c b/progress.c
index 261314e..ff676bc 100644
--- a/progress.c
+++ b/progress.c
@@ -66,8 +66,12 @@ static void set_progress_signal(void)
 static void clear_progress_signal(void)
 {
struct itimerval v = {{0,},};
+   struct sigaction sa;
+
setitimer(ITIMER_REAL, v, NULL);
-   signal(SIGALRM, SIG_IGN);
+   memset(sa, 0, sizeof(sa));
+   sa.sa_handler = SIG_IGN;
+   sigaction(SIGALRM, sa, NULL);
progress_update = 0;
 }
 
-- 
2.0.0.8.g7bf6e1f.dirty

--
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 5/9] daemon.c: replace signal() with sigaction()

2014-06-01 Thread Jeremiah Mahler
From the signal(2) man page:

  The behavior of signal() varies across UNIX versions, and has also var‐
  ied historically across different versions of Linux.   Avoid  its  use:
  use sigaction(2) instead.

Replaced signal() with sigaction() in daemon.c

Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
 daemon.c | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/daemon.c b/daemon.c
index eba1255..615426e 100644
--- a/daemon.c
+++ b/daemon.c
@@ -322,6 +322,7 @@ static int run_service(char *dir, struct daemon_service 
*service)
 {
const char *path;
int enabled = service-enabled;
+   struct sigaction sa;
 
loginfo(Request %s for '%s', service-name, dir);
 
@@ -376,7 +377,9 @@ static int run_service(char *dir, struct daemon_service 
*service)
 * We'll ignore SIGTERM from now on, we have a
 * good client.
 */
-   signal(SIGTERM, SIG_IGN);
+   memset(sa, 0, sizeof(sa));
+   sa.sa_handler = SIG_IGN;
+   sigaction(SIGTERM, sa, NULL);
 
return service-fn();
 }
@@ -788,12 +791,16 @@ static void handle(int incoming, struct sockaddr *addr, 
socklen_t addrlen)
 
 static void child_handler(int signo)
 {
+   struct sigaction sa;
+
/*
 * Otherwise empty handler because systemcalls will get interrupted
 * upon signal receipt
 * SysV needs the handler to be rearmed
 */
-   signal(SIGCHLD, child_handler);
+   memset(sa, 0, sizeof(sa));
+   sa.sa_handler = child_handler;
+   sigaction(SIGCHLD, sa, NULL);
 }
 
 static int set_reuse_addr(int sockfd)
@@ -996,6 +1003,7 @@ static int service_loop(struct socketlist *socklist)
 {
struct pollfd *pfd;
int i;
+   struct sigaction sa;
 
pfd = xcalloc(socklist-nr, sizeof(struct pollfd));
 
@@ -1004,7 +1012,9 @@ static int service_loop(struct socketlist *socklist)
pfd[i].events = POLLIN;
}
 
-   signal(SIGCHLD, child_handler);
+   memset(sa, 0, sizeof(sa));
+   sa.sa_handler = child_handler;
+   sigaction(SIGCHLD, sa, NULL);
 
for (;;) {
int i;
-- 
2.0.0.8.g7bf6e1f.dirty

--
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 6/9] builtin/log.c: replace signal() with sigaction()

2014-06-01 Thread Jeremiah Mahler
From the signal(2) man page:

  The behavior of signal() varies across UNIX versions, and has also var‐
  ied historically across different versions of Linux.   Avoid  its  use:
  use sigaction(2) instead.

Replaced signal() with sigaction() in builtin/log.c

Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
 builtin/log.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/builtin/log.c b/builtin/log.c
index 39e8836..f1deea1 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -315,7 +315,11 @@ static void setup_early_output(struct rev_info *rev)
 static void finish_early_output(struct rev_info *rev)
 {
int n = estimate_commit_count(rev, rev-commits);
-   signal(SIGALRM, SIG_IGN);
+   struct sigaction sa;
+
+   memset(sa, 0, sizeof(sa));
+   sa.sa_handler = SIG_IGN;
+   sigaction(SIGALRM, sa, NULL);
show_early_header(rev, done, n);
 }
 
-- 
2.0.0.8.g7bf6e1f.dirty

--
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 9/9] sigchain.c: replace signal() with sigaction()

2014-06-01 Thread Jeremiah Mahler
From the signal(2) man page:

  The behavior of signal() varies across UNIX versions, and has also var‐
  ied historically across different versions of Linux.   Avoid  its  use:
  use sigaction(2) instead.

Replaced signal() with sigaction() in sigchain.c

Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
 sigchain.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/sigchain.c b/sigchain.c
index 1118b99..deab262 100644
--- a/sigchain.c
+++ b/sigchain.c
@@ -19,11 +19,16 @@ static void check_signum(int sig)
 int sigchain_push(int sig, sigchain_fun f)
 {
struct sigchain_signal *s = signals + sig;
+   struct sigaction sa, sa_old;
+   int result;
check_signum(sig);
 
ALLOC_GROW(s-old, s-n + 1, s-alloc);
-   s-old[s-n] = signal(sig, f);
-   if (s-old[s-n] == SIG_ERR)
+   memset(sa, 0, sizeof(sa));
+   sa.sa_handler = f;
+   result = sigaction(sig, sa, sa_old);
+   s-old[s-n] = sa_old.sa_handler;
+   if (result == -1)
return -1;
s-n++;
return 0;
@@ -32,11 +37,14 @@ int sigchain_push(int sig, sigchain_fun f)
 int sigchain_pop(int sig)
 {
struct sigchain_signal *s = signals + sig;
+   struct sigaction sa, sa_old;
check_signum(sig);
if (s-n  1)
return 0;
 
-   if (signal(sig, s-old[s-n - 1]) == SIG_ERR)
+   memset(sa, 0, sizeof(sa));
+   sa.sa_handler = s-old[s-n - 1];
+   if (sigaction(sig, sa, sa_old) == -1)
return -1;
s-n--;
return 0;
-- 
2.0.0.8.g7bf6e1f.dirty

--
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 8/9] builtin/verify-tag.c: replace signal() with sigaction()

2014-06-01 Thread Jeremiah Mahler
From the signal(2) man page:

  The behavior of signal() varies across UNIX versions, and has also var‐
  ied historically across different versions of Linux.   Avoid  its  use:
  use sigaction(2) instead.

Replaced signal() with sigaction() in builtin/verify-tag.c

Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
 builtin/verify-tag.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 9cdf332..d5ccbad 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -73,6 +73,7 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
OPT__VERBOSE(verbose, N_(print tag contents)),
OPT_END()
};
+   struct sigaction sa;
 
git_config(git_verify_tag_config, NULL);
 
@@ -83,7 +84,9 @@ int cmd_verify_tag(int argc, const char **argv, const char 
*prefix)
 
/* sometimes the program was terminated because this signal
 * was received in the process of writing the gpg input: */
-   signal(SIGPIPE, SIG_IGN);
+   memset(sa, 0, sizeof(sa));
+   sa.sa_handler = SIG_IGN;
+   sigaction(SIGPIPE, sa, NULL);
while (i  argc)
if (verify_tag(argv[i++], verbose))
had_error = 1;
-- 
2.0.0.8.g7bf6e1f.dirty

--
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 7/9] builtin/merge-index.c: replace signal() with sigaction()

2014-06-01 Thread Jeremiah Mahler
From the signal(2) man page:

  The behavior of signal() varies across UNIX versions, and has also var‐
  ied historically across different versions of Linux.   Avoid  its  use:
  use sigaction(2) instead.

Replaced signal() with sigaction() in builtin/merge-index.c

Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
 builtin/merge-index.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/builtin/merge-index.c b/builtin/merge-index.c
index b416d92..25db374 100644
--- a/builtin/merge-index.c
+++ b/builtin/merge-index.c
@@ -68,11 +68,14 @@ static void merge_all(void)
 int cmd_merge_index(int argc, const char **argv, const char *prefix)
 {
int i, force_file = 0;
+   struct sigaction sa;
 
/* Without this we cannot rely on waitpid() to tell
 * what happened to our children.
 */
-   signal(SIGCHLD, SIG_DFL);
+   memset(sa, 0, sizeof(sa));
+   sa.sa_handler = SIG_DFL;
+   sigaction(SIGCHLD, sa, NULL);
 
if (argc  3)
usage(git merge-index [-o] [-q] merge-program (-a | [--] 
filename*));
-- 
2.0.0.8.g7bf6e1f.dirty

--
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 4/4] contrib: add convert-grafts-to-replace-refs.sh

2014-06-01 Thread Eric Sunshine
On Sun, Jun 1, 2014 at 11:10 AM, Christian Couder
chrisc...@tuxfamily.org wrote:
 This patch adds into contrib/ an example script to convert
 grafts from an existing grafts file into replace refs using
 the new --graft option of git replace.

 While at it let's mention this new script in the
 git replace documentation for the --graft option.

 Signed-off-by: Christian Couder chrisc...@tuxfamily.org
 ---
 diff --git a/contrib/convert-grafts-to-replace-refs.sh 
 b/contrib/convert-grafts-to-replace-refs.sh
 new file mode 100755
 index 000..7718a53
 --- /dev/null
 +++ b/contrib/convert-grafts-to-replace-refs.sh
 @@ -0,0 +1,29 @@
 +#!/bin/sh
 +
 +# You should execute this script in the repository where you
 +# want to convert grafts to replace refs.
 +
 +die () {
 +   echo 2 $@
 +   exit 1
 +}
 +
 +GRAFTS_FILE=${GIT_DIR:-.git}/info/grafts
 +
 +test -f $GRAFTS_FILE || die Could not find graft file: '$GRAFTS_FILE'
 +
 +grep '^[^# ]' $GRAFTS_FILE | while read definition
 +do
 +   test -n $definition  {
 +   echo Converting: $definition
 +   git replace --graft $definition ||
 +   die Convertion failed for: $definition

s/Convertion/Conversion/  [1]

[1]: 
http://git.661346.n2.nabble.com/Re-PATCH-contrib-add-convert-grafts-to-replace-refs-sh-tp7611822.html

 +   }
 +done
 +
 +mv $GRAFTS_FILE $GRAFTS_FILE.bak ||
 +   die Could not mv '$GRAFTS_FILE' to '$GRAFTS_FILE.bak'

Could not rename... might be a bit more friendly to non-Unixy folk.

 +echo Success!
 +echo All the grafts in '$GRAFTS_FILE' have been converted to replace refs!
 +echo The grafts file '$GRAFTS_FILE' has been renamed: '$GRAFTS_FILE.bak'
 --
 2.0.0.rc0.40.gd30ccc4
--
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 1/2] refs.c: optimize check_refname_component()

2014-06-01 Thread Philip Oakley

From: David Turner dtur...@twopensource.com

In a repository with many refs, check_refname_component can be a major
contributor to the runtime of some git commands. One such command is

git rev-parse HEAD

Timings for one particular repo, with about 60k refs, almost all
packed, are:

Old: 35 ms
New: 29 ms

Many other commands which read refs are also sped up.

Signed-off-by: David Turner dtur...@twitter.com
---
refs.c | 68 
--

1 file changed, 41 insertions(+), 27 deletions(-)

diff --git a/refs.c b/refs.c
index 28d5eca..62e2301 100644
--- a/refs.c
+++ b/refs.c
@@ -5,9 +5,32 @@
#include dir.h
#include string-list.h

+/* How to handle various characters in refnames:
+ * 0: An acceptable character for refs
+ * 1: End-of-component
+ * 2: ., look for a following . to reject .. in refs
+ * 3: @, look for a following { to reject @{ in refs
+ * 9: A bad character, reject ref
+ *
+ * See below for the list of illegal characters, from which
+ * this table is derived.


The see below doesn't quite scan right.  Perhaps
   The character handling rules above are used to
   derive the table below.


+ */
+static unsigned char refname_disposition[] = {
+ 1, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9,
+ 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9,
+ 9, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, 0, 0, 0, 2, 1,
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, 0, 0, 0, 0, 9,
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, 9, 0, 9, 0,
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 9, 9
+};
+
/*
- * Make sure ref is something reasonable to have under 
.git/refs/;

- * We do not like it if:
+ * Try to read one refname component from the front of refname.
+ * Return the length of the component found, or -1 if the component 
is
+ * not legal.  It is legal if it is something reasonable to have 
under

+ * .git/refs/; We do not like it if:
 *
 * - any path component of it begins with ., or
 * - it has double dots .., or
@@ -15,24 +38,7 @@
 * - it ends with a /.
 * - it ends with .lock
 * - it contains a \ (backslash)
- */

-/* Return true iff ch is not allowed in reference names. */
-static inline int bad_ref_char(int ch)
-{
- if (((unsigned) ch) = ' ' || ch == 0x7f ||
- ch == '~' || ch == '^' || ch == ':' || ch == '\\')
- return 1;
- /* 2.13 Pattern Matching Notation */
- if (ch == '*' || ch == '?' || ch == '[') /* Unsupported */
- return 1;
- return 0;
-}
-
-/*
- * Try to read one refname component from the front of refname. 
Return

- * the length of the component found, or -1 if the component is not
- * legal.
 */
static int check_refname_component(const char *refname, int flags)
{
@@ -40,17 +46,25 @@ static int check_refname_component(const char 
*refname, int flags)

 char last = '\0';

 for (cp = refname; ; cp++) {
- char ch = *cp;
- if (ch == '\0' || ch == '/')
+ unsigned char ch = (unsigned char) *cp;
+ char disp = refname_disposition[ch];
+ switch(disp) {
+ case 1:
+ goto out;
+ case 2:
+ if (last == '.')
+ return -1; /* Refname contains ... */
 break;
- if (bad_ref_char(ch))
- return -1; /* Illegal character in refname. */
- if (last == '.'  ch == '.')
- return -1; /* Refname contains ... */
- if (last == '@'  ch == '{')
- return -1; /* Refname contains @{. */
+ case 3:
+ if (last == '@')
+ return -1; /* Refname contains @{. */
+ break;
+ case 9:
+ return -1;
+ }
 last = ch;
 }
+out:
 if (cp == refname)
 return 0; /* Component has zero length. */
 if (refname[0] == '.') {
--
2.0.0.rc1.18.gf763c0f

--


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: [PATCH v4 1/2] refs.c: optimize check_refname_component()

2014-06-01 Thread Michael Haggerty
Thanks for splitting this up into two patches.  See my comments below.

On 06/01/2014 07:17 AM, David Turner wrote:
 In a repository with many refs, check_refname_component can be a major
 contributor to the runtime of some git commands. One such command is
 
 git rev-parse HEAD
 
 Timings for one particular repo, with about 60k refs, almost all
 packed, are:
 
 Old: 35 ms
 New: 29 ms
 
 Many other commands which read refs are also sped up.
 
 Signed-off-by: David Turner dtur...@twitter.com
 ---
  refs.c | 68 
 --
  1 file changed, 41 insertions(+), 27 deletions(-)
 
 diff --git a/refs.c b/refs.c
 index 28d5eca..62e2301 100644
 --- a/refs.c
 +++ b/refs.c
 @@ -5,9 +5,32 @@
  #include dir.h
  #include string-list.h
  
 +/* How to handle various characters in refnames:

We format multiline comments with no text on the opening line:

/*
 * How to handle...
 * ... is derived.
 */

 + * 0: An acceptable character for refs
 + * 1: End-of-component
 + * 2: ., look for a following . to reject .. in refs
 + * 3: @, look for a following { to reject @{ in refs
 + * 9: A bad character, reject ref

This explanation does not agree with the code (or I'm not reading it
correctly).  For example, the character with disposition 3 is '{', not
'@', so ISTM the comment should be

 * 3: {, look for a preceding @ to reject @{ in refnames

BTW, is there a special reason that the values in your table jump from 3
to 9?  I imagine that compilers would use a jump table to implement the
switch statement where these values are used, and they might have a
slightly easier time if there are no holes between the legal values.

 + *
 + * See below for the list of illegal characters, from which
 + * this table is derived.
 + */
 +static unsigned char refname_disposition[] = {

I think you need to define the length explicitly to 256 here to cause
the entries for 0x80..0xff to be created and initialized to zeros.

The fact that you didn't notice this bug suggests that there might be no
tests involving refnames with non-ASCII characters, which would be
another nice thing to remedy while you are working in this area.

 + 1, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9,
 + 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9, 9,
 + 9, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, 0, 0, 0, 2, 1,
 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, 0, 0, 0, 0, 9,
 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 9, 9, 0, 9, 0,
 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 9, 9
 +};
 +
  /*
 - * Make sure ref is something reasonable to have under .git/refs/;
 - * We do not like it if:
 + * Try to read one refname component from the front of refname.
 + * Return the length of the component found, or -1 if the component is
 + * not legal.  It is legal if it is something reasonable to have under
 + * .git/refs/; We do not like it if:
   *
   * - any path component of it begins with ., or
   * - it has double dots .., or
 @@ -15,24 +38,7 @@
   * - it ends with a /.
   * - it ends with .lock
   * - it contains a \ (backslash)
 - */
  

^^^ doesn't this leave a blank line inside the comment?

 -/* Return true iff ch is not allowed in reference names. */
 -static inline int bad_ref_char(int ch)
 -{
 - if (((unsigned) ch) = ' ' || ch == 0x7f ||
 - ch == '~' || ch == '^' || ch == ':' || ch == '\\')
 - return 1;
 - /* 2.13 Pattern Matching Notation */
 - if (ch == '*' || ch == '?' || ch == '[') /* Unsupported */
 - return 1;
 - return 0;
 -}
 -
 -/*
 - * Try to read one refname component from the front of refname.  Return
 - * the length of the component found, or -1 if the component is not
 - * legal.
   */
 [...]

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
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


[no subject]

2014-06-01 Thread C. Benson Manica
The documentation for installing git from source here,
http://git-scm.com/book/en/Getting-Started-Installing-Git, incorrectly
fails to mention that the MakeMaker perl module is also required and
is installable via

$ yum install perl-ExtUtils-MakeMaker

Also, you might want to let people know that you've configured your
mail system for 1987 mode and do not accept HTML-formatted mail.

-- 
C. Benson Manica
cbman...@gmail.com
--
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 2/2] userdiff: support Java try keyword

2014-06-01 Thread Sup Yut Sum
try keyword is enhanced in Java 7, see try-with-resources Statement
try (XX yy = new XX()) {
} catch (Exception e){
}

Signed-off-by: Sup Yut Sum ch3co...@gmail.com
---
 userdiff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/userdiff.c b/userdiff.c
index 96eda6c..49e898b 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -39,7 +39,7 @@ IPATTERN(fortran,
 PATTERNS(html, ^[ \t]*([Hh][1-6][ \t].*.*)$,
 [^= \t]+),
 PATTERNS(java,
-!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n
+!^[ 
\t]*(try|catch|do|for|if|instanceof|new|return|switch|throw|while)\n
 ^[ \t]*(([A-Za-z_][A-Za-z_0-9]*[ \t]+)+[A-Za-z_][A-Za-z_0-9]*[ 
\t]*\\([^;]*)$,
 /* -- */
 [a-zA-Z_][a-zA-Z0-9_]*
-- 
1.9.1

--
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 1/2] userdiff: support C# async methods and correct C# keywords

2014-06-01 Thread Sup Yut Sum
async is in C# 5.0
foreach is in C# 1.0
instanceof is in Java. The similar keywords are typeof, is, as in C# 1.0
default, try are in C# 1.0

Signed-off-by: Sup Yut Sum ch3co...@gmail.com
---
 userdiff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/userdiff.c b/userdiff.c
index fad52d6..96eda6c 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -134,9 +134,9 @@ PATTERNS(cpp,
 |[-+*/%^|=!]=|--|\\+\\+|=?|=?||\\|\\||::|-\\*?|\\.\\*),
 PATTERNS(csharp,
 /* Keywords */
-!^[ 
\t]*(do|while|for|if|else|instanceof|new|return|switch|case|throw|catch|using)\n
+!^[ 
\t]*(do|while|for|foreach|if|else|typeof|is|as|new|return|switch|case|default|throw|try|catch|using)\n
 /* Methods and constructors */
-^[ 
\t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[
 \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+[ \t]*\\(.*\\))[ \t]*$\n
+^[ 
\t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe|async)[
 \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+[ \t]*\\(.*\\))[ \t]*$\n
 /* Properties */
 ^[ 
\t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[
 \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+)[ \t]*$\n
 /* Type definitions */
-- 
1.9.1

--
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 1/9] compat/mingw.c: expand MinGW support for sigaction

2014-06-01 Thread Johannes Sixt
Am 6/1/2014 20:10, schrieb Jeremiah Mahler:
 Due to portability issues across UNIX versions sigaction(2) should be used
 instead of signal(2).
 
From the signal(2) man page:
 
   The behavior of signal() varies across UNIX versions, and has also var‐
   ied historically across different versions of Linux.   Avoid  its  use:
   use sigaction(2) instead.
 
 Unfortunately MinGW under Windows has limited support for signal and no
 support for sigaction.  And this prevents sigaction from being used across
 the entire Git project.
 
 In compat/mingw.c there is a faux sigaction function but it only supports
 SIGALARM.  Hence the need for continuing to use signal() in other cases.
 
 This patch expands the faux sigaction function so that it calls signal in
 cases other than SIGALRM.  Now sigaction can be used across the entire Git
 project and MinGW will still work with signal as it did before.
 
 Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
 ---
  compat/mingw.c | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)
 
 diff --git a/compat/mingw.c b/compat/mingw.c
 index e9892f8..e504cef 100644
 --- a/compat/mingw.c
 +++ b/compat/mingw.c
 @@ -1651,14 +1651,15 @@ int setitimer(int type, struct itimerval *in, struct 
 itimerval *out)
  
  int sigaction(int sig, struct sigaction *in, struct sigaction *out)
  {
 - if (sig != SIGALRM)
 - return errno = EINVAL,
 - error(sigaction only implemented for SIGALRM);
   if (out != NULL)
   return errno = EINVAL,
   error(sigaction: param 3 != NULL not implemented);

A fix for this missing implementation is needed before patch 9/9 can be
applied.

  
 - timer_fn = in-sa_handler;
 + if (sig == SIGALRM)
 + timer_fn = in-sa_handler;
 + else
 + signal(sig, in-sa_handler);
 +
   return 0;
  }
  
 

-- 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