[PATCH v4] Replace memcpy with hashcpy when dealing hash copy globally
hashcpy() can keep the abstraction of object name behind it. Use it instead of memcpy() with hard-coded 20-byte length when moving object names between pieces of memory. We can benefit from it, when we switch to another hash algorithm, eg. MD5, by just fixing the hashcpy(). Leave ppc/sha1.c as it is, because the function is about the SHA-1 hash algorithm whose output is and will always be 20-byte. Helped-by: Michael Haggerty mhag...@alum.mit.edu Helped-by: Duy Nguyen pclo...@gmail.com Helped-by: Junio C Hamano gits...@pobox.com Signed-off-by: Sun He sunheeh...@gmail.com --- PATCH v4 changed the reason why should take this patch as tutored by Junio C Hamano. Thanks to Junio C Hamano PATCH v3 delete the one-space indentation on each line of commit message as is suggested by Eric Sunshine. Thanks to Eric Sunshine. PATCH v2 leave ppc/sha1.c alone. The general rule is if cache.h or git-compat-util.h is included, it is the first #include, and system includes will be always in git-compat-tuil.h. via Duy Nguyen The change in PATCH v1 is not proper because I placed cache.h in the end. And adding it to the head is not a good way to achieve the goal, as is said above ---. Thanks to Duy Nguyen. Find the potential places with memcpy by the bash command: $ git grep 'memcpy.*20' Thanks to Junio C Hamano bundle.c| 2 +- grep.c | 2 +- pack-bitmap-write.c | 2 +- reflog-walk.c | 4 ++-- refs.c | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bundle.c b/bundle.c index e99065c..7809fbb 100644 --- a/bundle.c +++ b/bundle.c @@ -19,7 +19,7 @@ static void add_to_ref_list(const unsigned char *sha1, const char *name, list-list = xrealloc(list-list, list-alloc * sizeof(list-list[0])); } - memcpy(list-list[list-nr].sha1, sha1, 20); + hashcpy(list-list[list-nr].sha1, sha1); list-list[list-nr].name = xstrdup(name); list-nr++; } diff --git a/grep.c b/grep.c index c668034..f5101f7 100644 --- a/grep.c +++ b/grep.c @@ -1650,7 +1650,7 @@ void grep_source_init(struct grep_source *gs, enum grep_source_type type, break; case GREP_SOURCE_SHA1: gs-identifier = xmalloc(20); - memcpy(gs-identifier, identifier, 20); + hashcpy(gs-identifier, identifier); break; case GREP_SOURCE_BUF: gs-identifier = NULL; diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c index 1218bef..5f1791a 100644 --- a/pack-bitmap-write.c +++ b/pack-bitmap-write.c @@ -530,7 +530,7 @@ void bitmap_writer_finish(struct pack_idx_entry **index, header.version = htons(default_version); header.options = htons(flags | options); header.entry_count = htonl(writer.selected_nr); - memcpy(header.checksum, writer.pack_checksum, 20); + hashcpy(header.checksum, writer.pack_checksum); sha1write(f, header, sizeof(header)); dump_bitmap(f, writer.commits); diff --git a/reflog-walk.c b/reflog-walk.c index b2fbdb2..d490f7d 100644 --- a/reflog-walk.c +++ b/reflog-walk.c @@ -32,8 +32,8 @@ static int read_one_reflog(unsigned char *osha1, unsigned char *nsha1, sizeof(struct reflog_info)); } item = array-items + array-nr; - memcpy(item-osha1, osha1, 20); - memcpy(item-nsha1, nsha1, 20); + hashcpy(item-osha1, osha1); + hashcpy(item-nsha1, nsha1); item-email = xstrdup(email); item-timestamp = timestamp; item-tz = tz; diff --git a/refs.c b/refs.c index 89228e2..f90b7ea 100644 --- a/refs.c +++ b/refs.c @@ -1222,7 +1222,7 @@ static int resolve_gitlink_packed_ref(struct ref_cache *refs, if (ref == NULL) return -1; - memcpy(sha1, ref-u.value.sha1, 20); + hashcpy(sha1, ref-u.value.sha1); return 0; } -- 1.9.0.138.g2de3478.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 v5] Replace memcpy with hashcpy when dealing hash copy globally
We invented hashcpy() to keep the abstraction of object name behind it. Use it instead of calling memcpy() with hard-coded 20-byte length when moving object names between pieces of memory. Leave ppc/sha1.c as it is, because the function is about the SHA-1 hash algorithm whose output is and will always be 20-byte. Helped-by: Michael Haggerty mhag...@alum.mit.edu Helped-by: Duy Nguyen pclo...@gmail.com Helped-by: Junio C Hamano gits...@pobox.com Signed-off-by: Sun He sunheeh...@gmail.com --- PATCH v5 changed the reason why should take this patch as tutored by Junio C Hamano. Thanks to Junio C Hamano again. :-) PATCH v5 move the two line behind the --- We can benefit from it, when we switch to another hash algorithm, eg. MD5, by just updating the hashcpy(). PATCH v4 changed the reason why should take this patch as tutored by Junio C Hamano. Thanks to Junio C Hamano PATCH v3 delete the one-space indentation on each line of commit message as is suggested by Eric Sunshine. Thanks to Eric Sunshine. PATCH v2 leave ppc/sha1.c alone. The general rule is if cache.h or git-compat-util.h is included, it is the first #include, and system includes will be always in git-compat-tuil.h. via Duy Nguyen The change in PATCH v1 is not proper because I placed cache.h in the end. And adding it to the head is not a good way to achieve the goal, as is said above ---. Thanks to Duy Nguyen. Find the potential places with memcpy by the bash command: $ git grep 'memcpy.*20' Thanks to Junio C Hamano bundle.c| 2 +- grep.c | 2 +- pack-bitmap-write.c | 2 +- reflog-walk.c | 4 ++-- refs.c | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bundle.c b/bundle.c index e99065c..7809fbb 100644 --- a/bundle.c +++ b/bundle.c @@ -19,7 +19,7 @@ static void add_to_ref_list(const unsigned char *sha1, const char *name, list-list = xrealloc(list-list, list-alloc * sizeof(list-list[0])); } - memcpy(list-list[list-nr].sha1, sha1, 20); + hashcpy(list-list[list-nr].sha1, sha1); list-list[list-nr].name = xstrdup(name); list-nr++; } diff --git a/grep.c b/grep.c index c668034..f5101f7 100644 --- a/grep.c +++ b/grep.c @@ -1650,7 +1650,7 @@ void grep_source_init(struct grep_source *gs, enum grep_source_type type, break; case GREP_SOURCE_SHA1: gs-identifier = xmalloc(20); - memcpy(gs-identifier, identifier, 20); + hashcpy(gs-identifier, identifier); break; case GREP_SOURCE_BUF: gs-identifier = NULL; diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c index 1218bef..5f1791a 100644 --- a/pack-bitmap-write.c +++ b/pack-bitmap-write.c @@ -530,7 +530,7 @@ void bitmap_writer_finish(struct pack_idx_entry **index, header.version = htons(default_version); header.options = htons(flags | options); header.entry_count = htonl(writer.selected_nr); - memcpy(header.checksum, writer.pack_checksum, 20); + hashcpy(header.checksum, writer.pack_checksum); sha1write(f, header, sizeof(header)); dump_bitmap(f, writer.commits); diff --git a/reflog-walk.c b/reflog-walk.c index b2fbdb2..d490f7d 100644 --- a/reflog-walk.c +++ b/reflog-walk.c @@ -32,8 +32,8 @@ static int read_one_reflog(unsigned char *osha1, unsigned char *nsha1, sizeof(struct reflog_info)); } item = array-items + array-nr; - memcpy(item-osha1, osha1, 20); - memcpy(item-nsha1, nsha1, 20); + hashcpy(item-osha1, osha1); + hashcpy(item-nsha1, nsha1); item-email = xstrdup(email); item-timestamp = timestamp; item-tz = tz; diff --git a/refs.c b/refs.c index 89228e2..f90b7ea 100644 --- a/refs.c +++ b/refs.c @@ -1222,7 +1222,7 @@ static int resolve_gitlink_packed_ref(struct ref_cache *refs, if (ref == NULL) return -1; - memcpy(sha1, ref-u.value.sha1, 20); + hashcpy(sha1, ref-u.value.sha1); return 0; } -- 1.9.0.138.g2de3478.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] Setup.c: PATH_MAX is the length including the Nil
Signed-off-by: Sun He sunheeh...@gmail.com
---
Check the limit.h of linux and find out that the MACRO
#define PATH_MAX4096/* # chars in a path name including nul */
So if the magic number 40 is just the size it should be. (e.g. hash code)
It may bring bugs with the length(4056) of long name(gitdirenv).
As gitdirenv could be set by GIT_DIR_ENVIRONMENT.
If it is a bug, it will almost never occur.
But I need your help to know if there is the PATH_MAX of git is the mirror of
the
PATH_MAX of linux and if this fix is right?
If it was, there may be many places like PATH_MAX + 1 could be replaced by
just PATH_MAX. And there may be many places like this.
Cheers,
He Sun
setup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/setup.c b/setup.c
index cffb6d6..1511612 100644
--- a/setup.c
+++ b/setup.c
@@ -395,7 +395,7 @@ static const char *setup_explicit_git_dir(const char
*gitdirenv,
char *gitfile;
int offset;
- if (PATH_MAX - 40 strlen(gitdirenv))
+ if (PATH_MAX - 41 strlen(gitdirenv))
die('$%s' too big, GIT_DIR_ENVIRONMENT);
gitfile = (char*)read_gitfile(gitdirenv);
--
1.9.0.138.g2de3478.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] finish_tmp_packfile():use strbuf for pathname construction
The old version fixes a maximum length on the buffer, which could be a problem
if one is not certain of the length of get_object_directory().
Using strbuf can avoid the protential bug.
Helped-by: Michael Haggerty mhag...@alum.mit.edu
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Sun He sunheeh...@gmail.com
---
PATCH v3 adds the reason why we should apply this patch.
Thanks to Micheal Haggerty.
PATCH v3 transposes the space and comma before the third argument as
Eric Sunshine suggested to meet the style of existing code.
Thanks to Eric Sunshine.
PATCH v3 fixes the order of Helped-by and Signed-off-by.
PATCH v2 follows the suggestions of Eric Sunshine to use strbuf_setlen()
instead of strbuf_remove(), etc.
Thanks to Eric Sunshine.
This patch has assumed that you have already fix the bug of
tmpname in builtin/pack-objects.c:write_pack_file() warning()
builtin/pack-objects.c | 15 ++-
bulk-checkin.c | 8 +---
pack-write.c | 18 ++
pack.h | 2 +-
4 files changed, 22 insertions(+), 21 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index c733379..099d6ed 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -803,7 +803,7 @@ static void write_pack_file(void)
if (!pack_to_stdout) {
struct stat st;
- char tmpname[PATH_MAX];
+ struct strbuf tmpname = STRBUF_INIT;
/*
* Packs are runtime accessed in their mtime
@@ -826,23 +826,19 @@ static void write_pack_file(void)
tmpname, strerror(errno));
}
- /* Enough space for -sha-1.pack? */
- if (sizeof(tmpname) = strlen(base_name) + 50)
- die(pack base name '%s' too long, base_name);
- snprintf(tmpname, sizeof(tmpname), %s-, base_name);
+ strbuf_addf(tmpname, %s-, base_name);
if (write_bitmap_index) {
bitmap_writer_set_checksum(sha1);
bitmap_writer_build_type_index(written_list,
nr_written);
}
- finish_tmp_packfile(tmpname, pack_tmp_name,
+ finish_tmp_packfile(tmpname, pack_tmp_name,
written_list, nr_written,
pack_idx_opts, sha1);
if (write_bitmap_index) {
- char *end_of_name_prefix = strrchr(tmpname, 0);
- sprintf(end_of_name_prefix, %s.bitmap,
sha1_to_hex(sha1));
+ strbuf_addf(tmpname, %s.bitmap,
sha1_to_hex(sha1));
stop_progress(progress_state);
@@ -851,10 +847,11 @@ static void write_pack_file(void)
bitmap_writer_select_commits(indexed_commits,
indexed_commits_nr, -1);
bitmap_writer_build(to_pack);
bitmap_writer_finish(written_list, nr_written,
-tmpname,
write_bitmap_options);
+tmpname.buf,
write_bitmap_options);
write_bitmap_index = 0;
}
+ strbuf_release(tmpname);
free(pack_tmp_name);
puts(sha1_to_hex(sha1));
}
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 118c625..98e651c 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -4,6 +4,7 @@
#include bulk-checkin.h
#include csum-file.h
#include pack.h
+#include strbuf.h
static int pack_compression_level = Z_DEFAULT_COMPRESSION;
@@ -23,7 +24,7 @@ static struct bulk_checkin_state {
static void finish_bulk_checkin(struct bulk_checkin_state *state)
{
unsigned char sha1[20];
- char packname[PATH_MAX];
+ struct strbuf packname = STRBUF_INIT;
int i;
if (!state-f)
@@ -43,8 +44,8 @@ static void finish_bulk_checkin(struct bulk_checkin_state
*state)
close(fd);
}
- sprintf(packname, %s/pack/pack-, get_object_directory());
- finish_tmp_packfile(packname, state-pack_tmp_name,
+ strbuf_addf(packname, %s/pack/pack-, get_object_directory());
+ finish_tmp_packfile(packname, state-pack_tmp_name,
state-written, state-nr_written,
state-pack_idx_opts, sha1);
for (i = 0; i state-nr_written; i++)
@@ -54,6 +55,7 @@ clear_exit:
free(state-written);
memset(state, 0, sizeof(*state));
+ strbuf_release(packname);
/* Make objects we just wrote available
[PATCH v3] Replace memcpy with hashcpy when dealing hash copy globally
Replacing memcpy with hashcpy is more directly and elegant. Leave ppc/sha1.c alone, as it is an isolated component. Pull cache.h(actually ../cache.h) in just for one memcpy there is not proper. Helped-by: Michael Haggerty mhag...@alum.mit.edu Helped-by: Duy Nguyen pclo...@gmail.com Signed-off-by: Sun He sunheeh...@gmail.com --- PATCH v3 delete the one-space indentation on each line of commit message as is suggested by Eric Sunshine. Thanks to Eric Sunshine. PATCH v2 leave ppc/sha1.c alone. The general rule is if cache.h or git-compat-util.h is included, it is the first #include, and system includes will be always in git-compat-tuil.h. via Duy Nguyen The change in PATCH v1 is not proper because I placed cache.h in the end. And adding it to the head is not a good way to achieve the goal, as is said above ---. Thanks to Duy Nguyen. Find the potential places with memcpy by the bash command: $ find . | xargs grep memcpy.*\(.*20.*\) ppc/sha1.c doesn't include cache.h and it cannot use hashcpy(). So just leave memcpy(in ppc/sha1.c) alone. bundle.c| 2 +- grep.c | 2 +- pack-bitmap-write.c | 2 +- reflog-walk.c | 4 ++-- refs.c | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bundle.c b/bundle.c index e99065c..7809fbb 100644 --- a/bundle.c +++ b/bundle.c @@ -19,7 +19,7 @@ static void add_to_ref_list(const unsigned char *sha1, const char *name, list-list = xrealloc(list-list, list-alloc * sizeof(list-list[0])); } - memcpy(list-list[list-nr].sha1, sha1, 20); + hashcpy(list-list[list-nr].sha1, sha1); list-list[list-nr].name = xstrdup(name); list-nr++; } diff --git a/grep.c b/grep.c index c668034..f5101f7 100644 --- a/grep.c +++ b/grep.c @@ -1650,7 +1650,7 @@ void grep_source_init(struct grep_source *gs, enum grep_source_type type, break; case GREP_SOURCE_SHA1: gs-identifier = xmalloc(20); - memcpy(gs-identifier, identifier, 20); + hashcpy(gs-identifier, identifier); break; case GREP_SOURCE_BUF: gs-identifier = NULL; diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c index 1218bef..5f1791a 100644 --- a/pack-bitmap-write.c +++ b/pack-bitmap-write.c @@ -530,7 +530,7 @@ void bitmap_writer_finish(struct pack_idx_entry **index, header.version = htons(default_version); header.options = htons(flags | options); header.entry_count = htonl(writer.selected_nr); - memcpy(header.checksum, writer.pack_checksum, 20); + hashcpy(header.checksum, writer.pack_checksum); sha1write(f, header, sizeof(header)); dump_bitmap(f, writer.commits); diff --git a/reflog-walk.c b/reflog-walk.c index b2fbdb2..d490f7d 100644 --- a/reflog-walk.c +++ b/reflog-walk.c @@ -32,8 +32,8 @@ static int read_one_reflog(unsigned char *osha1, unsigned char *nsha1, sizeof(struct reflog_info)); } item = array-items + array-nr; - memcpy(item-osha1, osha1, 20); - memcpy(item-nsha1, nsha1, 20); + hashcpy(item-osha1, osha1); + hashcpy(item-nsha1, nsha1); item-email = xstrdup(email); item-timestamp = timestamp; item-tz = tz; diff --git a/refs.c b/refs.c index 89228e2..f90b7ea 100644 --- a/refs.c +++ b/refs.c @@ -1222,7 +1222,7 @@ static int resolve_gitlink_packed_ref(struct ref_cache *refs, if (ref == NULL) return -1; - memcpy(sha1, ref-u.value.sha1, 20); + hashcpy(sha1, ref-u.value.sha1); return 0; } -- 1.9.0.138.g2de3478.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] Place cache.h at the first place to match general rule
The general rule is if cache.h or git-compat-util.h is included,
it is the first #include.
As builtin.h starts with git-compat-util.h, files that start with builtin.h
are not changed.
Helped-by: Duy Nguyen pclo...@gmail.com
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Sun He sunheeh...@gmail.com
---
PATCH v3 fix the position of information I want to convey to readers,
with the directions of Eric Sunshine.
sigchain.c and test-sigchain.c are started with sigchain.h
I checked sigchain.h, and it didn't import any bug.
But to keep consistant with general rule, we should take this patch.
sigchain.c | 2 +-
test-sigchain.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sigchain.c b/sigchain.c
index 1118b99..faa375d 100644
--- a/sigchain.c
+++ b/sigchain.c
@@ -1,5 +1,5 @@
-#include sigchain.h
#include cache.h
+#include sigchain.h
#define SIGCHAIN_MAX_SIGNALS 32
diff --git a/test-sigchain.c b/test-sigchain.c
index 42db234..e499fce 100644
--- a/test-sigchain.c
+++ b/test-sigchain.c
@@ -1,5 +1,5 @@
-#include sigchain.h
#include cache.h
+#include sigchain.h
#define X(f) \
static void f(int sig) { \
--
1.9.0.138.g2de3478.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 v2] Replace memcpy with hashcpy when dealing hash copy globally
Replacing memcpy with hashcpy is more directly and elegant. Leave ppc/sha1.c alone, as it is an isolated component. Pull cache.h(actually ../cache.h) in just for one memcpy there is not proper. Helped-by: Michael Haggerty mhag...@alum.mit.edu Helped-by: Duy Nguyen pclo...@gmail.com Signed-off-by: Sun He sunheeh...@gmail.com --- PATCH v2 leave ppc/sha1.c alone. The general rule is if cache.h or git-compat-util.h is included, it is the first #include, and system includes will be always in git-compat-tuil.h. via Duy Nguyen The change in PATCH v1 is not proper because I placed cache.h in the end. And adding it to the head is not a good way to achieve the goal, as is said above ---. Thanks to Duy Nguyen. Find the potential places with memcpy by the bash command: $ find . | xargs grep memcpy.*\(.*20.*\) ppc/sha1.c doesn't include cache.h and it cannot use So just leave memcpy(in ppc/sha1.c) alone bundle.c| 2 +- grep.c | 2 +- pack-bitmap-write.c | 2 +- reflog-walk.c | 4 ++-- refs.c | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bundle.c b/bundle.c index e99065c..7809fbb 100644 --- a/bundle.c +++ b/bundle.c @@ -19,7 +19,7 @@ static void add_to_ref_list(const unsigned char *sha1, const char *name, list-list = xrealloc(list-list, list-alloc * sizeof(list-list[0])); } - memcpy(list-list[list-nr].sha1, sha1, 20); + hashcpy(list-list[list-nr].sha1, sha1); list-list[list-nr].name = xstrdup(name); list-nr++; } diff --git a/grep.c b/grep.c index c668034..f5101f7 100644 --- a/grep.c +++ b/grep.c @@ -1650,7 +1650,7 @@ void grep_source_init(struct grep_source *gs, enum grep_source_type type, break; case GREP_SOURCE_SHA1: gs-identifier = xmalloc(20); - memcpy(gs-identifier, identifier, 20); + hashcpy(gs-identifier, identifier); break; case GREP_SOURCE_BUF: gs-identifier = NULL; diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c index 1218bef..5f1791a 100644 --- a/pack-bitmap-write.c +++ b/pack-bitmap-write.c @@ -530,7 +530,7 @@ void bitmap_writer_finish(struct pack_idx_entry **index, header.version = htons(default_version); header.options = htons(flags | options); header.entry_count = htonl(writer.selected_nr); - memcpy(header.checksum, writer.pack_checksum, 20); + hashcpy(header.checksum, writer.pack_checksum); sha1write(f, header, sizeof(header)); dump_bitmap(f, writer.commits); diff --git a/reflog-walk.c b/reflog-walk.c index b2fbdb2..d490f7d 100644 --- a/reflog-walk.c +++ b/reflog-walk.c @@ -32,8 +32,8 @@ static int read_one_reflog(unsigned char *osha1, unsigned char *nsha1, sizeof(struct reflog_info)); } item = array-items + array-nr; - memcpy(item-osha1, osha1, 20); - memcpy(item-nsha1, nsha1, 20); + hashcpy(item-osha1, osha1); + hashcpy(item-nsha1, nsha1); item-email = xstrdup(email); item-timestamp = timestamp; item-tz = tz; diff --git a/refs.c b/refs.c index 89228e2..f90b7ea 100644 --- a/refs.c +++ b/refs.c @@ -1222,7 +1222,7 @@ static int resolve_gitlink_packed_ref(struct ref_cache *refs, if (ref == NULL) return -1; - memcpy(sha1, ref-u.value.sha1, 20); + hashcpy(sha1, ref-u.value.sha1); return 0; } -- 1.9.0.138.g2de3478.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] Place cache.h at the first place to match generl rule
Signed-off-by: Sun He sunheeh...@gmail.com
Helped-by: Duy Nguyen pclo...@gmail.com
---
The general rule is if cache.h or git-compat-util.h is included,
it is the first #include.
I parsed all the source files, and find many files start with builtin.h.
And git-compat-util.h is the first in it. So they don't need any change.
sigchain.c and test-sigchain.c are started with sigchain.h
I checked sigchain.h, and it didn't import any bug.
But to keep consistant with general rule, we should take this patch.
Thanks.
sigchain.c | 2 +-
test-sigchain.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sigchain.c b/sigchain.c
index 1118b99..faa375d 100644
--- a/sigchain.c
+++ b/sigchain.c
@@ -1,5 +1,5 @@
-#include sigchain.h
#include cache.h
+#include sigchain.h
#define SIGCHAIN_MAX_SIGNALS 32
diff --git a/test-sigchain.c b/test-sigchain.c
index 42db234..e499fce 100644
--- a/test-sigchain.c
+++ b/test-sigchain.c
@@ -1,5 +1,5 @@
-#include sigchain.h
#include cache.h
+#include sigchain.h
#define X(f) \
static void f(int sig) { \
--
1.9.0.138.g2de3478.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 v2] Place cache.h at the first place to match general rule
Signed-off-by: Sun He sunheeh...@gmail.com
Helped-by: Duy Nguyen pclo...@gmail.com
---
PATCH v2 Fix the spelling bug of general in subject as is suggested
by brain m.calson sand...@crustytoothpaste.net
The general rule is if cache.h or git-compat-util.h is included,
it is the first #include.
I parsed all the source files, and find many files start with builtin.h.
And git-compat-util.h is the first in it. So they don't need any change.
sigchain.c and test-sigchain.c are started with sigchain.h
I checked sigchain.h, and it didn't import any bug.
But to keep consistant with general rule, we should take this patch.
Thanks.
sigchain.c | 2 +-
test-sigchain.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sigchain.c b/sigchain.c
index 1118b99..faa375d 100644
--- a/sigchain.c
+++ b/sigchain.c
@@ -1,5 +1,5 @@
-#include sigchain.h
#include cache.h
+#include sigchain.h
#define SIGCHAIN_MAX_SIGNALS 32
diff --git a/test-sigchain.c b/test-sigchain.c
index 42db234..e499fce 100644
--- a/test-sigchain.c
+++ b/test-sigchain.c
@@ -1,5 +1,5 @@
-#include sigchain.h
#include cache.h
+#include sigchain.h
#define X(f) \
static void f(int sig) { \
--
1.9.0.138.g2de3478.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 v2] finish_tmp_packfile():use strbuf for pathname construction
Signed-off-by: Sun He sunheeh...@gmail.com
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Helped-by: Michael Haggerty mhag...@alum.mit.edu
---
This patch has assumed that you have already fix the bug of
tmpname in builtin/pack-objects.c:write_pack_file() warning()
builtin/pack-objects.c | 15 ++-
bulk-checkin.c | 8 +---
pack-write.c | 18 ++
pack.h | 2 +-
4 files changed, 22 insertions(+), 21 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index c733379..099d6ed 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -803,7 +803,7 @@ static void write_pack_file(void)
if (!pack_to_stdout) {
struct stat st;
- char tmpname[PATH_MAX];
+ struct strbuf tmpname = STRBUF_INIT;
/*
* Packs are runtime accessed in their mtime
@@ -826,23 +826,19 @@ static void write_pack_file(void)
tmpname, strerror(errno));
}
- /* Enough space for -sha-1.pack? */
- if (sizeof(tmpname) = strlen(base_name) + 50)
- die(pack base name '%s' too long, base_name);
- snprintf(tmpname, sizeof(tmpname), %s-, base_name);
+ strbuf_addf(tmpname, %s-, base_name);
if (write_bitmap_index) {
bitmap_writer_set_checksum(sha1);
bitmap_writer_build_type_index(written_list,
nr_written);
}
- finish_tmp_packfile(tmpname, pack_tmp_name,
+ finish_tmp_packfile(tmpname, pack_tmp_name,
written_list, nr_written,
pack_idx_opts, sha1);
if (write_bitmap_index) {
- char *end_of_name_prefix = strrchr(tmpname, 0);
- sprintf(end_of_name_prefix, %s.bitmap,
sha1_to_hex(sha1));
+ strbuf_addf(tmpname, %s.bitmap
,sha1_to_hex(sha1));
stop_progress(progress_state);
@@ -851,10 +847,11 @@ static void write_pack_file(void)
bitmap_writer_select_commits(indexed_commits,
indexed_commits_nr, -1);
bitmap_writer_build(to_pack);
bitmap_writer_finish(written_list, nr_written,
-tmpname,
write_bitmap_options);
+tmpname.buf,
write_bitmap_options);
write_bitmap_index = 0;
}
+ strbuf_release(tmpname);
free(pack_tmp_name);
puts(sha1_to_hex(sha1));
}
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 118c625..98e651c 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -4,6 +4,7 @@
#include bulk-checkin.h
#include csum-file.h
#include pack.h
+#include strbuf.h
static int pack_compression_level = Z_DEFAULT_COMPRESSION;
@@ -23,7 +24,7 @@ static struct bulk_checkin_state {
static void finish_bulk_checkin(struct bulk_checkin_state *state)
{
unsigned char sha1[20];
- char packname[PATH_MAX];
+ struct strbuf packname = STRBUF_INIT;
int i;
if (!state-f)
@@ -43,8 +44,8 @@ static void finish_bulk_checkin(struct bulk_checkin_state
*state)
close(fd);
}
- sprintf(packname, %s/pack/pack-, get_object_directory());
- finish_tmp_packfile(packname, state-pack_tmp_name,
+ strbuf_addf(packname, %s/pack/pack-, get_object_directory());
+ finish_tmp_packfile(packname, state-pack_tmp_name,
state-written, state-nr_written,
state-pack_idx_opts, sha1);
for (i = 0; i state-nr_written; i++)
@@ -54,6 +55,7 @@ clear_exit:
free(state-written);
memset(state, 0, sizeof(*state));
+ strbuf_release(packname);
/* Make objects we just wrote available to ourselves */
reprepare_packed_git();
}
diff --git a/pack-write.c b/pack-write.c
index 9b8308b..9ccf804 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -336,7 +336,7 @@ struct sha1file *create_tmp_packfile(char **pack_tmp_name)
return sha1fd(fd, *pack_tmp_name);
}
-void finish_tmp_packfile(char *name_buffer,
+void finish_tmp_packfile(struct strbuf *name_buffer,
const char *pack_tmp_name,
struct pack_idx_entry **written_list,
uint32_t nr_written,
@@ -344,7 +344,7 @@ void finish_tmp_packfile(char *name_buffer
[PATCH v2] Replace tmpname with pack_tmp_name in warning. The developer mistook tmpname for pack_tmp_name.
Signed-off-by: Sun He sunheeh...@gmail.com --- As tmpname is used without initialization, it should be a mistake. builtin/pack-objects.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index c733379..4922ce5 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -823,7 +823,7 @@ static void write_pack_file(void) utb.modtime = --last_mtime; if (utime(pack_tmp_name, utb) 0) warning(failed utime() on %s: %s, - tmpname, strerror(errno)); + pack_tmp_name, strerror(errno)); } /* Enough space for -sha-1.pack? */ -- 1.9.0.138.g2de3478.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] write_pack_file: use correct variable in diagnostic
'pack_tmp_name' is the subject of the utime() check, so report it in the warning, not the uninitialized 'tmpname' Signed-off-by: Sun He sunheeh...@gmail.com --- Changing the subject and adding valid information as tutored by Eric Sunshine. Thanks to him. builtin/pack-objects.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index c733379..4922ce5 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -823,7 +823,7 @@ static void write_pack_file(void) utb.modtime = --last_mtime; if (utime(pack_tmp_name, utb) 0) warning(failed utime() on %s: %s, - tmpname, strerror(errno)); + pack_tmp_name, strerror(errno)); } /* Enough space for -sha-1.pack? */ -- 1.9.0.138.g2de3478.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] Rewrite bulk-checkin.c:finish_bulk_checkin() to use a strbuf for handling packname
Signed-off-by: Sun He sunheeh...@gmail.com
---
builtin/pack-objects.c | 17 +++--
bulk-checkin.c | 8 +---
pack-write.c | 20
pack.h | 2 +-
4 files changed, 25 insertions(+), 22 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index c733379..72fb82b 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -803,7 +803,7 @@ static void write_pack_file(void)
if (!pack_to_stdout) {
struct stat st;
- char tmpname[PATH_MAX];
+ struct strbuf tmpname = STRBUF_INIT;
/*
* Packs are runtime accessed in their mtime
@@ -823,26 +823,22 @@ static void write_pack_file(void)
utb.modtime = --last_mtime;
if (utime(pack_tmp_name, utb) 0)
warning(failed utime() on %s: %s,
- tmpname, strerror(errno));
+ pack_tmp_name, strerror(errno));
}
- /* Enough space for -sha-1.pack? */
- if (sizeof(tmpname) = strlen(base_name) + 50)
- die(pack base name '%s' too long, base_name);
- snprintf(tmpname, sizeof(tmpname), %s-, base_name);
+ strbuf_addf(tmpname, %s-, base_name);
if (write_bitmap_index) {
bitmap_writer_set_checksum(sha1);
bitmap_writer_build_type_index(written_list,
nr_written);
}
- finish_tmp_packfile(tmpname, pack_tmp_name,
+ finish_tmp_packfile(tmpname, pack_tmp_name,
written_list, nr_written,
pack_idx_opts, sha1);
if (write_bitmap_index) {
- char *end_of_name_prefix = strrchr(tmpname, 0);
- sprintf(end_of_name_prefix, %s.bitmap,
sha1_to_hex(sha1));
+ strbuf_addf(tmpname, %s.bitmap
,sha1_to_hex(sha1));
stop_progress(progress_state);
@@ -851,10 +847,11 @@ static void write_pack_file(void)
bitmap_writer_select_commits(indexed_commits,
indexed_commits_nr, -1);
bitmap_writer_build(to_pack);
bitmap_writer_finish(written_list, nr_written,
-tmpname,
write_bitmap_options);
+tmpname.buf,
write_bitmap_options);
write_bitmap_index = 0;
}
+ strbuf_release(tmpname);
free(pack_tmp_name);
puts(sha1_to_hex(sha1));
}
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 118c625..98e651c 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -4,6 +4,7 @@
#include bulk-checkin.h
#include csum-file.h
#include pack.h
+#include strbuf.h
static int pack_compression_level = Z_DEFAULT_COMPRESSION;
@@ -23,7 +24,7 @@ static struct bulk_checkin_state {
static void finish_bulk_checkin(struct bulk_checkin_state *state)
{
unsigned char sha1[20];
- char packname[PATH_MAX];
+ struct strbuf packname = STRBUF_INIT;
int i;
if (!state-f)
@@ -43,8 +44,8 @@ static void finish_bulk_checkin(struct bulk_checkin_state
*state)
close(fd);
}
- sprintf(packname, %s/pack/pack-, get_object_directory());
- finish_tmp_packfile(packname, state-pack_tmp_name,
+ strbuf_addf(packname, %s/pack/pack-, get_object_directory());
+ finish_tmp_packfile(packname, state-pack_tmp_name,
state-written, state-nr_written,
state-pack_idx_opts, sha1);
for (i = 0; i state-nr_written; i++)
@@ -54,6 +55,7 @@ clear_exit:
free(state-written);
memset(state, 0, sizeof(*state));
+ strbuf_release(packname);
/* Make objects we just wrote available to ourselves */
reprepare_packed_git();
}
diff --git a/pack-write.c b/pack-write.c
index 9b8308b..2204aa9 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -336,7 +336,7 @@ struct sha1file *create_tmp_packfile(char **pack_tmp_name)
return sha1fd(fd, *pack_tmp_name);
}
-void finish_tmp_packfile(char *name_buffer,
+void finish_tmp_packfile(struct strbuf *name_buffer,
const char *pack_tmp_name,
struct pack_idx_entry **written_list,
uint32_t nr_written,
@@ -344,7 +344,7
[PATCH] parse-options.c:parse_options_check() change OPTION_NUMBER to OPTION_CMDMODE
Signed-off-by: Sun He sunheeh...@gmail.com --- parse-options.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parse-options.c b/parse-options.c index 7b8d3fa..59a52b0 100644 --- a/parse-options.c +++ b/parse-options.c @@ -371,7 +371,7 @@ static void parse_options_check(const struct option *opts) case OPTION_NEGBIT: case OPTION_SET_INT: case OPTION_SET_PTR: - case OPTION_NUMBER: + case OPTION_CMDMODE: if ((opts-flags PARSE_OPT_OPTARG) || !(opts-flags PARSE_OPT_NOARG)) err |= optbug(opts, should not accept an argument); -- 1.9.0.138.g2de3478.dirty Hi, When I was reading code yesterday, I came across this protential bug. parse-options.h says that OPTION_CMDMODE is an option with no arguments and OPTION_NUMBER is special type option. According to the information the program says (Should not accept an argument), I think you should take this patch into consideration. Thanks. He Sun -- 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] OPTION_CMDMODE should be used when not accept an argument, and OPTION_NUMBER is of special type. So change the mode to OPTION_CMDMODE
Signed-off-by: Sun He sunheeh...@gmail.com --- parse-options.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parse-options.c b/parse-options.c index 7b8d3fa..59a52b0 100644 --- a/parse-options.c +++ b/parse-options.c @@ -371,7 +371,7 @@ static void parse_options_check(const struct option *opts) case OPTION_NEGBIT: case OPTION_SET_INT: case OPTION_SET_PTR: - case OPTION_NUMBER: + case OPTION_CMDMODE: if ((opts-flags PARSE_OPT_OPTARG) || !(opts-flags PARSE_OPT_NOARG)) err |= optbug(opts, should not accept an argument); -- 1.9.0.138.g2de3478.dirty --- I came across this protential bug. According to parse-options.h OPTION_CMDMODE is an option with noarguments and OPTION_NUMBER is special type option. Thanks, He Sun -- 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] OPTION_NUMBER should be replaced by OPTION_CMDMODE
Signed-off-by: Sun He sunheeh...@gmail.com --- parse-options.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parse-options.c b/parse-options.c index 7b8d3fa..59a52b0 100644 --- a/parse-options.c +++ b/parse-options.c @@ -371,7 +371,7 @@ static void parse_options_check(const struct option *opts) case OPTION_NEGBIT: case OPTION_SET_INT: case OPTION_SET_PTR: - case OPTION_NUMBER: + case OPTION_CMDMODE: if ((opts-flags PARSE_OPT_OPTARG) || !(opts-flags PARSE_OPT_NOARG)) err |= optbug(opts, should not accept an argument); -- 1.9.0.138.g2de3478.dirty --- I came I came across this protential bug. According to parse-options.h OPTION_CMDMODE is an option with noarguments and OPTION_NUMBER is special type option Thanks, He Sun -- 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] builtin/pack-objects.c:write_pack_file() replace tmpname with pack_tmp_name in warning
Signed-off-by: Sun He sunheeh...@gmail.com --- builtin/pack-objects.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index c733379..4922ce5 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -823,7 +823,7 @@ static void write_pack_file(void) utb.modtime = --last_mtime; if (utime(pack_tmp_name, utb) 0) warning(failed utime() on %s: %s, - tmpname, strerror(errno)); + pack_tmp_name, strerror(errno)); } /* Enough space for -sha-1.pack? */ -- 1.9.0.138.g2de3478.dirty --- The tmpname is uninitialized and it should a bug -- 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] builtin/pack-objects.c:write_pack_file() replace tmpname with pack_tmp_name in warning
Signed-off-by: Sun He sunheeh...@gmail.com --- The tmpname is uninitialized and it should a bug Please ignore the former patches about this with wrong format. I am sorry to cause a jam in your inbox. ^_^ In the end, I wanna thank Michael Haggerty who give me help. builtin/pack-objects.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index c733379..4922ce5 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -823,7 +823,7 @@ static void write_pack_file(void) utb.modtime = --last_mtime; if (utime(pack_tmp_name, utb) 0) warning(failed utime() on %s: %s, - tmpname, strerror(errno)); + pack_tmp_name, strerror(errno)); } /* Enough space for -sha-1.pack? */ -- 1.9.0.138.g2de3478.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] finish_tmp_packfile():use strbuf for pathname construction
Signed-off-by: Sun He sunheeh...@gmail.com
---
I follow the suggestions of Eric Sunshine to fix the patch.
Of cause this patch has assumed that you have already fix the bug of
tmpname in builtin/pack-objects.c:write_pack_file() warning()
I want to say thank you to Eric Sunshine and Michael Haggerty who give me
lots of help.
builtin/pack-objects.c | 15 ++-
bulk-checkin.c | 8 +---
pack-write.c | 18 ++
pack.h | 2 +-
4 files changed, 22 insertions(+), 21 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index c733379..099d6ed 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -803,7 +803,7 @@ static void write_pack_file(void)
if (!pack_to_stdout) {
struct stat st;
- char tmpname[PATH_MAX];
+ struct strbuf tmpname = STRBUF_INIT;
/*
* Packs are runtime accessed in their mtime
@@ -826,23 +826,19 @@ static void write_pack_file(void)
tmpname, strerror(errno));
}
- /* Enough space for -sha-1.pack? */
- if (sizeof(tmpname) = strlen(base_name) + 50)
- die(pack base name '%s' too long, base_name);
- snprintf(tmpname, sizeof(tmpname), %s-, base_name);
+ strbuf_addf(tmpname, %s-, base_name);
if (write_bitmap_index) {
bitmap_writer_set_checksum(sha1);
bitmap_writer_build_type_index(written_list,
nr_written);
}
- finish_tmp_packfile(tmpname, pack_tmp_name,
+ finish_tmp_packfile(tmpname, pack_tmp_name,
written_list, nr_written,
pack_idx_opts, sha1);
if (write_bitmap_index) {
- char *end_of_name_prefix = strrchr(tmpname, 0);
- sprintf(end_of_name_prefix, %s.bitmap,
sha1_to_hex(sha1));
+ strbuf_addf(tmpname, %s.bitmap
,sha1_to_hex(sha1));
stop_progress(progress_state);
@@ -851,10 +847,11 @@ static void write_pack_file(void)
bitmap_writer_select_commits(indexed_commits,
indexed_commits_nr, -1);
bitmap_writer_build(to_pack);
bitmap_writer_finish(written_list, nr_written,
-tmpname,
write_bitmap_options);
+tmpname.buf,
write_bitmap_options);
write_bitmap_index = 0;
}
+ strbuf_release(tmpname);
free(pack_tmp_name);
puts(sha1_to_hex(sha1));
}
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 118c625..98e651c 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -4,6 +4,7 @@
#include bulk-checkin.h
#include csum-file.h
#include pack.h
+#include strbuf.h
static int pack_compression_level = Z_DEFAULT_COMPRESSION;
@@ -23,7 +24,7 @@ static struct bulk_checkin_state {
static void finish_bulk_checkin(struct bulk_checkin_state *state)
{
unsigned char sha1[20];
- char packname[PATH_MAX];
+ struct strbuf packname = STRBUF_INIT;
int i;
if (!state-f)
@@ -43,8 +44,8 @@ static void finish_bulk_checkin(struct bulk_checkin_state
*state)
close(fd);
}
- sprintf(packname, %s/pack/pack-, get_object_directory());
- finish_tmp_packfile(packname, state-pack_tmp_name,
+ strbuf_addf(packname, %s/pack/pack-, get_object_directory());
+ finish_tmp_packfile(packname, state-pack_tmp_name,
state-written, state-nr_written,
state-pack_idx_opts, sha1);
for (i = 0; i state-nr_written; i++)
@@ -54,6 +55,7 @@ clear_exit:
free(state-written);
memset(state, 0, sizeof(*state));
+ strbuf_release(packname);
/* Make objects we just wrote available to ourselves */
reprepare_packed_git();
}
diff --git a/pack-write.c b/pack-write.c
index 9b8308b..9ccf804 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -336,7 +336,7 @@ struct sha1file *create_tmp_packfile(char **pack_tmp_name)
return sha1fd(fd, *pack_tmp_name);
}
-void finish_tmp_packfile(char *name_buffer,
+void finish_tmp_packfile(struct strbuf *name_buffer,
const char *pack_tmp_name,
struct pack_idx_entry **written_list,
uint32_t nr_written,
@@ -344,7 +344,7
[PATCH] Replace memcpy with hashcpy when dealing hash copy globally
Signed-off-by: Sun He sunheeh...@gmail.com --- Find the potential places with memcpy by the bash command: $ find . | xargs grep memcpy.*\(.*20.*\) Helped-by: Michael Haggertymhag...@alum.mit.edu bundle.c| 2 +- grep.c | 2 +- pack-bitmap-write.c | 2 +- ppc/sha1.c | 3 ++- reflog-walk.c | 4 ++-- refs.c | 2 +- 6 files changed, 8 insertions(+), 7 deletions(-) diff --git a/bundle.c b/bundle.c index e99065c..7809fbb 100644 --- a/bundle.c +++ b/bundle.c @@ -19,7 +19,7 @@ static void add_to_ref_list(const unsigned char *sha1, const char *name, list-list = xrealloc(list-list, list-alloc * sizeof(list-list[0])); } - memcpy(list-list[list-nr].sha1, sha1, 20); + hashcpy(list-list[list-nr].sha1, sha1); list-list[list-nr].name = xstrdup(name); list-nr++; } diff --git a/grep.c b/grep.c index c668034..f5101f7 100644 --- a/grep.c +++ b/grep.c @@ -1650,7 +1650,7 @@ void grep_source_init(struct grep_source *gs, enum grep_source_type type, break; case GREP_SOURCE_SHA1: gs-identifier = xmalloc(20); - memcpy(gs-identifier, identifier, 20); + hashcpy(gs-identifier, identifier); break; case GREP_SOURCE_BUF: gs-identifier = NULL; diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c index 1218bef..5f1791a 100644 --- a/pack-bitmap-write.c +++ b/pack-bitmap-write.c @@ -530,7 +530,7 @@ void bitmap_writer_finish(struct pack_idx_entry **index, header.version = htons(default_version); header.options = htons(flags | options); header.entry_count = htonl(writer.selected_nr); - memcpy(header.checksum, writer.pack_checksum, 20); + hashcpy(header.checksum, writer.pack_checksum); sha1write(f, header, sizeof(header)); dump_bitmap(f, writer.commits); diff --git a/ppc/sha1.c b/ppc/sha1.c index ec6a192..8a87fea 100644 --- a/ppc/sha1.c +++ b/ppc/sha1.c @@ -9,6 +9,7 @@ #include stdio.h #include string.h #include sha1.h +#include cache.h extern void ppc_sha1_core(uint32_t *hash, const unsigned char *p, unsigned int nblocks); @@ -67,6 +68,6 @@ int ppc_SHA1_Final(unsigned char *hash, ppc_SHA_CTX *c) memset(c-buf.b[cnt], 0, 56 - cnt); c-buf.l[7] = c-len; ppc_sha1_core(c-hash, c-buf.b, 1); - memcpy(hash, c-hash, 20); + hashcpy(hash, c-hash); return 0; } diff --git a/reflog-walk.c b/reflog-walk.c index b2fbdb2..d490f7d 100644 --- a/reflog-walk.c +++ b/reflog-walk.c @@ -32,8 +32,8 @@ static int read_one_reflog(unsigned char *osha1, unsigned char *nsha1, sizeof(struct reflog_info)); } item = array-items + array-nr; - memcpy(item-osha1, osha1, 20); - memcpy(item-nsha1, nsha1, 20); + hashcpy(item-osha1, osha1); + hashcpy(item-nsha1, nsha1); item-email = xstrdup(email); item-timestamp = timestamp; item-tz = tz; diff --git a/refs.c b/refs.c index 89228e2..f90b7ea 100644 --- a/refs.c +++ b/refs.c @@ -1222,7 +1222,7 @@ static int resolve_gitlink_packed_ref(struct ref_cache *refs, if (ref == NULL) return -1; - memcpy(sha1, ref-u.value.sha1, 20); + hashcpy(sha1, ref-u.value.sha1); return 0; } -- 1.9.0.138.g2de3478.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] Rewrite git-compat-util.h:skip_prefix() as a loop
Signed-off-by: Sun He sunheeh...@gmail.com
---
git-compat-util.h |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index cbd86c3..4daa6cf 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -357,8 +357,8 @@ extern int suffixcmp(const char *str, const char *suffix);
static inline const char *skip_prefix(const char *str, const char *prefix)
{
- size_t len = strlen(prefix);
- return strncmp(str, prefix, len) ? NULL : str + len;
+while( *prefix != '\0' *str++ == *prefix++ );
+return *prefix == '\0' ? str : NULL;
}
#if defined(NO_MMAP) || defined(USE_WIN32_MMAP)
--
1.7.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] Change branch.c:install_branch_config()
Signed-off-by: Sun He sunheeh...@gmail.com
---
branch.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/branch.c b/branch.c
index 723a36b..2fe9c05 100644
--- a/branch.c
+++ b/branch.c
@@ -50,7 +50,7 @@ static int should_setup_rebase(const char *origin)
void install_branch_config(int flag, const char *local, const char *origin,
const char *remote)
{
const char *shortname = remote + 11;
- int remote_is_branch = starts_with(remote, refs/heads/);
+ int remote_is_branch = skip_prefix(remote,refs/heads)!=NULL;
struct strbuf key = STRBUF_INIT;
int rebasing = should_setup_rebase(origin);
--
1.7.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] Rewrite bulk-checkin.c:finish_bulk_checkin() to use a strbuf for handling packname
Signed-off-by: Sun He sunheeh...@gmail.com
---
bulk-checkin.c | 10 +-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 118c625..8c47d71 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -23,7 +23,8 @@ static struct bulk_checkin_state {
static void finish_bulk_checkin(struct bulk_checkin_state *state)
{
unsigned char sha1[20];
- char packname[PATH_MAX];
+ char *packname;
+struct strbuf sb;
int i;
if (!state-f)
@@ -43,6 +44,10 @@ static void finish_bulk_checkin(struct bulk_checkin_state
*state)
close(fd);
}
+/* 64-1 is more than the sum of len(sha1_to_hex(sha1)) and len(.pack) */
+strbuf_init(sb,strlen(get_object_directory())+64);
+packname = sb.buf;
+
sprintf(packname, %s/pack/pack-, get_object_directory());
finish_tmp_packfile(packname, state-pack_tmp_name,
state-written, state-nr_written,
@@ -54,6 +59,9 @@ clear_exit:
free(state-written);
memset(state, 0, sizeof(*state));
+/* release sb space */
+strbuf_release(sb);
+
/* Make objects we just wrote available to ourselves */
reprepare_packed_git();
}
--
1.7.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] GSoC2014 microprojects Rewrite bulk-checkin.c:finish_bulk_checkin() to use a strbuf for handling packname
Signed-off-by: Sun He sunheeh...@gmail.com
---
bulk-checkin.c | 10 +-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 118c625..e3c7fb2 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -23,7 +23,8 @@ static struct bulk_checkin_state {
static void finish_bulk_checkin(struct bulk_checkin_state *state)
{
unsigned char sha1[20];
- char packname[PATH_MAX];
+char *packname;
+struct strbuf sb;
int i;
if (!state-f)
@@ -43,6 +44,10 @@ static void finish_bulk_checkin(struct bulk_checkin_state
*state)
close(fd);
}
+ /* 64-1 is more than the sum of len(sha1_to_hex(sha1)) and len(.pack) */
+strbuf_init(sb,strlen(get_object_directory())+64);
+packname = sb.buf;
+
sprintf(packname, %s/pack/pack-, get_object_directory());
finish_tmp_packfile(packname, state-pack_tmp_name,
state-written, state-nr_written,
@@ -54,6 +59,9 @@ clear_exit:
free(state-written);
memset(state, 0, sizeof(*state));
+/* release sb space */
+strbuf_release(sb);
+
/* Make objects we just wrote available to ourselves */
reprepare_packed_git();
}
--
1.7.1
Rewrite bulk-checkin.c:finish_bulk_checkin() to use a strbuf for handling
packname, and explain why this is useful.
char packname[PATH_MAX] (PATH_MAX=4096) in a local function costs too much
space, it may cause stack overflow.
Using strbuf to deal with packname is more space-friendly.
I only use the API strbuf_init to alloc enough space for packname. I didn't use
other APIs because I want to make the change as little as possible that I don't
have to fix other functions which may import new bugs.
Because the space spared for sb is on heap, we must release it after it is not
useful.
Also check if the first argument of pack-write.c:finish_tmp_packfile() can be
made const.
Tht first argument of pack-write.c:finish_tmp_packfile() can be made const
because we didn't use *name_buffer to change anything.
Cheers,
He Sun
PS:
Why I cannot sent email to git@vger.kernel.org via my Firefox?
Can you receive my former emails?
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
[PATCH] GSoC2014 microprojects #5 Change bundle.c:add_to_ref_list() to use hashcpy()
Signed-off-by: Sun He sunheeh...@gmail.com --- bundle.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/bundle.c b/bundle.c index e99065c..7809fbb 100644 --- a/bundle.c +++ b/bundle.c @@ -19,7 +19,7 @@ static void add_to_ref_list(const unsigned char *sha1, const char *name, list-list = xrealloc(list-list, list-alloc * sizeof(list-list[0])); } - memcpy(list-list[list-nr].sha1, sha1, 20); + hashcpy(list-list[list-nr].sha1, sha1); list-list[list-nr].name = xstrdup(name); list-nr++; } -- 1.7.1 See if you can find other places where hashcpy() should be used instead of memcpy() grep.c:grep_source_init() reflog-walk.c:read_one_reflog() ppc/sha1.c:ppc_SHA1_Final() refs.c:resolve_gitlink_packed_ref() We can find those by the shell command: $ find . | xargs grep memcpy\(.*20.*\) Cheers, He Sun -- 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] GSoC2014 microprojects #6 Change bundle.c:add_to_ref_list() to use ALLOC_GROW()
Signed-off-by: Sun He sunheeh...@gmail.com
---
bundle.c |6 +-
1 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/bundle.c b/bundle.c
index 7809fbb..1a7b7eb 100644
--- a/bundle.c
+++ b/bundle.c
@@ -14,11 +14,7 @@ static const char bundle_signature[] = # v2 git bundle\n;
static void add_to_ref_list(const unsigned char *sha1, const char *name,
struct ref_list *list)
{
- if (list-nr + 1 = list-alloc) {
- list-alloc = alloc_nr(list-nr + 1);
- list-list = xrealloc(list-list,
- list-alloc * sizeof(list-list[0]));
- }
+ALLOC_GROW(list-list,list-nr,list-alloc);
hashcpy(list-list[list-nr].sha1, sha1);
list-list[list-nr].name = xstrdup(name);
list-nr++;
--
1.7.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
