Re: [PATCH v2 1/1] setup: recognise extensions.objectFormat

2018-01-30 Thread Patryk Obara

On 30/01/2018 02:38, Jeff King wrote:

On Sun, Jan 28, 2018 at 01:36:17AM +0100, Patryk Obara wrote:


This extension selects which hashing algorithm from vtable should be
used for reading and writing objects in the object store.  At the moment
supports only single value (sha-1).

In case value of objectFormat is an unknown hashing algorithm, Git
command will fail with following message:

   fatal: unknown repository extensions found:
  objectformat = 

To indicate, that this specific objectFormat value is not recognised.


I don't have a strong opinion on this, but it does feel a little funny
to add this extension now, before we quite know what the code that uses
it is going to look like (or maybe we're farther along there than I
realize).


Code using this is already in master - in the result of overwriting
data->hash_algo, every piece of code, that was modernised starts using
the selected hash algorithm (through the_hash_algo) instead of hardcoded
sha-1.

As far as I can tell status is following:

Once following topics will land:
- po/object-id (in pu)
- brian's object-id-part-11 (in review now)
- upcoming brian's object-id-part-12 (not sent to mailing list yet)
- few more object-id conversions and uses of the_hash_algo

we'll be in state, where just dropping new entry in hash_algos table
will enable experimental switching of object format.

With changes listed above "git hash-object -w -t blob" and
"git cat-file" work with NewHash (whatever it may be - brian is using 
blake2 in his experiments, I am using openssl sha3-256).


Right now I am looking at updating index structures and functions - 
after which git commit should work. In the transition plan it's 
described as "introducing index v3" (are there any new requirements, 
that constitute "v3" besides longer hash?).



What do we gain by doing this now as opposed to later? By the design of
the extension code, we should complain on older versions anyway. And by
doing it now we carry a small risk that it might not give us the
interface we want, and it will be slightly harder to paper over this
failed direction.


Mostly convenience for developers, who want to work on transition. 
There's no need to re-compile only for changing default hashing 
algorithm (which is useful for testing and debugging). I could carry 
this patch around to every NewHash-related branch, that I work on but 
it's annoying me already ;)


As long as hash_algos table contains only sha-1, users effectively see
this extension as noop.


It wasn't intended that anyone would specify preciousObjects with repo
version 0. It's a dangerous misconfiguration (because versions which
predate the extensions mechanism won't actually respect it at all!).

So we probably ought to complain loudly on having anything in
extensions.* when the repositoryformat is less than 1.

>

I originally wrote it the other way out of an abundance of
backward-compatibility. After all "extension.*" doesn't mean anything in
format 0, and somebody _could_ have added such a config key for their
own purposes. But that's a pretty weak argument, and if we are going to
start marking some extensions as forbidden there, we might as well do
them all.


What about users, who are using new version of Git, but have it 
misconfigured with preciousObjects and repo format 0? That's why I 
decided to make repo format check specific to objectFormat extension 
(initially I made it generic to all extensions).


At the same time... there's extension.partialclone in pu and it does not 
have check on repo format.


--
| ← Ceci n'est pas une pipe
Patryk Obara


Re: How juggle branches?

2018-01-29 Thread Patryk Obara

On 29/01/2018 22:24, Andrzej wrote:


I am in master branch and am changing to hbase:
git checkout -b hbase
git push origin hbase


These two commands create new branch called "hbase" in your local repo,
and then in remote repo - so probably not what you wanted to do.


now worse:
I am in branch before_hbase and need change to master
git checkout -b master  - not works because master exists


"git checkout -b name" creates new branch called "name", starting in 
your latest commit and switches you to this new branch.


"git checkout name" switches your working tree to branch "name".

So just drop "-b". You can read more in manual for git-checkout:
https://git-scm.com/docs/git-checkout

(in polish) Jeśli masz jakieś konkretne pytania, to możesz napisać do
mnie po polsku :).

--
| ← Ceci n'est pas une pipe
Patryk Obara


[PATCH v3 1/1] setup: recognise extensions.objectFormat

2018-01-29 Thread Patryk Obara
This extension selects which hashing algorithm from vtable should be
used for reading and writing objects in the object store.  At the moment
it supports only single value (sha-1).

In case value of objectFormat is an unknown hashing algorithm, Git
command will fail with the following message:

  fatal: unknown repository extensions found:
  objectformat = 

To indicate, that this specific objectFormat value is not recognised.

The objectFormat extension is not allowed in repository marked as
version 0 to prevent any possibility of accidentally writing a NewHash
object in the sha-1 object store. This extension behaviour is different
than preciousObjects extension (which is allowed in repo version 0).

Add tests and documentation note about new extension.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 Documentation/technical/repository-version.txt | 12 
 setup.c| 27 ++
 t/t1302-repo-version.sh| 15 ++
 3 files changed, 54 insertions(+)

diff --git a/Documentation/technical/repository-version.txt 
b/Documentation/technical/repository-version.txt
index 00ad37986e..7e2b832603 100644
--- a/Documentation/technical/repository-version.txt
+++ b/Documentation/technical/repository-version.txt
@@ -86,3 +86,15 @@ for testing format-1 compatibility.
 When the config key `extensions.preciousObjects` is set to `true`,
 objects in the repository MUST NOT be deleted (e.g., by `git-prune` or
 `git repack -d`).
+
+`objectFormat`
+~~
+
+This extension instructs Git to use a specific algorithm for addressing
+and interpreting objects in the object store. Currently, the only
+supported object format is `sha-1`. At the moment, the primary purpose
+of this option is to enable Git developers to experiment with different
+hashing algorithms without re-compilation of git client.
+
+See `hash-function-transition.txt` document for more detailed explanation.
+
diff --git a/setup.c b/setup.c
index 8cc34186ce..9b9993a14e 100644
--- a/setup.c
+++ b/setup.c
@@ -405,6 +405,31 @@ void setup_work_tree(void)
initialized = 1;
 }
 
+static int find_object_format(const char *value)
+{
+   int i;
+   for (i = GIT_HASH_SHA1; i < GIT_HASH_NALGOS; ++i) {
+   if (strcmp(value, hash_algos[i].name) == 0)
+   return i;
+   }
+   return GIT_HASH_UNKNOWN;
+}
+
+static void detect_object_format(struct repository_format *data,
+const char *value)
+{
+   if (data->version == 0)
+   die(_("invalid repository format version '%d'"), data->version);
+
+   data->hash_algo = find_object_format(value);
+   if (data->hash_algo == GIT_HASH_UNKNOWN) {
+   struct strbuf object_format = STRBUF_INIT;
+   strbuf_addf(_format, "objectformat = %s", value);
+   string_list_append(>unknown_extensions, 
object_format.buf);
+   strbuf_release(_format);
+   }
+}
+
 static int check_repo_format(const char *var, const char *value, void *vdata)
 {
struct repository_format *data = vdata;
@@ -422,6 +447,8 @@ static int check_repo_format(const char *var, const char 
*value, void *vdata)
;
else if (!strcmp(ext, "preciousobjects"))
data->precious_objects = git_config_bool(var, value);
+   else if (!strcmp(ext, "objectformat"))
+   detect_object_format(data, value);
else
string_list_append(>unknown_extensions, ext);
} else if (strcmp(var, "core.bare") == 0) {
diff --git a/t/t1302-repo-version.sh b/t/t1302-repo-version.sh
index ce4cff13bb..227b397ff2 100755
--- a/t/t1302-repo-version.sh
+++ b/t/t1302-repo-version.sh
@@ -107,4 +107,19 @@ test_expect_success 'gc runs without complaint' '
git gc
 '
 
+test_expect_success 'object-format not allowed in repo version=0' '
+   mkconfig 0 "objectFormat = sha-1" >.git/config &&
+   check_abort
+'
+
+test_expect_success 'object-format=sha-1 allowed' '
+   mkconfig 1 "objectFormat = sha-1" >.git/config &&
+   check_allow
+'
+
+test_expect_success 'object-format=foo unsupported' '
+   mkconfig 1 "objectFormat = foo" >.git/config &&
+   check_abort
+'
+
 test_done
-- 
2.14.3



[PATCH v3 0/1] setup: recognise extensions.objectFormat

2018-01-29 Thread Patryk Obara
Compred to v2:

Fixed commit message.

Patryk Obara (1):
  setup: recognise extensions.objectFormat

 Documentation/technical/repository-version.txt | 12 
 setup.c| 27 ++
 t/t1302-repo-version.sh| 15 ++
 3 files changed, 54 insertions(+)


base-commit: 5be1f00a9a701532232f57958efab4be8c959a29
-- 
2.14.3



Re: [PATCH 00/12] object_id part 11 (the_hash_algo)

2018-01-28 Thread Patryk Obara

I looked at your branch object-id-part-12, and it conflicts with my next
batch of object_id conversions in quite many places (mostly through
formatting). Therefore I'll hold my horses and postpone my conversion
patches at least until part 12 will be sent.

--
| ← Ceci n'est pas une pipe
Patryk Obara


Re: [PATCH 08/12] pack-write: switch various SHA-1 values to abstract forms

2018-01-28 Thread Patryk Obara

On 28/01/2018 16:57, brian m. carlson wrote:

if (partial_pack_offset == 0) {
-   unsigned char sha1[20];
-   git_SHA1_Final(sha1, _sha1_ctx);
-   if (hashcmp(sha1, partial_pack_sha1) != 0)
+   unsigned char hash[GIT_MAX_RAWSZ];
+   the_hash_algo->final_fn(hash, _hash_ctx);
+   if (hashcmp(hash, partial_pack_hash) != 0)


Maybe "hash" should be struct object_id here?


  char *index_pack_lockfile(int ip_out)
  {
-   char packname[46];
+   char packname[GIT_MAX_HEXSZ + 6];
+   int len = the_hash_algo->hexsz + 6;


Just me nitpicking, but "len" can be const :)

--
| ← Ceci n'est pas une pipe
Patryk Obara


Re: [PATCH 06/12] fast-import: switch various uses of SHA-1 to the_hash_algo

2018-01-28 Thread Patryk Obara

On 28/01/2018 16:57, brian m. carlson wrote:

-   if (last && last->data.buf && last->depth < max_depth && dat->len > 20) 
{
+   if (last && last->data.buf && last->depth < max_depth && dat->len > 
the_hash_algo->rawsz) {


At this point line is almost 100 characters long - maybe it's time to
break it ;)

--
| ← Ceci n'est pas une pipe
Patryk Obara


Re: [PATCH 02/12] hash: create union for hash context allocation

2018-01-28 Thread Patryk Obara

On 28/01/2018 16:57, brian m. carlson wrote:

In various parts of our code, we want to allocate a structure
representing the internal state of a hash algorithm.  The original
implementation of the hash algorithm abstraction assumed we would do
that using heap allocations, and added a context size element to struct
git_hash_algo.  However, most of the existing code uses stack
allocations and conversion would needlessly complicate various parts of
the code.  Add a union for the purpose of allocating hash contexts on
the stack and a typedef for ease of use.  Remove the ctxsz element for
struct git_hash_algo, which is no longer very useful.


Overall, I am OK with this approach (it's straightforward change and 
cleanest way to replace direct calls to git_SHA1_* functions), but just 
to play devil's advocate: OpenSSL decided to sway users into heap 
allocated contexts, citing binary compatibility issues if they change 
the size of context structure. [1]


I think we might need to revisit this design decision in future - 
perhaps as soon as we'll transition away from calling git_SHA1_* 
functions directly.



+/* A suitably aligned type for stack allocations of hash contexts. */
+union git_hash_ctx {
+   git_SHA_CTX sha1;
+};
+typedef union git_hash_ctx git_hash_ctx;
+
  typedef void (*git_hash_init_fn)(void *ctx);
  typedef void (*git_hash_update_fn)(void *ctx, const void *in, size_t len);
  typedef void (*git_hash_final_fn)(unsigned char *hash, void *ctx);


I think it would be appropriate to replace "void *ctx" with 
"git_hash_ctx *ctx". This way we can avoid unnecessary casting in 
git_hash_sha1_* functions.


[1] https://wiki.openssl.org/index.php/Manual:EVP_DigestInit(3)#NOTES

--
| ← Ceci n'est pas une pipe
Patryk Obara


[PATCH v2 0/1] setup: recognise extensions.objectFormat

2018-01-27 Thread Patryk Obara
Compared to v1:

Implemented code suggestions from Duy Nguyễn (string for translation and
strbuf instead of char array). I also added an annotation in
repository-version.txt, clarifying, that this option is useful only for
development purpose for now.

Patryk Obara (1):
  setup: recognise extensions.objectFormat

 Documentation/technical/repository-version.txt | 12 
 setup.c| 27 ++
 t/t1302-repo-version.sh| 15 ++
 3 files changed, 54 insertions(+)


base-commit: 5be1f00a9a701532232f57958efab4be8c959a29
-- 
2.14.3



[PATCH v2 1/1] setup: recognise extensions.objectFormat

2018-01-27 Thread Patryk Obara
This extension selects which hashing algorithm from vtable should be
used for reading and writing objects in the object store.  At the moment
supports only single value (sha-1).

In case value of objectFormat is an unknown hashing algorithm, Git
command will fail with following message:

  fatal: unknown repository extensions found:
  objectformat = 

To indicate, that this specific objectFormat value is not recognised.

The objectFormat extension is not allowed in repository marked as
version 0 to prevent any possibility of accidentally writing a NewHash
object in the sha-1 object store. This extension behaviour is different
than preciousObjects extension (which is allowed in repo version 0).

Add tests and documentation note about new extension.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 Documentation/technical/repository-version.txt | 12 
 setup.c| 27 ++
 t/t1302-repo-version.sh| 15 ++
 3 files changed, 54 insertions(+)

diff --git a/Documentation/technical/repository-version.txt 
b/Documentation/technical/repository-version.txt
index 00ad37986e..7e2b832603 100644
--- a/Documentation/technical/repository-version.txt
+++ b/Documentation/technical/repository-version.txt
@@ -86,3 +86,15 @@ for testing format-1 compatibility.
 When the config key `extensions.preciousObjects` is set to `true`,
 objects in the repository MUST NOT be deleted (e.g., by `git-prune` or
 `git repack -d`).
+
+`objectFormat`
+~~
+
+This extension instructs Git to use a specific algorithm for addressing
+and interpreting objects in the object store. Currently, the only
+supported object format is `sha-1`. At the moment, the primary purpose
+of this option is to enable Git developers to experiment with different
+hashing algorithms without re-compilation of git client.
+
+See `hash-function-transition.txt` document for more detailed explanation.
+
diff --git a/setup.c b/setup.c
index 8cc34186ce..9b9993a14e 100644
--- a/setup.c
+++ b/setup.c
@@ -405,6 +405,31 @@ void setup_work_tree(void)
initialized = 1;
 }
 
+static int find_object_format(const char *value)
+{
+   int i;
+   for (i = GIT_HASH_SHA1; i < GIT_HASH_NALGOS; ++i) {
+   if (strcmp(value, hash_algos[i].name) == 0)
+   return i;
+   }
+   return GIT_HASH_UNKNOWN;
+}
+
+static void detect_object_format(struct repository_format *data,
+const char *value)
+{
+   if (data->version == 0)
+   die(_("invalid repository format version '%d'"), data->version);
+
+   data->hash_algo = find_object_format(value);
+   if (data->hash_algo == GIT_HASH_UNKNOWN) {
+   struct strbuf object_format = STRBUF_INIT;
+   strbuf_addf(_format, "objectformat = %s", value);
+   string_list_append(>unknown_extensions, 
object_format.buf);
+   strbuf_release(_format);
+   }
+}
+
 static int check_repo_format(const char *var, const char *value, void *vdata)
 {
struct repository_format *data = vdata;
@@ -422,6 +447,8 @@ static int check_repo_format(const char *var, const char 
*value, void *vdata)
;
else if (!strcmp(ext, "preciousobjects"))
data->precious_objects = git_config_bool(var, value);
+   else if (!strcmp(ext, "objectformat"))
+   detect_object_format(data, value);
else
string_list_append(>unknown_extensions, ext);
} else if (strcmp(var, "core.bare") == 0) {
diff --git a/t/t1302-repo-version.sh b/t/t1302-repo-version.sh
index ce4cff13bb..227b397ff2 100755
--- a/t/t1302-repo-version.sh
+++ b/t/t1302-repo-version.sh
@@ -107,4 +107,19 @@ test_expect_success 'gc runs without complaint' '
git gc
 '
 
+test_expect_success 'object-format not allowed in repo version=0' '
+   mkconfig 0 "objectFormat = sha-1" >.git/config &&
+   check_abort
+'
+
+test_expect_success 'object-format=sha-1 allowed' '
+   mkconfig 1 "objectFormat = sha-1" >.git/config &&
+   check_allow
+'
+
+test_expect_success 'object-format=foo unsupported' '
+   mkconfig 1 "objectFormat = foo" >.git/config &&
+   check_abort
+'
+
 test_done
-- 
2.14.3



[PATCH v4 09/12] sha1_file: convert write_sha1_file to object_id

2018-01-27 Thread Patryk Obara
Convert the definition and declaration of write_sha1_file to
struct object_id and adjust usage of this function.

This commit also converts static function write_sha1_file_prepare, as it
is closely related.

Rename these functions to write_object_file and
write_object_file_prepare respectively.

Replace sha1_to_hex, hashcpy and hashclr with their oid equivalents
wherever possible.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 apply.c  |  8 
 builtin/checkout.c   |  3 +--
 builtin/mktag.c  |  6 +++---
 builtin/mktree.c | 10 +-
 builtin/notes.c  |  8 
 builtin/receive-pack.c   | 11 ++-
 builtin/replace.c|  2 +-
 builtin/tag.c|  2 +-
 builtin/unpack-objects.c |  9 ++---
 cache-tree.c |  5 +++--
 cache.h  |  4 +++-
 commit.c |  2 +-
 match-trees.c|  2 +-
 merge-recursive.c|  5 +++--
 notes-cache.c|  2 +-
 notes.c  |  9 -
 read-cache.c |  6 +++---
 sha1_file.c  | 29 +++--
 18 files changed, 65 insertions(+), 58 deletions(-)

diff --git a/apply.c b/apply.c
index 57ab8a8a29..4cd4504008 100644
--- a/apply.c
+++ b/apply.c
@@ -3554,7 +3554,7 @@ static int try_threeway(struct apply_state *state,
 
/* Preimage the patch was prepared for */
if (patch->is_new)
-   write_sha1_file("", 0, blob_type, pre_oid.hash);
+   write_object_file("", 0, blob_type, _oid);
else if (get_oid(patch->old_sha1_prefix, _oid) ||
 read_blob_object(, _oid, patch->old_mode))
return error(_("repository lacks the necessary blob to fall 
back on 3-way merge."));
@@ -3570,7 +3570,7 @@ static int try_threeway(struct apply_state *state,
return -1;
}
/* post_oid is theirs */
-   write_sha1_file(tmp_image.buf, tmp_image.len, blob_type, post_oid.hash);
+   write_object_file(tmp_image.buf, tmp_image.len, blob_type, _oid);
clear_image(_image);
 
/* our_oid is ours */
@@ -3583,7 +3583,7 @@ static int try_threeway(struct apply_state *state,
return error(_("cannot read the current contents of 
'%s'"),
 patch->old_name);
}
-   write_sha1_file(tmp_image.buf, tmp_image.len, blob_type, our_oid.hash);
+   write_object_file(tmp_image.buf, tmp_image.len, blob_type, _oid);
clear_image(_image);
 
/* in-core three-way merge between post and our using pre as base */
@@ -4291,7 +4291,7 @@ static int add_index_file(struct apply_state *state,
}
fill_stat_cache_info(ce, );
}
-   if (write_sha1_file(buf, size, blob_type, ce->oid.hash) < 0) {
+   if (write_object_file(buf, size, blob_type, >oid) < 0) {
free(ce);
return error(_("unable to create backing store "
   "for newly created file %s"), path);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index c54c78df54..191b96c49c 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -227,8 +227,7 @@ static int checkout_merged(int pos, const struct checkout 
*state)
 * (it also writes the merge result to the object database even
 * when it may contain conflicts).
 */
-   if (write_sha1_file(result_buf.ptr, result_buf.size,
-   blob_type, oid.hash))
+   if (write_object_file(result_buf.ptr, result_buf.size, blob_type, ))
die(_("Unable to add merge result for '%s'"), path);
free(result_buf.ptr);
ce = make_cache_entry(mode, oid.hash, path, 2, 0);
diff --git a/builtin/mktag.c b/builtin/mktag.c
index 031b750f06..beb552847b 100644
--- a/builtin/mktag.c
+++ b/builtin/mktag.c
@@ -151,7 +151,7 @@ static int verify_tag(char *buffer, unsigned long size)
 int cmd_mktag(int argc, const char **argv, const char *prefix)
 {
struct strbuf buf = STRBUF_INIT;
-   unsigned char result_sha1[20];
+   struct object_id result;
 
if (argc != 1)
usage("git mktag");
@@ -165,10 +165,10 @@ int cmd_mktag(int argc, const char **argv, const char 
*prefix)
if (verify_tag(buf.buf, buf.len) < 0)
die("invalid tag signature file");
 
-   if (write_sha1_file(buf.buf, buf.len, tag_type, result_sha1) < 0)
+   if (write_object_file(buf.buf, buf.len, tag_type, ) < 0)
die("unable to write tag file");
 
strbuf_release();
-   printf("%s\n", sha1_to_hex(result_sha1));
+   printf("%s\n", oid_to_hex());
return 0;
 }
diff --git a/builtin/mktree.c b/builtin/mkt

[PATCH v4 02/12] dir: convert struct sha1_stat to use object_id

2018-01-27 Thread Patryk Obara
Convert the declaration of struct sha1_stat. Adjust all usages of this
struct and replace hash{clr,cmp,cpy} with oid{clr,cmp,cpy} wherever
possible.  Rename it to struct oid_stat.

Rename static function load_sha1_stat to load_oid_stat.

Remove macro EMPTY_BLOB_SHA1_BIN, as it's no longer used.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 cache.h  |   2 -
 dir.c| 104 +--
 dir.h|  12 ++--
 t/helper/test-dump-untracked-cache.c |   4 +-
 4 files changed, 58 insertions(+), 64 deletions(-)

diff --git a/cache.h b/cache.h
index e4e03ac51d..ed72933ba7 100644
--- a/cache.h
+++ b/cache.h
@@ -1047,8 +1047,6 @@ extern const struct object_id empty_tree_oid;
"\xe6\x9d\xe2\x9b\xb2\xd1\xd6\x43\x4b\x8b" \
"\x29\xae\x77\x5a\xd8\xc2\xe4\x8c\x53\x91"
 extern const struct object_id empty_blob_oid;
-#define EMPTY_BLOB_SHA1_BIN (empty_blob_oid.hash)
-
 
 static inline int is_empty_blob_sha1(const unsigned char *sha1)
 {
diff --git a/dir.c b/dir.c
index 7c4b45e30e..22cadbda9d 100644
--- a/dir.c
+++ b/dir.c
@@ -231,12 +231,10 @@ int within_depth(const char *name, int namelen,
  * 1 along with { data, size } of the (possibly augmented) buffer
  *   when successful.
  *
- * Optionally updates the given sha1_stat with the given OID (when valid).
+ * Optionally updates the given oid_stat with the given OID (when valid).
  */
-static int do_read_blob(const struct object_id *oid,
-   struct sha1_stat *sha1_stat,
-   size_t *size_out,
-   char **data_out)
+static int do_read_blob(const struct object_id *oid, struct oid_stat *oid_stat,
+   size_t *size_out, char **data_out)
 {
enum object_type type;
unsigned long sz;
@@ -251,9 +249,9 @@ static int do_read_blob(const struct object_id *oid,
return -1;
}
 
-   if (sha1_stat) {
-   memset(_stat->stat, 0, sizeof(sha1_stat->stat));
-   hashcpy(sha1_stat->sha1, oid->hash);
+   if (oid_stat) {
+   memset(_stat->stat, 0, sizeof(oid_stat->stat));
+   oidcpy(_stat->oid, oid);
}
 
if (sz == 0) {
@@ -654,9 +652,8 @@ void add_exclude(const char *string, const char *base,
 
 static int read_skip_worktree_file_from_index(const struct index_state *istate,
  const char *path,
- size_t *size_out,
- char **data_out,
- struct sha1_stat *sha1_stat)
+ size_t *size_out, char **data_out,
+ struct oid_stat *oid_stat)
 {
int pos, len;
 
@@ -667,7 +664,7 @@ static int read_skip_worktree_file_from_index(const struct 
index_state *istate,
if (!ce_skip_worktree(istate->cache[pos]))
return -1;
 
-   return do_read_blob(>cache[pos]->oid, sha1_stat, size_out, 
data_out);
+   return do_read_blob(>cache[pos]->oid, oid_stat, size_out, 
data_out);
 }
 
 /*
@@ -795,9 +792,8 @@ static int add_excludes_from_buffer(char *buf, size_t size,
  * ss_valid is non-zero, "ss" must contain good value as input.
  */
 static int add_excludes(const char *fname, const char *base, int baselen,
-   struct exclude_list *el,
-   struct index_state *istate,
-   struct sha1_stat *sha1_stat)
+   struct exclude_list *el, struct index_state *istate,
+   struct oid_stat *oid_stat)
 {
struct stat st;
int r;
@@ -815,16 +811,16 @@ static int add_excludes(const char *fname, const char 
*base, int baselen,
return -1;
r = read_skip_worktree_file_from_index(istate, fname,
   , ,
-  sha1_stat);
+  oid_stat);
if (r != 1)
return r;
} else {
size = xsize_t(st.st_size);
if (size == 0) {
-   if (sha1_stat) {
-   fill_stat_data(_stat->stat, );
-   hashcpy(sha1_stat->sha1, EMPTY_BLOB_SHA1_BIN);
-   sha1_stat->valid = 1;
+   if (oid_stat) {
+   fill_stat_data(_stat->stat, );
+   oidcpy(_stat->oid, _blob_oid);
+   oid_stat->valid = 1;
}
close(fd);
return 0;
@@ -8

[PATCH v4 04/12] cache: clear whole hash buffer with oidclr

2018-01-27 Thread Patryk Obara
As long as GIT_SHA1_RAWSZ is equal to GIT_MAX_RAWSZ there's no problem,
but when new hashing algorithm will be in place this memset will clear
only 20-byte prefix of hash buffer.

Alternatively, hashclr implementation could be adjusted, but this
function is almost removed from codebase already.  Separate
implementation of oidclr prevents potential buffer overrun in case
someone incorrectly used hashclr on object_id in future.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 cache.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cache.h b/cache.h
index 08f2b81e1b..d5d78d6a51 100644
--- a/cache.h
+++ b/cache.h
@@ -1029,7 +1029,7 @@ static inline void hashclr(unsigned char *hash)
 
 static inline void oidclr(struct object_id *oid)
 {
-   hashclr(oid->hash);
+   memset(oid->hash, 0, GIT_MAX_RAWSZ);
 }
 
 
-- 
2.14.3



[PATCH v4 11/12] sha1_file: convert write_loose_object to object_id

2018-01-27 Thread Patryk Obara
Convert the definition and declaration of static write_loose_object
function to struct object_id.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 sha1_file.c | 28 
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index d9ee966d74..59238f5bea 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1548,16 +1548,17 @@ static int create_tmpfile(struct strbuf *tmp, const 
char *filename)
return fd;
 }
 
-static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen,
- const void *buf, unsigned long len, time_t mtime)
+static int write_loose_object(const struct object_id *oid, char *hdr,
+ int hdrlen, const void *buf, unsigned long len,
+ time_t mtime)
 {
int fd, ret;
unsigned char compressed[4096];
git_zstream stream;
git_SHA_CTX c;
-   unsigned char parano_sha1[20];
+   struct object_id parano_oid;
static struct strbuf tmp_file = STRBUF_INIT;
-   const char *filename = sha1_file_name(sha1);
+   const char *filename = sha1_file_name(oid->hash);
 
fd = create_tmpfile(_file, filename);
if (fd < 0) {
@@ -1594,13 +1595,16 @@ static int write_loose_object(const unsigned char 
*sha1, char *hdr, int hdrlen,
} while (ret == Z_OK);
 
if (ret != Z_STREAM_END)
-   die("unable to deflate new object %s (%d)", sha1_to_hex(sha1), 
ret);
+   die("unable to deflate new object %s (%d)", oid_to_hex(oid),
+   ret);
ret = git_deflate_end_gently();
if (ret != Z_OK)
-   die("deflateEnd on object %s failed (%d)", sha1_to_hex(sha1), 
ret);
-   git_SHA1_Final(parano_sha1, );
-   if (hashcmp(sha1, parano_sha1) != 0)
-   die("confused by unstable object source data for %s", 
sha1_to_hex(sha1));
+   die("deflateEnd on object %s failed (%d)", oid_to_hex(oid),
+   ret);
+   git_SHA1_Final(parano_oid.hash, );
+   if (oidcmp(oid, _oid) != 0)
+   die("confused by unstable object source data for %s",
+   oid_to_hex(oid));
 
close_sha1_file(fd);
 
@@ -1645,7 +1649,7 @@ int write_object_file(const void *buf, unsigned long len, 
const char *type,
write_object_file_prepare(buf, len, type, oid, hdr, );
if (freshen_packed_object(oid->hash) || freshen_loose_object(oid->hash))
return 0;
-   return write_loose_object(oid->hash, hdr, hdrlen, buf, len, 0);
+   return write_loose_object(oid, hdr, hdrlen, buf, len, 0);
 }
 
 int hash_sha1_file_literally(const void *buf, unsigned long len, const char 
*type,
@@ -1663,7 +1667,7 @@ int hash_sha1_file_literally(const void *buf, unsigned 
long len, const char *typ
goto cleanup;
if (freshen_packed_object(oid->hash) || freshen_loose_object(oid->hash))
goto cleanup;
-   status = write_loose_object(oid->hash, header, hdrlen, buf, len, 0);
+   status = write_loose_object(oid, header, hdrlen, buf, len, 0);
 
 cleanup:
free(header);
@@ -1685,7 +1689,7 @@ int force_object_loose(const struct object_id *oid, 
time_t mtime)
if (!buf)
return error("cannot read sha1_file for %s", oid_to_hex(oid));
hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", typename(type), len) + 1;
-   ret = write_loose_object(oid->hash, hdr, hdrlen, buf, len, mtime);
+   ret = write_loose_object(oid, hdr, hdrlen, buf, len, mtime);
free(buf);
 
return ret;
-- 
2.14.3



[PATCH v4 10/12] sha1_file: convert force_object_loose to object_id

2018-01-27 Thread Patryk Obara
Convert the definition and declaration of force_object_loose to
struct object_id and adjust usage of this function.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 builtin/pack-objects.c |  2 +-
 cache.h|  3 ++-
 sha1_file.c| 10 +-
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 6b9cfc289d..f38197543d 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2768,7 +2768,7 @@ static void loosen_unused_packed_objects(struct rev_info 
*revs)
if (!packlist_find(_pack, oid.hash, NULL) &&
!has_sha1_pack_kept_or_nonlocal() &&
!loosened_object_can_be_discarded(, p->mtime))
-   if (force_object_loose(oid.hash, p->mtime))
+   if (force_object_loose(, p->mtime))
die("unable to force loose object");
}
}
diff --git a/cache.h b/cache.h
index d80141eb64..0a8be9c87f 100644
--- a/cache.h
+++ b/cache.h
@@ -1248,7 +1248,8 @@ extern int hash_sha1_file_literally(const void *buf, 
unsigned long len, const ch
 extern int pretend_object_file(void *, unsigned long, enum object_type,
   struct object_id *oid);
 
-extern int force_object_loose(const unsigned char *sha1, time_t mtime);
+extern int force_object_loose(const struct object_id *oid, time_t mtime);
+
 extern int git_open_cloexec(const char *name, int flags);
 #define git_open(name) git_open_cloexec(name, O_RDONLY)
 extern void *map_sha1_file(const unsigned char *sha1, unsigned long *size);
diff --git a/sha1_file.c b/sha1_file.c
index d1569b1b96..d9ee966d74 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1670,7 +1670,7 @@ int hash_sha1_file_literally(const void *buf, unsigned 
long len, const char *typ
return status;
 }
 
-int force_object_loose(const unsigned char *sha1, time_t mtime)
+int force_object_loose(const struct object_id *oid, time_t mtime)
 {
void *buf;
unsigned long len;
@@ -1679,13 +1679,13 @@ int force_object_loose(const unsigned char *sha1, 
time_t mtime)
int hdrlen;
int ret;
 
-   if (has_loose_object(sha1))
+   if (has_loose_object(oid->hash))
return 0;
-   buf = read_object(sha1, , );
+   buf = read_object(oid->hash, , );
if (!buf)
-   return error("cannot read sha1_file for %s", sha1_to_hex(sha1));
+   return error("cannot read sha1_file for %s", oid_to_hex(oid));
hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", typename(type), len) + 1;
-   ret = write_loose_object(sha1, hdr, hdrlen, buf, len, mtime);
+   ret = write_loose_object(oid->hash, hdr, hdrlen, buf, len, mtime);
free(buf);
 
return ret;
-- 
2.14.3



[PATCH v4 07/12] notes: convert combine_notes_* to object_id

2018-01-27 Thread Patryk Obara
Convert the definition and declarations of combine_notes_* functions
to struct object_id and adjust usage of these functions.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 notes.c | 46 +++---
 notes.h | 25 +++--
 2 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/notes.c b/notes.c
index c7f21fae44..3f4f94507a 100644
--- a/notes.c
+++ b/notes.c
@@ -270,8 +270,8 @@ static int note_tree_insert(struct notes_tree *t, struct 
int_node *tree,
if (!oidcmp(>val_oid, >val_oid))
return 0;
 
-   ret = combine_notes(l->val_oid.hash,
-   entry->val_oid.hash);
+   ret = combine_notes(>val_oid,
+   >val_oid);
if (!ret && is_null_oid(>val_oid))
note_tree_remove(t, tree, n, entry);
free(entry);
@@ -786,8 +786,8 @@ static int prune_notes_helper(const struct object_id 
*object_oid,
return 0;
 }
 
-int combine_notes_concatenate(unsigned char *cur_sha1,
-   const unsigned char *new_sha1)
+int combine_notes_concatenate(struct object_id *cur_oid,
+ const struct object_id *new_oid)
 {
char *cur_msg = NULL, *new_msg = NULL, *buf;
unsigned long cur_len, new_len, buf_len;
@@ -795,18 +795,18 @@ int combine_notes_concatenate(unsigned char *cur_sha1,
int ret;
 
/* read in both note blob objects */
-   if (!is_null_sha1(new_sha1))
-   new_msg = read_sha1_file(new_sha1, _type, _len);
+   if (!is_null_oid(new_oid))
+   new_msg = read_sha1_file(new_oid->hash, _type, _len);
if (!new_msg || !new_len || new_type != OBJ_BLOB) {
free(new_msg);
return 0;
}
-   if (!is_null_sha1(cur_sha1))
-   cur_msg = read_sha1_file(cur_sha1, _type, _len);
+   if (!is_null_oid(cur_oid))
+   cur_msg = read_sha1_file(cur_oid->hash, _type, _len);
if (!cur_msg || !cur_len || cur_type != OBJ_BLOB) {
free(cur_msg);
free(new_msg);
-   hashcpy(cur_sha1, new_sha1);
+   oidcpy(cur_oid, new_oid);
return 0;
}
 
@@ -825,20 +825,20 @@ int combine_notes_concatenate(unsigned char *cur_sha1,
free(new_msg);
 
/* create a new blob object from buf */
-   ret = write_sha1_file(buf, buf_len, blob_type, cur_sha1);
+   ret = write_sha1_file(buf, buf_len, blob_type, cur_oid->hash);
free(buf);
return ret;
 }
 
-int combine_notes_overwrite(unsigned char *cur_sha1,
-   const unsigned char *new_sha1)
+int combine_notes_overwrite(struct object_id *cur_oid,
+   const struct object_id *new_oid)
 {
-   hashcpy(cur_sha1, new_sha1);
+   oidcpy(cur_oid, new_oid);
return 0;
 }
 
-int combine_notes_ignore(unsigned char *cur_sha1,
-   const unsigned char *new_sha1)
+int combine_notes_ignore(struct object_id *cur_oid,
+const struct object_id *new_oid)
 {
return 0;
 }
@@ -848,17 +848,17 @@ int combine_notes_ignore(unsigned char *cur_sha1,
  * newlines removed.
  */
 static int string_list_add_note_lines(struct string_list *list,
- const unsigned char *sha1)
+ const struct object_id *oid)
 {
char *data;
unsigned long len;
enum object_type t;
 
-   if (is_null_sha1(sha1))
+   if (is_null_oid(oid))
return 0;
 
/* read_sha1_file NUL-terminates */
-   data = read_sha1_file(sha1, , );
+   data = read_sha1_file(oid->hash, , );
if (t != OBJ_BLOB || !data || !len) {
free(data);
return t != OBJ_BLOB || !data;
@@ -884,17 +884,17 @@ static int string_list_join_lines_helper(struct 
string_list_item *item,
return 0;
 }
 
-int combine_notes_cat_sort_uniq(unsigned char *cur_sha1,
-   const unsigned char *new_sha1)
+int combine_notes_cat_sort_uniq(struct object_id *cur_oid,
+   const struct object_id *new_oid)
 {
struct string_list sort_uniq_list = STRING_LIST_INIT_DUP;
struct strbuf buf = STRBUF_INIT;
int ret = 1;
 
/* read both note blob objects into unique_lines */
-   if (string_list_add_note_lines(_uniq_list, cur_sha1))
+   if (string_list_add_note_lines(_uniq_list, cur_oid))
goto out;
-   if (string_list_add_note_lines(_uniq_list, new_sha1))
+   if (string_list_add_note_lines(_uniq_list, new_oid))
goto out;
string_list_remove_empty_items(_uni

[PATCH v4 06/12] commit: convert commit_tree* to object_id

2018-01-27 Thread Patryk Obara
Convert the definitions and declarations of commit_tree and
commit_tree_extended to use struct object_id and adjust all usages of
these functions.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 builtin/am.c  |  4 ++--
 builtin/commit-tree.c |  4 ++--
 builtin/commit.c  |  5 +++--
 builtin/merge.c   |  8 
 commit.c  | 15 +++
 commit.h  | 11 ++-
 notes-cache.c |  4 ++--
 notes-merge.c |  9 -
 notes-utils.c |  7 ---
 notes-utils.h |  3 ++-
 10 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index acfe9d3c8c..6e6abb05cd 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1641,8 +1641,8 @@ static void do_commit(const struct am_state *state)
setenv("GIT_COMMITTER_DATE",
state->ignore_date ? "" : state->author_date, 1);
 
-   if (commit_tree(state->msg, state->msg_len, tree.hash, parents, 
commit.hash,
-   author, state->sign_commit))
+   if (commit_tree(state->msg, state->msg_len, , parents, ,
+   author, state->sign_commit))
die(_("failed to write commit object"));
 
reflog_msg = getenv("GIT_REFLOG_ACTION");
diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
index 2177251e24..e5bdf57b1e 100644
--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c
@@ -117,8 +117,8 @@ int cmd_commit_tree(int argc, const char **argv, const char 
*prefix)
die_errno("git commit-tree: failed to read");
}
 
-   if (commit_tree(buffer.buf, buffer.len, tree_oid.hash, parents,
-   commit_oid.hash, NULL, sign_commit)) {
+   if (commit_tree(buffer.buf, buffer.len, _oid, parents, _oid,
+   NULL, sign_commit)) {
strbuf_release();
return 1;
}
diff --git a/builtin/commit.c b/builtin/commit.c
index 4610e3d8e3..e5974a5999 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1794,8 +1794,9 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
append_merge_tag_headers(parents, );
}
 
-   if (commit_tree_extended(sb.buf, sb.len, active_cache_tree->oid.hash,
-parents, oid.hash, author_ident.buf, sign_commit, 
extra)) {
+   if (commit_tree_extended(sb.buf, sb.len, _cache_tree->oid,
+parents, , author_ident.buf, sign_commit,
+extra)) {
rollback_index_files();
die(_("failed to write commit object"));
}
diff --git a/builtin/merge.c b/builtin/merge.c
index 30264cfd7c..92ba99a1a5 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -820,8 +820,8 @@ static int merge_trivial(struct commit *head, struct 
commit_list *remoteheads)
pptr = commit_list_append(head, pptr);
pptr = commit_list_append(remoteheads->item, pptr);
prepare_to_commit(remoteheads);
-   if (commit_tree(merge_msg.buf, merge_msg.len, result_tree.hash, parents,
-   result_commit.hash, NULL, sign_commit))
+   if (commit_tree(merge_msg.buf, merge_msg.len, _tree, parents,
+   _commit, NULL, sign_commit))
die(_("failed to write commit object"));
finish(head, remoteheads, _commit, "In-index merge");
drop_save();
@@ -845,8 +845,8 @@ static int finish_automerge(struct commit *head,
commit_list_insert(head, );
strbuf_addch(_msg, '\n');
prepare_to_commit(remoteheads);
-   if (commit_tree(merge_msg.buf, merge_msg.len, result_tree->hash, 
parents,
-   result_commit.hash, NULL, sign_commit))
+   if (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents,
+   _commit, NULL, sign_commit))
die(_("failed to write commit object"));
strbuf_addf(, "Merge made by the '%s' strategy.", wt_strategy);
finish(head, remoteheads, _commit, buf.buf);
diff --git a/commit.c b/commit.c
index ff51c9f34a..643f3daec3 100644
--- a/commit.c
+++ b/commit.c
@@ -1380,9 +1380,8 @@ void free_commit_extra_headers(struct commit_extra_header 
*extra)
}
 }
 
-int commit_tree(const char *msg, size_t msg_len,
-   const unsigned char *tree,
-   struct commit_list *parents, unsigned char *ret,
+int commit_tree(const char *msg, size_t msg_len, const struct object_id *tree,
+   struct commit_list *parents, struct object_id *ret,
const char *author, const char *sign_commit)
 {
struct commit_extra_header *extra = NULL, **tail = 
@@ -1511,8 +1510,8 @@ N_("Warning: commit message did not conform to UTF-8.\n"
"variable i18

[PATCH v4 00/12] A bunch of object_id conversions

2018-01-27 Thread Patryk Obara
Thank you, everyone, for review and thanks, Junio for queueing two
first patches - I removed them from v4, as there were no more comments
to them anyway.

Compared to v3:

Patch 2 (formerly 4)
- renamed all parameter names sha1_stat to oid_stat
- renamed static function load sha1_stat to load_oid_stat
- adjusted a comment, that included "sha1_stat" in text

Patch 11 (formerly 13)
- fixed a typo in the commit message

Hopefully, this will be the last iteration on this batch of object_id
conversions; I already have next batch prepared (I just need to clean
it up before sending).

Patryk Obara (12):
  sha1_file: convert pretend_sha1_file to object_id
  dir: convert struct sha1_stat to use object_id
  sha1_file: convert hash_sha1_file to object_id
  cache: clear whole hash buffer with oidclr
  match-trees: convert splice_tree to object_id
  commit: convert commit_tree* to object_id
  notes: convert combine_notes_* to object_id
  notes: convert write_notes_tree to object_id
  sha1_file: convert write_sha1_file to object_id
  sha1_file: convert force_object_loose to object_id
  sha1_file: convert write_loose_object to object_id
  sha1_file: rename hash_sha1_file_literally

 Documentation/technical/api-object-access.txt |   2 +-
 apply.c   |  12 +--
 blame.c   |   2 +-
 builtin/am.c  |   4 +-
 builtin/checkout.c|   3 +-
 builtin/commit-tree.c |   4 +-
 builtin/commit.c  |   5 +-
 builtin/hash-object.c |   3 +-
 builtin/index-pack.c  |   5 +-
 builtin/merge.c   |   8 +-
 builtin/mktag.c   |   6 +-
 builtin/mktree.c  |  10 +--
 builtin/notes.c   |   8 +-
 builtin/pack-objects.c|   2 +-
 builtin/receive-pack.c|  11 +--
 builtin/replace.c |   4 +-
 builtin/tag.c |   2 +-
 builtin/unpack-objects.c  |  11 ++-
 cache-tree.c  |  16 ++--
 cache.h   |  25 +--
 commit.c  |  15 ++--
 commit.h  |  11 +--
 convert.c |   6 +-
 diffcore-rename.c |   4 +-
 dir.c | 104 +-
 dir.h |  12 +--
 log-tree.c|   2 +-
 match-trees.c |  46 ++--
 merge-recursive.c |   5 +-
 notes-cache.c |   8 +-
 notes-merge.c |   9 +--
 notes-utils.c |   9 ++-
 notes-utils.h |   3 +-
 notes.c   |  63 
 notes.h   |  29 ---
 read-cache.c  |   6 +-
 sha1_file.c   | 100 +
 t/helper/test-dump-untracked-cache.c  |   4 +-
 38 files changed, 300 insertions(+), 279 deletions(-)


base-commit: 5be1f00a9a701532232f57958efab4be8c959a29
-- 
2.14.3



[PATCH v4 03/12] sha1_file: convert hash_sha1_file to object_id

2018-01-27 Thread Patryk Obara
Convert the declaration and definition of hash_sha1_file to use
struct object_id and adjust all function calls.

Rename this function to hash_object_file.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 apply.c  |  4 ++--
 builtin/index-pack.c |  5 ++---
 builtin/replace.c|  2 +-
 builtin/unpack-objects.c |  2 +-
 cache-tree.c | 11 +--
 cache.h  |  5 -
 convert.c|  6 +++---
 diffcore-rename.c|  4 ++--
 dir.c|  4 ++--
 log-tree.c   |  2 +-
 sha1_file.c  | 26 +-
 11 files changed, 36 insertions(+), 35 deletions(-)

diff --git a/apply.c b/apply.c
index 321a9fa68d..57ab8a8a29 100644
--- a/apply.c
+++ b/apply.c
@@ -3154,7 +3154,7 @@ static int apply_binary(struct apply_state *state,
 * See if the old one matches what the patch
 * applies to.
 */
-   hash_sha1_file(img->buf, img->len, blob_type, oid.hash);
+   hash_object_file(img->buf, img->len, blob_type, );
if (strcmp(oid_to_hex(), patch->old_sha1_prefix))
return error(_("the patch applies to '%s' (%s), "
   "which does not match the "
@@ -3199,7 +3199,7 @@ static int apply_binary(struct apply_state *state,
 name);
 
/* verify that the result matches */
-   hash_sha1_file(img->buf, img->len, blob_type, oid.hash);
+   hash_object_file(img->buf, img->len, blob_type, );
if (strcmp(oid_to_hex(), patch->new_sha1_prefix))
return error(_("binary patch to '%s' creates incorrect 
result (expecting %s, got %s)"),
name, patch->new_sha1_prefix, oid_to_hex());
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 4c51aec81f..7f5a95e6ff 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -958,9 +958,8 @@ static void resolve_delta(struct object_entry *delta_obj,
free(delta_data);
if (!result->data)
bad_object(delta_obj->idx.offset, _("failed to apply delta"));
-   hash_sha1_file(result->data, result->size,
-  typename(delta_obj->real_type),
-  delta_obj->idx.oid.hash);
+   hash_object_file(result->data, result->size,
+typename(delta_obj->real_type), _obj->idx.oid);
sha1_object(result->data, NULL, result->size, delta_obj->real_type,
_obj->idx.oid);
counter_lock();
diff --git a/builtin/replace.c b/builtin/replace.c
index 10078ae371..814bf6bfde 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -355,7 +355,7 @@ static void check_one_mergetag(struct commit *commit,
struct tag *tag;
int i;
 
-   hash_sha1_file(extra->value, extra->len, typename(OBJ_TAG), 
tag_oid.hash);
+   hash_object_file(extra->value, extra->len, typename(OBJ_TAG), _oid);
tag = lookup_tag(_oid);
if (!tag)
die(_("bad mergetag in commit '%s'"), ref);
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 62ea264c46..85a40d1af7 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -258,7 +258,7 @@ static void write_object(unsigned nr, enum object_type type,
} else {
struct object *obj;
int eaten;
-   hash_sha1_file(buf, size, typename(type), 
obj_list[nr].oid.hash);
+   hash_object_file(buf, size, typename(type), _list[nr].oid);
added_object(nr, type, buf, size);
obj = parse_object_buffer(_list[nr].oid, type, size, buf,
  );
diff --git a/cache-tree.c b/cache-tree.c
index e03e72c34a..6574eeb80d 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -400,15 +400,14 @@ static int update_one(struct cache_tree *it,
}
 
if (repair) {
-   unsigned char sha1[20];
-   hash_sha1_file(buffer.buf, buffer.len, tree_type, sha1);
-   if (has_sha1_file(sha1))
-   hashcpy(it->oid.hash, sha1);
+   struct object_id oid;
+   hash_object_file(buffer.buf, buffer.len, tree_type, );
+   if (has_sha1_file(oid.hash))
+   oidcpy(>oid, );
else
to_invalidate = 1;
} else if (dryrun)
-   hash_sha1_file(buffer.buf, buffer.len, tree_type,
-  it->oid.hash);
+   hash_object_file(buffer.buf, buffer.len, tree_type, >oid);
else if (write_sha1_file(buffer.buf, buffer.len, tree_type, 
it->oid.hash)) {
s

[PATCH v4 12/12] sha1_file: rename hash_sha1_file_literally

2018-01-27 Thread Patryk Obara
This function was already converted to use struct object_id earlier.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 builtin/hash-object.c | 3 ++-
 cache.h   | 4 +++-
 sha1_file.c   | 5 +++--
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index c532ff9320..526da5c185 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -24,7 +24,8 @@ static int hash_literally(struct object_id *oid, int fd, 
const char *type, unsig
if (strbuf_read(, fd, 4096) < 0)
ret = -1;
else
-   ret = hash_sha1_file_literally(buf.buf, buf.len, type, oid, 
flags);
+   ret = hash_object_file_literally(buf.buf, buf.len, type, oid,
+flags);
strbuf_release();
return ret;
 }
diff --git a/cache.h b/cache.h
index 0a8be9c87f..6ef4248931 100644
--- a/cache.h
+++ b/cache.h
@@ -1243,7 +1243,9 @@ extern int hash_object_file(const void *buf, unsigned 
long len,
 extern int write_object_file(const void *buf, unsigned long len,
 const char *type, struct object_id *oid);
 
-extern int hash_sha1_file_literally(const void *buf, unsigned long len, const 
char *type, struct object_id *oid, unsigned flags);
+extern int hash_object_file_literally(const void *buf, unsigned long len,
+ const char *type, struct object_id *oid,
+ unsigned flags);
 
 extern int pretend_object_file(void *, unsigned long, enum object_type,
   struct object_id *oid);
diff --git a/sha1_file.c b/sha1_file.c
index 59238f5bea..34c041e8cd 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1652,8 +1652,9 @@ int write_object_file(const void *buf, unsigned long len, 
const char *type,
return write_loose_object(oid, hdr, hdrlen, buf, len, 0);
 }
 
-int hash_sha1_file_literally(const void *buf, unsigned long len, const char 
*type,
-struct object_id *oid, unsigned flags)
+int hash_object_file_literally(const void *buf, unsigned long len,
+  const char *type, struct object_id *oid,
+  unsigned flags)
 {
char *header;
int hdrlen, status = 0;
-- 
2.14.3



[PATCH v4 05/12] match-trees: convert splice_tree to object_id

2018-01-27 Thread Patryk Obara
Convert the definition of static recursive splice_tree function to use
struct object_id and adjust single caller.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 match-trees.c | 46 ++
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/match-trees.c b/match-trees.c
index 396b7338df..afb771c4f5 100644
--- a/match-trees.c
+++ b/match-trees.c
@@ -158,22 +158,20 @@ static void match_trees(const struct object_id *hash1,
 }
 
 /*
- * A tree "hash1" has a subdirectory at "prefix".  Come up with a
- * tree object by replacing it with another tree "hash2".
+ * A tree "oid1" has a subdirectory at "prefix".  Come up with a tree object by
+ * replacing it with another tree "oid2".
  */
-static int splice_tree(const unsigned char *hash1,
-  const char *prefix,
-  const unsigned char *hash2,
-  unsigned char *result)
+static int splice_tree(const struct object_id *oid1, const char *prefix,
+  const struct object_id *oid2, struct object_id *result)
 {
char *subpath;
int toplen;
char *buf;
unsigned long sz;
struct tree_desc desc;
-   unsigned char *rewrite_here;
-   const unsigned char *rewrite_with;
-   unsigned char subtree[20];
+   struct object_id *rewrite_here;
+   const struct object_id *rewrite_with;
+   struct object_id subtree;
enum object_type type;
int status;
 
@@ -182,9 +180,9 @@ static int splice_tree(const unsigned char *hash1,
if (*subpath)
subpath++;
 
-   buf = read_sha1_file(hash1, , );
+   buf = read_sha1_file(oid1->hash, , );
if (!buf)
-   die("cannot read tree %s", sha1_to_hex(hash1));
+   die("cannot read tree %s", oid_to_hex(oid1));
init_tree_desc(, buf, sz);
 
rewrite_here = NULL;
@@ -197,26 +195,26 @@ static int splice_tree(const unsigned char *hash1,
if (strlen(name) == toplen &&
!memcmp(name, prefix, toplen)) {
if (!S_ISDIR(mode))
-   die("entry %s in tree %s is not a tree",
-   name, sha1_to_hex(hash1));
-   rewrite_here = (unsigned char *) oid->hash;
+   die("entry %s in tree %s is not a tree", name,
+   oid_to_hex(oid1));
+   rewrite_here = (struct object_id *)oid;
break;
}
update_tree_entry();
}
if (!rewrite_here)
-   die("entry %.*s not found in tree %s",
-   toplen, prefix, sha1_to_hex(hash1));
+   die("entry %.*s not found in tree %s", toplen, prefix,
+   oid_to_hex(oid1));
if (*subpath) {
-   status = splice_tree(rewrite_here, subpath, hash2, subtree);
+   status = splice_tree(rewrite_here, subpath, oid2, );
if (status)
return status;
-   rewrite_with = subtree;
+   rewrite_with = 
+   } else {
+   rewrite_with = oid2;
}
-   else
-   rewrite_with = hash2;
-   hashcpy(rewrite_here, rewrite_with);
-   status = write_sha1_file(buf, sz, tree_type, result);
+   oidcpy(rewrite_here, rewrite_with);
+   status = write_sha1_file(buf, sz, tree_type, result->hash);
free(buf);
return status;
 }
@@ -280,7 +278,7 @@ void shift_tree(const struct object_id *hash1,
if (!*add_prefix)
return;
 
-   splice_tree(hash1->hash, add_prefix, hash2->hash, shifted->hash);
+   splice_tree(hash1, add_prefix, hash2, shifted);
 }
 
 /*
@@ -334,7 +332,7 @@ void shift_tree_by(const struct object_id *hash1,
 * shift tree2 down by adding shift_prefix above it
 * to match tree1.
 */
-   splice_tree(hash1->hash, shift_prefix, hash2->hash, 
shifted->hash);
+   splice_tree(hash1, shift_prefix, hash2, shifted);
else
/*
 * shift tree2 up by removing shift_prefix from it
-- 
2.14.3



[PATCH v4 01/12] sha1_file: convert pretend_sha1_file to object_id

2018-01-27 Thread Patryk Obara
Convert the declaration and definition of pretend_sha1_file to use
struct object_id and adjust all usages of this function.  Rename it to
pretend_object_file.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 Documentation/technical/api-object-access.txt |  2 +-
 blame.c   |  2 +-
 cache.h   |  5 -
 sha1_file.c   | 10 +-
 4 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/Documentation/technical/api-object-access.txt 
b/Documentation/technical/api-object-access.txt
index 03bb0e950d..a1162e5bcd 100644
--- a/Documentation/technical/api-object-access.txt
+++ b/Documentation/technical/api-object-access.txt
@@ -7,7 +7,7 @@ Talk about  and  family, things like
 * read_object_with_reference()
 * has_sha1_file()
 * write_sha1_file()
-* pretend_sha1_file()
+* pretend_object_file()
 * lookup_{object,commit,tag,blob,tree}
 * parse_{object,commit,tag,blob,tree}
 * Use of object flags
diff --git a/blame.c b/blame.c
index 2893f3c103..1fc22b304b 100644
--- a/blame.c
+++ b/blame.c
@@ -232,7 +232,7 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,
convert_to_git(_index, path, buf.buf, buf.len, , 0);
origin->file.ptr = buf.buf;
origin->file.size = buf.len;
-   pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_oid.hash);
+   pretend_object_file(buf.buf, buf.len, OBJ_BLOB, >blob_oid);
 
/*
 * Read the current index, replace the path entry with
diff --git a/cache.h b/cache.h
index d8b975a571..e4e03ac51d 100644
--- a/cache.h
+++ b/cache.h
@@ -1241,7 +1241,10 @@ extern int sha1_object_info(const unsigned char *, 
unsigned long *);
 extern int hash_sha1_file(const void *buf, unsigned long len, const char 
*type, unsigned char *sha1);
 extern int write_sha1_file(const void *buf, unsigned long len, const char 
*type, unsigned char *return_sha1);
 extern int hash_sha1_file_literally(const void *buf, unsigned long len, const 
char *type, struct object_id *oid, unsigned flags);
-extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned 
char *);
+
+extern int pretend_object_file(void *, unsigned long, enum object_type,
+  struct object_id *oid);
+
 extern int force_object_loose(const unsigned char *sha1, time_t mtime);
 extern int git_open_cloexec(const char *name, int flags);
 #define git_open(name) git_open_cloexec(name, O_RDONLY)
diff --git a/sha1_file.c b/sha1_file.c
index 3da70ac650..830b93b428 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1312,13 +1312,13 @@ static void *read_object(const unsigned char *sha1, 
enum object_type *type,
return content;
 }
 
-int pretend_sha1_file(void *buf, unsigned long len, enum object_type type,
- unsigned char *sha1)
+int pretend_object_file(void *buf, unsigned long len, enum object_type type,
+   struct object_id *oid)
 {
struct cached_object *co;
 
-   hash_sha1_file(buf, len, typename(type), sha1);
-   if (has_sha1_file(sha1) || find_cached_object(sha1))
+   hash_sha1_file(buf, len, typename(type), oid->hash);
+   if (has_sha1_file(oid->hash) || find_cached_object(oid->hash))
return 0;
ALLOC_GROW(cached_objects, cached_object_nr + 1, cached_object_alloc);
co = _objects[cached_object_nr++];
@@ -1326,7 +1326,7 @@ int pretend_sha1_file(void *buf, unsigned long len, enum 
object_type type,
co->type = type;
co->buf = xmalloc(len);
memcpy(co->buf, buf, len);
-   hashcpy(co->sha1, sha1);
+   hashcpy(co->sha1, oid->hash);
return 0;
 }
 
-- 
2.14.3



[PATCH v4 08/12] notes: convert write_notes_tree to object_id

2018-01-27 Thread Patryk Obara
Convert the definition and declaration of write_notes_tree to
struct object_id and adjust usage of this function.

Additionally, improve style of small part of this function, as old
formatting made it hard to understand at glance what this part of
code is doing.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 notes-cache.c |  2 +-
 notes-utils.c |  2 +-
 notes.c   | 16 +---
 notes.h   |  4 ++--
 4 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/notes-cache.c b/notes-cache.c
index d2f87147cc..010ad236cb 100644
--- a/notes-cache.c
+++ b/notes-cache.c
@@ -54,7 +54,7 @@ int notes_cache_write(struct notes_cache *c)
if (!c->tree.dirty)
return 0;
 
-   if (write_notes_tree(>tree, tree_oid.hash))
+   if (write_notes_tree(>tree, _oid))
return -1;
if (commit_tree(c->validity, strlen(c->validity), _oid, NULL,
_oid, NULL, NULL) < 0)
diff --git a/notes-utils.c b/notes-utils.c
index 058c642dac..02407fe2a7 100644
--- a/notes-utils.c
+++ b/notes-utils.c
@@ -12,7 +12,7 @@ void create_notes_commit(struct notes_tree *t, struct 
commit_list *parents,
 
assert(t->initialized);
 
-   if (write_notes_tree(t, tree_oid.hash))
+   if (write_notes_tree(t, _oid))
die("Failed to write notes tree to database");
 
if (!parents) {
diff --git a/notes.c b/notes.c
index 3f4f94507a..09ef1ce33a 100644
--- a/notes.c
+++ b/notes.c
@@ -1123,11 +1123,12 @@ int for_each_note(struct notes_tree *t, int flags, 
each_note_fn fn,
return for_each_note_helper(t, t->root, 0, 0, flags, fn, cb_data);
 }
 
-int write_notes_tree(struct notes_tree *t, unsigned char *result)
+int write_notes_tree(struct notes_tree *t, struct object_id *result)
 {
struct tree_write_stack root;
struct write_each_note_data cb_data;
int ret;
+   int flags;
 
if (!t)
t = _notes_tree;
@@ -1141,12 +1142,13 @@ int write_notes_tree(struct notes_tree *t, unsigned 
char *result)
cb_data.next_non_note = t->first_non_note;
 
/* Write tree objects representing current notes tree */
-   ret = for_each_note(t, FOR_EACH_NOTE_DONT_UNPACK_SUBTREES |
-   FOR_EACH_NOTE_YIELD_SUBTREES,
-   write_each_note, _data) ||
-   write_each_non_note_until(NULL, _data) ||
-   tree_write_stack_finish_subtree() ||
-   write_sha1_file(root.buf.buf, root.buf.len, tree_type, result);
+   flags = FOR_EACH_NOTE_DONT_UNPACK_SUBTREES |
+   FOR_EACH_NOTE_YIELD_SUBTREES;
+   ret = for_each_note(t, flags, write_each_note, _data) ||
+ write_each_non_note_until(NULL, _data) ||
+ tree_write_stack_finish_subtree() ||
+ write_sha1_file(root.buf.buf, root.buf.len, tree_type,
+ result->hash);
strbuf_release();
return ret;
 }
diff --git a/notes.h b/notes.h
index 88da38b5f4..0433f45db5 100644
--- a/notes.h
+++ b/notes.h
@@ -217,7 +217,7 @@ int for_each_note(struct notes_tree *t, int flags, 
each_note_fn fn,
  * Write the given notes_tree structure to the object database
  *
  * Creates a new tree object encapsulating the current state of the given
- * notes_tree, and stores its SHA1 into the 'result' argument.
+ * notes_tree, and stores its object id into the 'result' argument.
  *
  * Returns zero on success, non-zero on failure.
  *
@@ -225,7 +225,7 @@ int for_each_note(struct notes_tree *t, int flags, 
each_note_fn fn,
  * this function has returned zero. Please also remember to create a
  * corresponding commit object, and update the appropriate notes ref.
  */
-int write_notes_tree(struct notes_tree *t, unsigned char *result);
+int write_notes_tree(struct notes_tree *t, struct object_id *result);
 
 /* Flags controlling the operation of prune */
 #define NOTES_PRUNE_VERBOSE 1
-- 
2.14.3



Re: [PATCH] setup: recognise extensions.objectFormat

2018-01-26 Thread Patryk Obara

On 26/01/2018 15:41, Johannes Schindelin wrote:


On Thu, 25 Jan 2018, Duy Nguyen wrote:


This config is so sensitive I wonder if we should forbid changing it
via git-config. You can't simply change this and expect anything to
work anyway.


I don't think it makes sense to forbid `git config` from changing these
values, as it is all-too-easy to change them via `git config -e` *anyway*.
And we already have the repositoryFormat precedent with the exact same
issue.

In my opinion, it would only complicate the code, for very little (if at
all noticable) benefit.


That's my sentiment as well, but some measure of user protection might 
be necessary.


I was thinking about doing a sanity check: when objectFormat is set,
re-hash N (randomly selected?) objects, where N is sufficient to get 
3-sigma confidence.


This might be necessary to prevent object store corruption e.g. when
objectFormat is set in repo, that is cloned with reference from repo 
without extension.


--
| ← Ceci n'est pas une pipe
Patryk Obara


Re: [PATCH] setup: recognise extensions.objectFormat

2018-01-24 Thread Patryk Obara

Argh! Forgot to sign-off the commit…

--
| ← Ceci n'est pas une pipe
Patryk Obara


[PATCH] setup: recognise extensions.objectFormat

2018-01-24 Thread Patryk Obara
This extension selects which hashing algorithm from vtable should be
used for reading and writing objects in the object store.  At the moment
supports only single value (sha-1).

In case value of objectFormat is an unknown hashing algorithm, Git
command will fail with following message:

  fatal: unknown repository extensions found:
  objectformat = 

To indicate, that this specific objectFormat value is not recognised.

The objectFormat extension is not allowed in repository marked as
version 0 to prevent any possibility of accidentally writing a NewHash
object in the sha-1 object store. This extension behaviour is different
than preciousObjects extension (which is allowed in repo version 0).

Add tests and documentation note about new extension.
---
 Documentation/technical/repository-version.txt |  8 
 setup.c| 27 ++
 t/t1302-repo-version.sh| 15 ++
 3 files changed, 50 insertions(+)

diff --git a/Documentation/technical/repository-version.txt 
b/Documentation/technical/repository-version.txt
index 00ad37986e..14a75a7fee 100644
--- a/Documentation/technical/repository-version.txt
+++ b/Documentation/technical/repository-version.txt
@@ -86,3 +86,11 @@ for testing format-1 compatibility.
 When the config key `extensions.preciousObjects` is set to `true`,
 objects in the repository MUST NOT be deleted (e.g., by `git-prune` or
 `git repack -d`).
+
+`objectFormat`
+~~
+
+This extension instructs Git to use a specific algorithm for addressing
+and interpreting objects in the object store. Currently, the only
+supported object format is `sha-1`.  See `hash-function-transition.txt`
+document for more detailed explanation.
diff --git a/setup.c b/setup.c
index 8cc34186ce..b48a90d9ce 100644
--- a/setup.c
+++ b/setup.c
@@ -405,6 +405,31 @@ void setup_work_tree(void)
initialized = 1;
 }
 
+static int find_object_format(const char *value)
+{
+   int i;
+   for (i = GIT_HASH_SHA1; i < GIT_HASH_NALGOS; ++i) {
+   if (strcmp(value, hash_algos[i].name) == 0)
+   return i;
+   }
+   return GIT_HASH_UNKNOWN;
+}
+
+static void detect_object_format(struct repository_format *data,
+const char *value)
+{
+   if (data->version == 0)
+   die("invalid repository format version");
+
+   data->hash_algo = find_object_format(value);
+   if (data->hash_algo == GIT_HASH_UNKNOWN) {
+   char object_format[25];
+   xsnprintf(object_format, sizeof(object_format),
+ "objectformat = %s", value);
+   string_list_append(>unknown_extensions, object_format);
+   }
+}
+
 static int check_repo_format(const char *var, const char *value, void *vdata)
 {
struct repository_format *data = vdata;
@@ -422,6 +447,8 @@ static int check_repo_format(const char *var, const char 
*value, void *vdata)
;
else if (!strcmp(ext, "preciousobjects"))
data->precious_objects = git_config_bool(var, value);
+   else if (!strcmp(ext, "objectformat"))
+   detect_object_format(data, value);
else
string_list_append(>unknown_extensions, ext);
} else if (strcmp(var, "core.bare") == 0) {
diff --git a/t/t1302-repo-version.sh b/t/t1302-repo-version.sh
index ce4cff13bb..227b397ff2 100755
--- a/t/t1302-repo-version.sh
+++ b/t/t1302-repo-version.sh
@@ -107,4 +107,19 @@ test_expect_success 'gc runs without complaint' '
git gc
 '
 
+test_expect_success 'object-format not allowed in repo version=0' '
+   mkconfig 0 "objectFormat = sha-1" >.git/config &&
+   check_abort
+'
+
+test_expect_success 'object-format=sha-1 allowed' '
+   mkconfig 1 "objectFormat = sha-1" >.git/config &&
+   check_allow
+'
+
+test_expect_success 'object-format=foo unsupported' '
+   mkconfig 1 "objectFormat = foo" >.git/config &&
+   check_abort
+'
+
 test_done

base-commit: 5be1f00a9a701532232f57958efab4be8c959a29
-- 
2.14.3



[PATCH v3 06/14] cache: clear whole hash buffer with oidclr

2018-01-24 Thread Patryk Obara
As long as GIT_SHA1_RAWSZ is equal to GIT_MAX_RAWSZ there's no problem,
but when new hashing algorithm will be in place this memset will clear
only 20-byte prefix of hash buffer.

Alternatively, hashclr implementation could be adjusted, but this
function is almost removed from codebase already.  Separate
implementation of oidclr prevents potential buffer overrun in case
someone incorrectly used hashclr on object_id in future.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 cache.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cache.h b/cache.h
index 08f2b81e1b..d5d78d6a51 100644
--- a/cache.h
+++ b/cache.h
@@ -1029,7 +1029,7 @@ static inline void hashclr(unsigned char *hash)
 
 static inline void oidclr(struct object_id *oid)
 {
-   hashclr(oid->hash);
+   memset(oid->hash, 0, GIT_MAX_RAWSZ);
 }
 
 
-- 
2.14.3



[PATCH v3 03/14] sha1_file: convert pretend_sha1_file to object_id

2018-01-24 Thread Patryk Obara
Convert the declaration and definition of pretend_sha1_file to use
struct object_id and adjust all usages of this function.  Rename it to
pretend_object_file.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 Documentation/technical/api-object-access.txt |  2 +-
 blame.c   |  2 +-
 cache.h   |  5 -
 sha1_file.c   | 10 +-
 4 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/Documentation/technical/api-object-access.txt 
b/Documentation/technical/api-object-access.txt
index 03bb0e950d..a1162e5bcd 100644
--- a/Documentation/technical/api-object-access.txt
+++ b/Documentation/technical/api-object-access.txt
@@ -7,7 +7,7 @@ Talk about  and  family, things like
 * read_object_with_reference()
 * has_sha1_file()
 * write_sha1_file()
-* pretend_sha1_file()
+* pretend_object_file()
 * lookup_{object,commit,tag,blob,tree}
 * parse_{object,commit,tag,blob,tree}
 * Use of object flags
diff --git a/blame.c b/blame.c
index 2893f3c103..1fc22b304b 100644
--- a/blame.c
+++ b/blame.c
@@ -232,7 +232,7 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,
convert_to_git(_index, path, buf.buf, buf.len, , 0);
origin->file.ptr = buf.buf;
origin->file.size = buf.len;
-   pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_oid.hash);
+   pretend_object_file(buf.buf, buf.len, OBJ_BLOB, >blob_oid);
 
/*
 * Read the current index, replace the path entry with
diff --git a/cache.h b/cache.h
index d8b975a571..e4e03ac51d 100644
--- a/cache.h
+++ b/cache.h
@@ -1241,7 +1241,10 @@ extern int sha1_object_info(const unsigned char *, 
unsigned long *);
 extern int hash_sha1_file(const void *buf, unsigned long len, const char 
*type, unsigned char *sha1);
 extern int write_sha1_file(const void *buf, unsigned long len, const char 
*type, unsigned char *return_sha1);
 extern int hash_sha1_file_literally(const void *buf, unsigned long len, const 
char *type, struct object_id *oid, unsigned flags);
-extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned 
char *);
+
+extern int pretend_object_file(void *, unsigned long, enum object_type,
+  struct object_id *oid);
+
 extern int force_object_loose(const unsigned char *sha1, time_t mtime);
 extern int git_open_cloexec(const char *name, int flags);
 #define git_open(name) git_open_cloexec(name, O_RDONLY)
diff --git a/sha1_file.c b/sha1_file.c
index 3da70ac650..830b93b428 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1312,13 +1312,13 @@ static void *read_object(const unsigned char *sha1, 
enum object_type *type,
return content;
 }
 
-int pretend_sha1_file(void *buf, unsigned long len, enum object_type type,
- unsigned char *sha1)
+int pretend_object_file(void *buf, unsigned long len, enum object_type type,
+   struct object_id *oid)
 {
struct cached_object *co;
 
-   hash_sha1_file(buf, len, typename(type), sha1);
-   if (has_sha1_file(sha1) || find_cached_object(sha1))
+   hash_sha1_file(buf, len, typename(type), oid->hash);
+   if (has_sha1_file(oid->hash) || find_cached_object(oid->hash))
return 0;
ALLOC_GROW(cached_objects, cached_object_nr + 1, cached_object_alloc);
co = _objects[cached_object_nr++];
@@ -1326,7 +1326,7 @@ int pretend_sha1_file(void *buf, unsigned long len, enum 
object_type type,
co->type = type;
co->buf = xmalloc(len);
memcpy(co->buf, buf, len);
-   hashcpy(co->sha1, sha1);
+   hashcpy(co->sha1, oid->hash);
return 0;
 }
 
-- 
2.14.3



[PATCH v3 05/14] sha1_file: convert hash_sha1_file to object_id

2018-01-24 Thread Patryk Obara
Convert the declaration and definition of hash_sha1_file to use
struct object_id and adjust all function calls.

Rename this function to hash_object_file.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 apply.c  |  4 ++--
 builtin/index-pack.c |  5 ++---
 builtin/replace.c|  2 +-
 builtin/unpack-objects.c |  2 +-
 cache-tree.c | 11 +--
 cache.h  |  5 -
 convert.c|  6 +++---
 diffcore-rename.c|  4 ++--
 dir.c|  4 ++--
 log-tree.c   |  2 +-
 sha1_file.c  | 26 +-
 11 files changed, 36 insertions(+), 35 deletions(-)

diff --git a/apply.c b/apply.c
index 321a9fa68d..57ab8a8a29 100644
--- a/apply.c
+++ b/apply.c
@@ -3154,7 +3154,7 @@ static int apply_binary(struct apply_state *state,
 * See if the old one matches what the patch
 * applies to.
 */
-   hash_sha1_file(img->buf, img->len, blob_type, oid.hash);
+   hash_object_file(img->buf, img->len, blob_type, );
if (strcmp(oid_to_hex(), patch->old_sha1_prefix))
return error(_("the patch applies to '%s' (%s), "
   "which does not match the "
@@ -3199,7 +3199,7 @@ static int apply_binary(struct apply_state *state,
 name);
 
/* verify that the result matches */
-   hash_sha1_file(img->buf, img->len, blob_type, oid.hash);
+   hash_object_file(img->buf, img->len, blob_type, );
if (strcmp(oid_to_hex(), patch->new_sha1_prefix))
return error(_("binary patch to '%s' creates incorrect 
result (expecting %s, got %s)"),
name, patch->new_sha1_prefix, oid_to_hex());
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 4c51aec81f..7f5a95e6ff 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -958,9 +958,8 @@ static void resolve_delta(struct object_entry *delta_obj,
free(delta_data);
if (!result->data)
bad_object(delta_obj->idx.offset, _("failed to apply delta"));
-   hash_sha1_file(result->data, result->size,
-  typename(delta_obj->real_type),
-  delta_obj->idx.oid.hash);
+   hash_object_file(result->data, result->size,
+typename(delta_obj->real_type), _obj->idx.oid);
sha1_object(result->data, NULL, result->size, delta_obj->real_type,
_obj->idx.oid);
counter_lock();
diff --git a/builtin/replace.c b/builtin/replace.c
index 10078ae371..814bf6bfde 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -355,7 +355,7 @@ static void check_one_mergetag(struct commit *commit,
struct tag *tag;
int i;
 
-   hash_sha1_file(extra->value, extra->len, typename(OBJ_TAG), 
tag_oid.hash);
+   hash_object_file(extra->value, extra->len, typename(OBJ_TAG), _oid);
tag = lookup_tag(_oid);
if (!tag)
die(_("bad mergetag in commit '%s'"), ref);
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 62ea264c46..85a40d1af7 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -258,7 +258,7 @@ static void write_object(unsigned nr, enum object_type type,
} else {
struct object *obj;
int eaten;
-   hash_sha1_file(buf, size, typename(type), 
obj_list[nr].oid.hash);
+   hash_object_file(buf, size, typename(type), _list[nr].oid);
added_object(nr, type, buf, size);
obj = parse_object_buffer(_list[nr].oid, type, size, buf,
  );
diff --git a/cache-tree.c b/cache-tree.c
index e03e72c34a..6574eeb80d 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -400,15 +400,14 @@ static int update_one(struct cache_tree *it,
}
 
if (repair) {
-   unsigned char sha1[20];
-   hash_sha1_file(buffer.buf, buffer.len, tree_type, sha1);
-   if (has_sha1_file(sha1))
-   hashcpy(it->oid.hash, sha1);
+   struct object_id oid;
+   hash_object_file(buffer.buf, buffer.len, tree_type, );
+   if (has_sha1_file(oid.hash))
+   oidcpy(>oid, );
else
to_invalidate = 1;
} else if (dryrun)
-   hash_sha1_file(buffer.buf, buffer.len, tree_type,
-  it->oid.hash);
+   hash_object_file(buffer.buf, buffer.len, tree_type, >oid);
else if (write_sha1_file(buffer.buf, buffer.len, tree_type, 
it->oid.hash)) {
s

[PATCH v3 01/14] http-push: improve error log

2018-01-24 Thread Patryk Obara
When git push fails due to server-side WebDAV error, it's not easy to
point to the main culprit.  Additional information about exact cURL
error and HTTP server response is helpful for debugging purpose.

New error log helped me pinpoint failing test t5540-http-push-webdav
to a missing Apache dependency in Fedora 27:
https://bugzilla.redhat.com/show_bug.cgi?id=1491151

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 http-push.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/http-push.c b/http-push.c
index 14435ab65d..0913f8ab86 100644
--- a/http-push.c
+++ b/http-push.c
@@ -915,6 +915,10 @@ static struct remote_lock *lock_remote(const char *path, 
long timeout)
lock->timeout = -1;
}
XML_ParserFree(parser);
+   } else {
+   fprintf(stderr,
+   "error: curl result=%d, HTTP code=%ld\n",
+   results.curl_result, results.http_code);
}
} else {
fprintf(stderr, "Unable to start LOCK request\n");
-- 
2.14.3



[PATCH v3 04/14] dir: convert struct sha1_stat to use object_id

2018-01-24 Thread Patryk Obara
Convert the declaration of struct sha1_stat. Adjust all usages of this
struct and replace hash{clr,cmp,cpy} with oid{clr,cmp,cpy} wherever
possible.  Rename it to struct oid_stat.

Remove macro EMPTY_BLOB_SHA1_BIN, as it's no longer used.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 cache.h  |  2 --
 dir.c| 56 +---
 dir.h| 12 
 t/helper/test-dump-untracked-cache.c |  4 +--
 4 files changed, 34 insertions(+), 40 deletions(-)

diff --git a/cache.h b/cache.h
index e4e03ac51d..ed72933ba7 100644
--- a/cache.h
+++ b/cache.h
@@ -1047,8 +1047,6 @@ extern const struct object_id empty_tree_oid;
"\xe6\x9d\xe2\x9b\xb2\xd1\xd6\x43\x4b\x8b" \
"\x29\xae\x77\x5a\xd8\xc2\xe4\x8c\x53\x91"
 extern const struct object_id empty_blob_oid;
-#define EMPTY_BLOB_SHA1_BIN (empty_blob_oid.hash)
-
 
 static inline int is_empty_blob_sha1(const unsigned char *sha1)
 {
diff --git a/dir.c b/dir.c
index 7c4b45e30e..ef977b8657 100644
--- a/dir.c
+++ b/dir.c
@@ -233,10 +233,8 @@ int within_depth(const char *name, int namelen,
  *
  * Optionally updates the given sha1_stat with the given OID (when valid).
  */
-static int do_read_blob(const struct object_id *oid,
-   struct sha1_stat *sha1_stat,
-   size_t *size_out,
-   char **data_out)
+static int do_read_blob(const struct object_id *oid, struct oid_stat 
*sha1_stat,
+   size_t *size_out, char **data_out)
 {
enum object_type type;
unsigned long sz;
@@ -253,7 +251,7 @@ static int do_read_blob(const struct object_id *oid,
 
if (sha1_stat) {
memset(_stat->stat, 0, sizeof(sha1_stat->stat));
-   hashcpy(sha1_stat->sha1, oid->hash);
+   oidcpy(_stat->oid, oid);
}
 
if (sz == 0) {
@@ -654,9 +652,8 @@ void add_exclude(const char *string, const char *base,
 
 static int read_skip_worktree_file_from_index(const struct index_state *istate,
  const char *path,
- size_t *size_out,
- char **data_out,
- struct sha1_stat *sha1_stat)
+ size_t *size_out, char **data_out,
+ struct oid_stat *sha1_stat)
 {
int pos, len;
 
@@ -795,9 +792,8 @@ static int add_excludes_from_buffer(char *buf, size_t size,
  * ss_valid is non-zero, "ss" must contain good value as input.
  */
 static int add_excludes(const char *fname, const char *base, int baselen,
-   struct exclude_list *el,
-   struct index_state *istate,
-   struct sha1_stat *sha1_stat)
+   struct exclude_list *el, struct index_state *istate,
+   struct oid_stat *sha1_stat)
 {
struct stat st;
int r;
@@ -823,7 +819,7 @@ static int add_excludes(const char *fname, const char 
*base, int baselen,
if (size == 0) {
if (sha1_stat) {
fill_stat_data(_stat->stat, );
-   hashcpy(sha1_stat->sha1, EMPTY_BLOB_SHA1_BIN);
+   oidcpy(_stat->oid, _blob_oid);
sha1_stat->valid = 1;
}
close(fd);
@@ -847,10 +843,11 @@ static int add_excludes(const char *fname, const char 
*base, int baselen,
 !ce_stage(istate->cache[pos]) &&
 ce_uptodate(istate->cache[pos]) &&
 !would_convert_to_git(istate, fname))
-   hashcpy(sha1_stat->sha1,
-   istate->cache[pos]->oid.hash);
+   oidcpy(_stat->oid,
+  >cache[pos]->oid);
else
-   hash_sha1_file(buf, size, "blob", 
sha1_stat->sha1);
+   hash_sha1_file(buf, size, "blob",
+  sha1_stat->oid.hash);
fill_stat_data(_stat->stat, );
sha1_stat->valid = 1;
}
@@ -930,7 +927,7 @@ struct exclude_list *add_exclude_list(struct dir_struct 
*dir,
  * Used to set up core.excludesfile and .git/info/exclude lists.
  */
 static void add_excludes_from_file_1(struct dir_struct *dir, const char *fname,
-struct sha1_stat *sha1_stat)
+struct oid_stat *sha1_stat

[PATCH v3 07/14] match-trees: convert splice_tree to object_id

2018-01-24 Thread Patryk Obara
Convert the definition of static recursive splice_tree function to use
struct object_id and adjust single caller.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 match-trees.c | 46 ++
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/match-trees.c b/match-trees.c
index 396b7338df..afb771c4f5 100644
--- a/match-trees.c
+++ b/match-trees.c
@@ -158,22 +158,20 @@ static void match_trees(const struct object_id *hash1,
 }
 
 /*
- * A tree "hash1" has a subdirectory at "prefix".  Come up with a
- * tree object by replacing it with another tree "hash2".
+ * A tree "oid1" has a subdirectory at "prefix".  Come up with a tree object by
+ * replacing it with another tree "oid2".
  */
-static int splice_tree(const unsigned char *hash1,
-  const char *prefix,
-  const unsigned char *hash2,
-  unsigned char *result)
+static int splice_tree(const struct object_id *oid1, const char *prefix,
+  const struct object_id *oid2, struct object_id *result)
 {
char *subpath;
int toplen;
char *buf;
unsigned long sz;
struct tree_desc desc;
-   unsigned char *rewrite_here;
-   const unsigned char *rewrite_with;
-   unsigned char subtree[20];
+   struct object_id *rewrite_here;
+   const struct object_id *rewrite_with;
+   struct object_id subtree;
enum object_type type;
int status;
 
@@ -182,9 +180,9 @@ static int splice_tree(const unsigned char *hash1,
if (*subpath)
subpath++;
 
-   buf = read_sha1_file(hash1, , );
+   buf = read_sha1_file(oid1->hash, , );
if (!buf)
-   die("cannot read tree %s", sha1_to_hex(hash1));
+   die("cannot read tree %s", oid_to_hex(oid1));
init_tree_desc(, buf, sz);
 
rewrite_here = NULL;
@@ -197,26 +195,26 @@ static int splice_tree(const unsigned char *hash1,
if (strlen(name) == toplen &&
!memcmp(name, prefix, toplen)) {
if (!S_ISDIR(mode))
-   die("entry %s in tree %s is not a tree",
-   name, sha1_to_hex(hash1));
-   rewrite_here = (unsigned char *) oid->hash;
+   die("entry %s in tree %s is not a tree", name,
+   oid_to_hex(oid1));
+   rewrite_here = (struct object_id *)oid;
break;
}
update_tree_entry();
}
if (!rewrite_here)
-   die("entry %.*s not found in tree %s",
-   toplen, prefix, sha1_to_hex(hash1));
+   die("entry %.*s not found in tree %s", toplen, prefix,
+   oid_to_hex(oid1));
if (*subpath) {
-   status = splice_tree(rewrite_here, subpath, hash2, subtree);
+   status = splice_tree(rewrite_here, subpath, oid2, );
if (status)
return status;
-   rewrite_with = subtree;
+   rewrite_with = 
+   } else {
+   rewrite_with = oid2;
}
-   else
-   rewrite_with = hash2;
-   hashcpy(rewrite_here, rewrite_with);
-   status = write_sha1_file(buf, sz, tree_type, result);
+   oidcpy(rewrite_here, rewrite_with);
+   status = write_sha1_file(buf, sz, tree_type, result->hash);
free(buf);
return status;
 }
@@ -280,7 +278,7 @@ void shift_tree(const struct object_id *hash1,
if (!*add_prefix)
return;
 
-   splice_tree(hash1->hash, add_prefix, hash2->hash, shifted->hash);
+   splice_tree(hash1, add_prefix, hash2, shifted);
 }
 
 /*
@@ -334,7 +332,7 @@ void shift_tree_by(const struct object_id *hash1,
 * shift tree2 down by adding shift_prefix above it
 * to match tree1.
 */
-   splice_tree(hash1->hash, shift_prefix, hash2->hash, 
shifted->hash);
+   splice_tree(hash1, shift_prefix, hash2, shifted);
else
/*
 * shift tree2 up by removing shift_prefix from it
-- 
2.14.3



[PATCH v3 10/14] notes: convert write_notes_tree to object_id

2018-01-24 Thread Patryk Obara
Convert the definition and declaration of write_notes_tree to
struct object_id and adjust usage of this function.

Additionally, improve style of small part of this function, as old
formatting made it hard to understand at glance what this part of
code is doing.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 notes-cache.c |  2 +-
 notes-utils.c |  2 +-
 notes.c   | 16 +---
 notes.h   |  4 ++--
 4 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/notes-cache.c b/notes-cache.c
index d2f87147cc..010ad236cb 100644
--- a/notes-cache.c
+++ b/notes-cache.c
@@ -54,7 +54,7 @@ int notes_cache_write(struct notes_cache *c)
if (!c->tree.dirty)
return 0;
 
-   if (write_notes_tree(>tree, tree_oid.hash))
+   if (write_notes_tree(>tree, _oid))
return -1;
if (commit_tree(c->validity, strlen(c->validity), _oid, NULL,
_oid, NULL, NULL) < 0)
diff --git a/notes-utils.c b/notes-utils.c
index 058c642dac..02407fe2a7 100644
--- a/notes-utils.c
+++ b/notes-utils.c
@@ -12,7 +12,7 @@ void create_notes_commit(struct notes_tree *t, struct 
commit_list *parents,
 
assert(t->initialized);
 
-   if (write_notes_tree(t, tree_oid.hash))
+   if (write_notes_tree(t, _oid))
die("Failed to write notes tree to database");
 
if (!parents) {
diff --git a/notes.c b/notes.c
index 3f4f94507a..09ef1ce33a 100644
--- a/notes.c
+++ b/notes.c
@@ -1123,11 +1123,12 @@ int for_each_note(struct notes_tree *t, int flags, 
each_note_fn fn,
return for_each_note_helper(t, t->root, 0, 0, flags, fn, cb_data);
 }
 
-int write_notes_tree(struct notes_tree *t, unsigned char *result)
+int write_notes_tree(struct notes_tree *t, struct object_id *result)
 {
struct tree_write_stack root;
struct write_each_note_data cb_data;
int ret;
+   int flags;
 
if (!t)
t = _notes_tree;
@@ -1141,12 +1142,13 @@ int write_notes_tree(struct notes_tree *t, unsigned 
char *result)
cb_data.next_non_note = t->first_non_note;
 
/* Write tree objects representing current notes tree */
-   ret = for_each_note(t, FOR_EACH_NOTE_DONT_UNPACK_SUBTREES |
-   FOR_EACH_NOTE_YIELD_SUBTREES,
-   write_each_note, _data) ||
-   write_each_non_note_until(NULL, _data) ||
-   tree_write_stack_finish_subtree() ||
-   write_sha1_file(root.buf.buf, root.buf.len, tree_type, result);
+   flags = FOR_EACH_NOTE_DONT_UNPACK_SUBTREES |
+   FOR_EACH_NOTE_YIELD_SUBTREES;
+   ret = for_each_note(t, flags, write_each_note, _data) ||
+ write_each_non_note_until(NULL, _data) ||
+ tree_write_stack_finish_subtree() ||
+ write_sha1_file(root.buf.buf, root.buf.len, tree_type,
+ result->hash);
strbuf_release();
return ret;
 }
diff --git a/notes.h b/notes.h
index 88da38b5f4..0433f45db5 100644
--- a/notes.h
+++ b/notes.h
@@ -217,7 +217,7 @@ int for_each_note(struct notes_tree *t, int flags, 
each_note_fn fn,
  * Write the given notes_tree structure to the object database
  *
  * Creates a new tree object encapsulating the current state of the given
- * notes_tree, and stores its SHA1 into the 'result' argument.
+ * notes_tree, and stores its object id into the 'result' argument.
  *
  * Returns zero on success, non-zero on failure.
  *
@@ -225,7 +225,7 @@ int for_each_note(struct notes_tree *t, int flags, 
each_note_fn fn,
  * this function has returned zero. Please also remember to create a
  * corresponding commit object, and update the appropriate notes ref.
  */
-int write_notes_tree(struct notes_tree *t, unsigned char *result);
+int write_notes_tree(struct notes_tree *t, struct object_id *result);
 
 /* Flags controlling the operation of prune */
 #define NOTES_PRUNE_VERBOSE 1
-- 
2.14.3



[PATCH v3 00/14] Some fixes and bunch of object_id conversions

2018-01-24 Thread Patryk Obara
Compared to v2:

* rebased to latest master

* patch 1 and 2
I kept them in, but if Junio prefers them separately then I'll send
them as separate patches.

* patch 5
- strbuf_add(buf, sha1_to_hex(oid.hash), GIT_SHA1_HEXSZ);
+ strbuf_addstr(buf, oid_to_hex());

* patch 7
On suggestion from Duy Nguyễn, I renamed parameter names:
'hash1' and 'hash2' with 'oid1' and 'oid2' to avoid e.g. 'hash1->hash'.

I decided *not to* implement suggested replace_tree_entry_hash
function (when implemented as in mail, t6026-merge-subtree fails).
I think this overwriting of object_id needs to be addressed, but
probably at the time of adjusting tree object format for longer hash.

Patryk Obara (14):
  http-push: improve error log
  clang-format: adjust penalty for return type line break
  sha1_file: convert pretend_sha1_file to object_id
  dir: convert struct sha1_stat to use object_id
  sha1_file: convert hash_sha1_file to object_id
  cache: clear whole hash buffer with oidclr
  match-trees: convert splice_tree to object_id
  commit: convert commit_tree* to object_id
  notes: convert combine_notes_* to object_id
  notes: convert write_notes_tree to object_id
  sha1_file: convert write_sha1_file to object_id
  sha1_file: convert force_object_loose to object_id
  sha1_file: convert write_loose_object to object_id
  sha1_file: rename hash_sha1_file_literally

 .clang-format |   2 +-
 Documentation/technical/api-object-access.txt |   2 +-
 apply.c   |  12 ++--
 blame.c   |   2 +-
 builtin/am.c  |   4 +-
 builtin/checkout.c|   3 +-
 builtin/commit-tree.c |   4 +-
 builtin/commit.c  |   5 +-
 builtin/hash-object.c |   3 +-
 builtin/index-pack.c  |   5 +-
 builtin/merge.c   |   8 +--
 builtin/mktag.c   |   6 +-
 builtin/mktree.c  |  10 +--
 builtin/notes.c   |   8 +--
 builtin/pack-objects.c|   2 +-
 builtin/receive-pack.c|  11 +--
 builtin/replace.c |   4 +-
 builtin/tag.c |   2 +-
 builtin/unpack-objects.c  |  11 +--
 cache-tree.c  |  16 ++---
 cache.h   |  25 ---
 commit.c  |  15 ++--
 commit.h  |  11 +--
 convert.c |   6 +-
 diffcore-rename.c |   4 +-
 dir.c |  56 +++
 dir.h |  12 ++--
 http-push.c   |   4 ++
 log-tree.c|   2 +-
 match-trees.c |  46 ++--
 merge-recursive.c |   5 +-
 notes-cache.c |   8 +--
 notes-merge.c |   9 ++-
 notes-utils.c |   9 +--
 notes-utils.h |   3 +-
 notes.c   |  63 
 notes.h   |  29 
 read-cache.c  |   6 +-
 sha1_file.c   | 100 ++
 t/helper/test-dump-untracked-cache.c  |   4 +-
 40 files changed, 281 insertions(+), 256 deletions(-)


base-commit: 5be1f00a9a701532232f57958efab4be8c959a29
-- 
2.14.3



[PATCH v3 02/14] clang-format: adjust penalty for return type line break

2018-01-24 Thread Patryk Obara
The penalty of 5 makes clang-format very eager to put even short type
declarations (e.g. "extern int") into a separate line, even when
breaking parameters list is sufficient.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 .clang-format | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.clang-format b/.clang-format
index 611ab4750b..12a89f95f9 100644
--- a/.clang-format
+++ b/.clang-format
@@ -163,7 +163,7 @@ PenaltyBreakComment: 10
 PenaltyBreakFirstLessLess: 0
 PenaltyBreakString: 10
 PenaltyExcessCharacter: 100
-PenaltyReturnTypeOnItsOwnLine: 5
+PenaltyReturnTypeOnItsOwnLine: 60
 
 # Don't sort #include's
 SortIncludes: false
-- 
2.14.3



[PATCH v3 11/14] sha1_file: convert write_sha1_file to object_id

2018-01-24 Thread Patryk Obara
Convert the definition and declaration of write_sha1_file to
struct object_id and adjust usage of this function.

This commit also converts static function write_sha1_file_prepare, as it
is closely related.

Rename these functions to write_object_file and
write_object_file_prepare respectively.

Replace sha1_to_hex, hashcpy and hashclr with their oid equivalents
wherever possible.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 apply.c  |  8 
 builtin/checkout.c   |  3 +--
 builtin/mktag.c  |  6 +++---
 builtin/mktree.c | 10 +-
 builtin/notes.c  |  8 
 builtin/receive-pack.c   | 11 ++-
 builtin/replace.c|  2 +-
 builtin/tag.c|  2 +-
 builtin/unpack-objects.c |  9 ++---
 cache-tree.c |  5 +++--
 cache.h  |  4 +++-
 commit.c |  2 +-
 match-trees.c|  2 +-
 merge-recursive.c|  5 +++--
 notes-cache.c|  2 +-
 notes.c  |  9 -
 read-cache.c |  6 +++---
 sha1_file.c  | 29 +++--
 18 files changed, 65 insertions(+), 58 deletions(-)

diff --git a/apply.c b/apply.c
index 57ab8a8a29..4cd4504008 100644
--- a/apply.c
+++ b/apply.c
@@ -3554,7 +3554,7 @@ static int try_threeway(struct apply_state *state,
 
/* Preimage the patch was prepared for */
if (patch->is_new)
-   write_sha1_file("", 0, blob_type, pre_oid.hash);
+   write_object_file("", 0, blob_type, _oid);
else if (get_oid(patch->old_sha1_prefix, _oid) ||
 read_blob_object(, _oid, patch->old_mode))
return error(_("repository lacks the necessary blob to fall 
back on 3-way merge."));
@@ -3570,7 +3570,7 @@ static int try_threeway(struct apply_state *state,
return -1;
}
/* post_oid is theirs */
-   write_sha1_file(tmp_image.buf, tmp_image.len, blob_type, post_oid.hash);
+   write_object_file(tmp_image.buf, tmp_image.len, blob_type, _oid);
clear_image(_image);
 
/* our_oid is ours */
@@ -3583,7 +3583,7 @@ static int try_threeway(struct apply_state *state,
return error(_("cannot read the current contents of 
'%s'"),
 patch->old_name);
}
-   write_sha1_file(tmp_image.buf, tmp_image.len, blob_type, our_oid.hash);
+   write_object_file(tmp_image.buf, tmp_image.len, blob_type, _oid);
clear_image(_image);
 
/* in-core three-way merge between post and our using pre as base */
@@ -4291,7 +4291,7 @@ static int add_index_file(struct apply_state *state,
}
fill_stat_cache_info(ce, );
}
-   if (write_sha1_file(buf, size, blob_type, ce->oid.hash) < 0) {
+   if (write_object_file(buf, size, blob_type, >oid) < 0) {
free(ce);
return error(_("unable to create backing store "
   "for newly created file %s"), path);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index c54c78df54..191b96c49c 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -227,8 +227,7 @@ static int checkout_merged(int pos, const struct checkout 
*state)
 * (it also writes the merge result to the object database even
 * when it may contain conflicts).
 */
-   if (write_sha1_file(result_buf.ptr, result_buf.size,
-   blob_type, oid.hash))
+   if (write_object_file(result_buf.ptr, result_buf.size, blob_type, ))
die(_("Unable to add merge result for '%s'"), path);
free(result_buf.ptr);
ce = make_cache_entry(mode, oid.hash, path, 2, 0);
diff --git a/builtin/mktag.c b/builtin/mktag.c
index 031b750f06..beb552847b 100644
--- a/builtin/mktag.c
+++ b/builtin/mktag.c
@@ -151,7 +151,7 @@ static int verify_tag(char *buffer, unsigned long size)
 int cmd_mktag(int argc, const char **argv, const char *prefix)
 {
struct strbuf buf = STRBUF_INIT;
-   unsigned char result_sha1[20];
+   struct object_id result;
 
if (argc != 1)
usage("git mktag");
@@ -165,10 +165,10 @@ int cmd_mktag(int argc, const char **argv, const char 
*prefix)
if (verify_tag(buf.buf, buf.len) < 0)
die("invalid tag signature file");
 
-   if (write_sha1_file(buf.buf, buf.len, tag_type, result_sha1) < 0)
+   if (write_object_file(buf.buf, buf.len, tag_type, ) < 0)
die("unable to write tag file");
 
strbuf_release();
-   printf("%s\n", sha1_to_hex(result_sha1));
+   printf("%s\n", oid_to_hex());
return 0;
 }
diff --git a/builtin/mktree.c b/builtin/mkt

[PATCH v3 09/14] notes: convert combine_notes_* to object_id

2018-01-24 Thread Patryk Obara
Convert the definition and declarations of combine_notes_* functions
to struct object_id and adjust usage of these functions.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 notes.c | 46 +++---
 notes.h | 25 +++--
 2 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/notes.c b/notes.c
index c7f21fae44..3f4f94507a 100644
--- a/notes.c
+++ b/notes.c
@@ -270,8 +270,8 @@ static int note_tree_insert(struct notes_tree *t, struct 
int_node *tree,
if (!oidcmp(>val_oid, >val_oid))
return 0;
 
-   ret = combine_notes(l->val_oid.hash,
-   entry->val_oid.hash);
+   ret = combine_notes(>val_oid,
+   >val_oid);
if (!ret && is_null_oid(>val_oid))
note_tree_remove(t, tree, n, entry);
free(entry);
@@ -786,8 +786,8 @@ static int prune_notes_helper(const struct object_id 
*object_oid,
return 0;
 }
 
-int combine_notes_concatenate(unsigned char *cur_sha1,
-   const unsigned char *new_sha1)
+int combine_notes_concatenate(struct object_id *cur_oid,
+ const struct object_id *new_oid)
 {
char *cur_msg = NULL, *new_msg = NULL, *buf;
unsigned long cur_len, new_len, buf_len;
@@ -795,18 +795,18 @@ int combine_notes_concatenate(unsigned char *cur_sha1,
int ret;
 
/* read in both note blob objects */
-   if (!is_null_sha1(new_sha1))
-   new_msg = read_sha1_file(new_sha1, _type, _len);
+   if (!is_null_oid(new_oid))
+   new_msg = read_sha1_file(new_oid->hash, _type, _len);
if (!new_msg || !new_len || new_type != OBJ_BLOB) {
free(new_msg);
return 0;
}
-   if (!is_null_sha1(cur_sha1))
-   cur_msg = read_sha1_file(cur_sha1, _type, _len);
+   if (!is_null_oid(cur_oid))
+   cur_msg = read_sha1_file(cur_oid->hash, _type, _len);
if (!cur_msg || !cur_len || cur_type != OBJ_BLOB) {
free(cur_msg);
free(new_msg);
-   hashcpy(cur_sha1, new_sha1);
+   oidcpy(cur_oid, new_oid);
return 0;
}
 
@@ -825,20 +825,20 @@ int combine_notes_concatenate(unsigned char *cur_sha1,
free(new_msg);
 
/* create a new blob object from buf */
-   ret = write_sha1_file(buf, buf_len, blob_type, cur_sha1);
+   ret = write_sha1_file(buf, buf_len, blob_type, cur_oid->hash);
free(buf);
return ret;
 }
 
-int combine_notes_overwrite(unsigned char *cur_sha1,
-   const unsigned char *new_sha1)
+int combine_notes_overwrite(struct object_id *cur_oid,
+   const struct object_id *new_oid)
 {
-   hashcpy(cur_sha1, new_sha1);
+   oidcpy(cur_oid, new_oid);
return 0;
 }
 
-int combine_notes_ignore(unsigned char *cur_sha1,
-   const unsigned char *new_sha1)
+int combine_notes_ignore(struct object_id *cur_oid,
+const struct object_id *new_oid)
 {
return 0;
 }
@@ -848,17 +848,17 @@ int combine_notes_ignore(unsigned char *cur_sha1,
  * newlines removed.
  */
 static int string_list_add_note_lines(struct string_list *list,
- const unsigned char *sha1)
+ const struct object_id *oid)
 {
char *data;
unsigned long len;
enum object_type t;
 
-   if (is_null_sha1(sha1))
+   if (is_null_oid(oid))
return 0;
 
/* read_sha1_file NUL-terminates */
-   data = read_sha1_file(sha1, , );
+   data = read_sha1_file(oid->hash, , );
if (t != OBJ_BLOB || !data || !len) {
free(data);
return t != OBJ_BLOB || !data;
@@ -884,17 +884,17 @@ static int string_list_join_lines_helper(struct 
string_list_item *item,
return 0;
 }
 
-int combine_notes_cat_sort_uniq(unsigned char *cur_sha1,
-   const unsigned char *new_sha1)
+int combine_notes_cat_sort_uniq(struct object_id *cur_oid,
+   const struct object_id *new_oid)
 {
struct string_list sort_uniq_list = STRING_LIST_INIT_DUP;
struct strbuf buf = STRBUF_INIT;
int ret = 1;
 
/* read both note blob objects into unique_lines */
-   if (string_list_add_note_lines(_uniq_list, cur_sha1))
+   if (string_list_add_note_lines(_uniq_list, cur_oid))
goto out;
-   if (string_list_add_note_lines(_uniq_list, new_sha1))
+   if (string_list_add_note_lines(_uniq_list, new_oid))
goto out;
string_list_remove_empty_items(_uni

[PATCH v3 14/14] sha1_file: rename hash_sha1_file_literally

2018-01-24 Thread Patryk Obara
This function was already converted to use struct object_id earlier.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 builtin/hash-object.c | 3 ++-
 cache.h   | 4 +++-
 sha1_file.c   | 5 +++--
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index c532ff9320..526da5c185 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -24,7 +24,8 @@ static int hash_literally(struct object_id *oid, int fd, 
const char *type, unsig
if (strbuf_read(, fd, 4096) < 0)
ret = -1;
else
-   ret = hash_sha1_file_literally(buf.buf, buf.len, type, oid, 
flags);
+   ret = hash_object_file_literally(buf.buf, buf.len, type, oid,
+flags);
strbuf_release();
return ret;
 }
diff --git a/cache.h b/cache.h
index 0a8be9c87f..6ef4248931 100644
--- a/cache.h
+++ b/cache.h
@@ -1243,7 +1243,9 @@ extern int hash_object_file(const void *buf, unsigned 
long len,
 extern int write_object_file(const void *buf, unsigned long len,
 const char *type, struct object_id *oid);
 
-extern int hash_sha1_file_literally(const void *buf, unsigned long len, const 
char *type, struct object_id *oid, unsigned flags);
+extern int hash_object_file_literally(const void *buf, unsigned long len,
+ const char *type, struct object_id *oid,
+ unsigned flags);
 
 extern int pretend_object_file(void *, unsigned long, enum object_type,
   struct object_id *oid);
diff --git a/sha1_file.c b/sha1_file.c
index 59238f5bea..34c041e8cd 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1652,8 +1652,9 @@ int write_object_file(const void *buf, unsigned long len, 
const char *type,
return write_loose_object(oid, hdr, hdrlen, buf, len, 0);
 }
 
-int hash_sha1_file_literally(const void *buf, unsigned long len, const char 
*type,
-struct object_id *oid, unsigned flags)
+int hash_object_file_literally(const void *buf, unsigned long len,
+  const char *type, struct object_id *oid,
+  unsigned flags)
 {
char *header;
int hdrlen, status = 0;
-- 
2.14.3



[PATCH v3 12/14] sha1_file: convert force_object_loose to object_id

2018-01-24 Thread Patryk Obara
Convert the definition and declaration of force_object_loose to
struct object_id and adjust usage of this function.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 builtin/pack-objects.c |  2 +-
 cache.h|  3 ++-
 sha1_file.c| 10 +-
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 6b9cfc289d..f38197543d 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2768,7 +2768,7 @@ static void loosen_unused_packed_objects(struct rev_info 
*revs)
if (!packlist_find(_pack, oid.hash, NULL) &&
!has_sha1_pack_kept_or_nonlocal() &&
!loosened_object_can_be_discarded(, p->mtime))
-   if (force_object_loose(oid.hash, p->mtime))
+   if (force_object_loose(, p->mtime))
die("unable to force loose object");
}
}
diff --git a/cache.h b/cache.h
index d80141eb64..0a8be9c87f 100644
--- a/cache.h
+++ b/cache.h
@@ -1248,7 +1248,8 @@ extern int hash_sha1_file_literally(const void *buf, 
unsigned long len, const ch
 extern int pretend_object_file(void *, unsigned long, enum object_type,
   struct object_id *oid);
 
-extern int force_object_loose(const unsigned char *sha1, time_t mtime);
+extern int force_object_loose(const struct object_id *oid, time_t mtime);
+
 extern int git_open_cloexec(const char *name, int flags);
 #define git_open(name) git_open_cloexec(name, O_RDONLY)
 extern void *map_sha1_file(const unsigned char *sha1, unsigned long *size);
diff --git a/sha1_file.c b/sha1_file.c
index d1569b1b96..d9ee966d74 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1670,7 +1670,7 @@ int hash_sha1_file_literally(const void *buf, unsigned 
long len, const char *typ
return status;
 }
 
-int force_object_loose(const unsigned char *sha1, time_t mtime)
+int force_object_loose(const struct object_id *oid, time_t mtime)
 {
void *buf;
unsigned long len;
@@ -1679,13 +1679,13 @@ int force_object_loose(const unsigned char *sha1, 
time_t mtime)
int hdrlen;
int ret;
 
-   if (has_loose_object(sha1))
+   if (has_loose_object(oid->hash))
return 0;
-   buf = read_object(sha1, , );
+   buf = read_object(oid->hash, , );
if (!buf)
-   return error("cannot read sha1_file for %s", sha1_to_hex(sha1));
+   return error("cannot read sha1_file for %s", oid_to_hex(oid));
hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", typename(type), len) + 1;
-   ret = write_loose_object(sha1, hdr, hdrlen, buf, len, mtime);
+   ret = write_loose_object(oid->hash, hdr, hdrlen, buf, len, mtime);
free(buf);
 
return ret;
-- 
2.14.3



[PATCH v3 08/14] commit: convert commit_tree* to object_id

2018-01-24 Thread Patryk Obara
Convert the definitions and declarations of commit_tree and
commit_tree_extended to use struct object_id and adjust all usages of
these functions.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 builtin/am.c  |  4 ++--
 builtin/commit-tree.c |  4 ++--
 builtin/commit.c  |  5 +++--
 builtin/merge.c   |  8 
 commit.c  | 15 +++
 commit.h  | 11 ++-
 notes-cache.c |  4 ++--
 notes-merge.c |  9 -
 notes-utils.c |  7 ---
 notes-utils.h |  3 ++-
 10 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index acfe9d3c8c..6e6abb05cd 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1641,8 +1641,8 @@ static void do_commit(const struct am_state *state)
setenv("GIT_COMMITTER_DATE",
state->ignore_date ? "" : state->author_date, 1);
 
-   if (commit_tree(state->msg, state->msg_len, tree.hash, parents, 
commit.hash,
-   author, state->sign_commit))
+   if (commit_tree(state->msg, state->msg_len, , parents, ,
+   author, state->sign_commit))
die(_("failed to write commit object"));
 
reflog_msg = getenv("GIT_REFLOG_ACTION");
diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
index 2177251e24..e5bdf57b1e 100644
--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c
@@ -117,8 +117,8 @@ int cmd_commit_tree(int argc, const char **argv, const char 
*prefix)
die_errno("git commit-tree: failed to read");
}
 
-   if (commit_tree(buffer.buf, buffer.len, tree_oid.hash, parents,
-   commit_oid.hash, NULL, sign_commit)) {
+   if (commit_tree(buffer.buf, buffer.len, _oid, parents, _oid,
+   NULL, sign_commit)) {
strbuf_release();
return 1;
}
diff --git a/builtin/commit.c b/builtin/commit.c
index 4610e3d8e3..e5974a5999 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1794,8 +1794,9 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
append_merge_tag_headers(parents, );
}
 
-   if (commit_tree_extended(sb.buf, sb.len, active_cache_tree->oid.hash,
-parents, oid.hash, author_ident.buf, sign_commit, 
extra)) {
+   if (commit_tree_extended(sb.buf, sb.len, _cache_tree->oid,
+parents, , author_ident.buf, sign_commit,
+extra)) {
rollback_index_files();
die(_("failed to write commit object"));
}
diff --git a/builtin/merge.c b/builtin/merge.c
index 30264cfd7c..92ba99a1a5 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -820,8 +820,8 @@ static int merge_trivial(struct commit *head, struct 
commit_list *remoteheads)
pptr = commit_list_append(head, pptr);
pptr = commit_list_append(remoteheads->item, pptr);
prepare_to_commit(remoteheads);
-   if (commit_tree(merge_msg.buf, merge_msg.len, result_tree.hash, parents,
-   result_commit.hash, NULL, sign_commit))
+   if (commit_tree(merge_msg.buf, merge_msg.len, _tree, parents,
+   _commit, NULL, sign_commit))
die(_("failed to write commit object"));
finish(head, remoteheads, _commit, "In-index merge");
drop_save();
@@ -845,8 +845,8 @@ static int finish_automerge(struct commit *head,
commit_list_insert(head, );
strbuf_addch(_msg, '\n');
prepare_to_commit(remoteheads);
-   if (commit_tree(merge_msg.buf, merge_msg.len, result_tree->hash, 
parents,
-   result_commit.hash, NULL, sign_commit))
+   if (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents,
+   _commit, NULL, sign_commit))
die(_("failed to write commit object"));
strbuf_addf(, "Merge made by the '%s' strategy.", wt_strategy);
finish(head, remoteheads, _commit, buf.buf);
diff --git a/commit.c b/commit.c
index ff51c9f34a..643f3daec3 100644
--- a/commit.c
+++ b/commit.c
@@ -1380,9 +1380,8 @@ void free_commit_extra_headers(struct commit_extra_header 
*extra)
}
 }
 
-int commit_tree(const char *msg, size_t msg_len,
-   const unsigned char *tree,
-   struct commit_list *parents, unsigned char *ret,
+int commit_tree(const char *msg, size_t msg_len, const struct object_id *tree,
+   struct commit_list *parents, struct object_id *ret,
const char *author, const char *sign_commit)
 {
struct commit_extra_header *extra = NULL, **tail = 
@@ -1511,8 +1510,8 @@ N_("Warning: commit message did not conform to UTF-8.\n"
"variable i18

[PATCH v3 13/14] sha1_file: convert write_loose_object to object_id

2018-01-24 Thread Patryk Obara
Convert the definition and declaration of statis write_loose_object
function to struct object_id.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 sha1_file.c | 28 
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index d9ee966d74..59238f5bea 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1548,16 +1548,17 @@ static int create_tmpfile(struct strbuf *tmp, const 
char *filename)
return fd;
 }
 
-static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen,
- const void *buf, unsigned long len, time_t mtime)
+static int write_loose_object(const struct object_id *oid, char *hdr,
+ int hdrlen, const void *buf, unsigned long len,
+ time_t mtime)
 {
int fd, ret;
unsigned char compressed[4096];
git_zstream stream;
git_SHA_CTX c;
-   unsigned char parano_sha1[20];
+   struct object_id parano_oid;
static struct strbuf tmp_file = STRBUF_INIT;
-   const char *filename = sha1_file_name(sha1);
+   const char *filename = sha1_file_name(oid->hash);
 
fd = create_tmpfile(_file, filename);
if (fd < 0) {
@@ -1594,13 +1595,16 @@ static int write_loose_object(const unsigned char 
*sha1, char *hdr, int hdrlen,
} while (ret == Z_OK);
 
if (ret != Z_STREAM_END)
-   die("unable to deflate new object %s (%d)", sha1_to_hex(sha1), 
ret);
+   die("unable to deflate new object %s (%d)", oid_to_hex(oid),
+   ret);
ret = git_deflate_end_gently();
if (ret != Z_OK)
-   die("deflateEnd on object %s failed (%d)", sha1_to_hex(sha1), 
ret);
-   git_SHA1_Final(parano_sha1, );
-   if (hashcmp(sha1, parano_sha1) != 0)
-   die("confused by unstable object source data for %s", 
sha1_to_hex(sha1));
+   die("deflateEnd on object %s failed (%d)", oid_to_hex(oid),
+   ret);
+   git_SHA1_Final(parano_oid.hash, );
+   if (oidcmp(oid, _oid) != 0)
+   die("confused by unstable object source data for %s",
+   oid_to_hex(oid));
 
close_sha1_file(fd);
 
@@ -1645,7 +1649,7 @@ int write_object_file(const void *buf, unsigned long len, 
const char *type,
write_object_file_prepare(buf, len, type, oid, hdr, );
if (freshen_packed_object(oid->hash) || freshen_loose_object(oid->hash))
return 0;
-   return write_loose_object(oid->hash, hdr, hdrlen, buf, len, 0);
+   return write_loose_object(oid, hdr, hdrlen, buf, len, 0);
 }
 
 int hash_sha1_file_literally(const void *buf, unsigned long len, const char 
*type,
@@ -1663,7 +1667,7 @@ int hash_sha1_file_literally(const void *buf, unsigned 
long len, const char *typ
goto cleanup;
if (freshen_packed_object(oid->hash) || freshen_loose_object(oid->hash))
goto cleanup;
-   status = write_loose_object(oid->hash, header, hdrlen, buf, len, 0);
+   status = write_loose_object(oid, header, hdrlen, buf, len, 0);
 
 cleanup:
free(header);
@@ -1685,7 +1689,7 @@ int force_object_loose(const struct object_id *oid, 
time_t mtime)
if (!buf)
return error("cannot read sha1_file for %s", oid_to_hex(oid));
hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", typename(type), len) + 1;
-   ret = write_loose_object(oid->hash, hdr, hdrlen, buf, len, mtime);
+   ret = write_loose_object(oid, hdr, hdrlen, buf, len, mtime);
free(buf);
 
return ret;
-- 
2.14.3



Re: [PATCH v2 00/14] Some fixes and bunch of object_id conversions

2018-01-22 Thread Patryk Obara
> Patches 1 and 2 should be sent separately though.

I hoped they could be piggy-backed, but since there will be v3 of this patch
series... I'll wait for Junio's opinion.

> I look forward to seeing you deal with the object reading part …

Yeah, even reading of loose objects does not work on my test branch yet.

Turns out some functions (*) depend on each other during conversion to oid -
so they are either refactored together in one BIG commit or I need to deploy
some temporary hacks to make it possible to split work into smaller batches.

(*) refactor of sha1_object_info transitively depends on lookup_replace_object
and the other way around. verify_object and verify_tag will also cause some
problems.

-- 
| ← Ceci n'est pas une pipe
Patryk Obara


Re: [PATCH v2 07/14] match-trees: convert splice_tree to object_id

2018-01-22 Thread Patryk Obara
On 22 January 2018 at 12:56, Duy Nguyen <pclo...@gmail.com> wrote:
> On Mon, Jan 22, 2018 at 12:04:30PM +0100, Patryk Obara wrote:
>> Convert the definition of static recursive splice_tree function to use
>> struct object_id and adjust single caller.
>>
>> Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
>> ---
>>  match-trees.c | 42 --
>>  1 file changed, 20 insertions(+), 22 deletions(-)
>>
>> diff --git a/match-trees.c b/match-trees.c
>> index 396b7338df..0f899a7212 100644
>> --- a/match-trees.c
>> +++ b/match-trees.c
>> @@ -161,19 +161,17 @@ static void match_trees(const struct object_id *hash1,
>>   * A tree "hash1" has a subdirectory at "prefix".  Come up with a
>>   * tree object by replacing it with another tree "hash2".
>>   */
>> -static int splice_tree(const unsigned char *hash1,
>> -const char *prefix,
>> -const unsigned char *hash2,
>> -unsigned char *result)
>> +static int splice_tree(const struct object_id *hash1, const char *prefix,
>> +const struct object_id *hash2, struct object_id *result)
>
> Maybe change the names to oid1 and oid2 too. I had a "what?" moment
> when I read hash1->hash below.

OK

>> @@ -197,26 +195,26 @@ static int splice_tree(const unsigned char *hash1,
>>   if (strlen(name) == toplen &&
>>   !memcmp(name, prefix, toplen)) {
>>   if (!S_ISDIR(mode))
>> - die("entry %s in tree %s is not a tree",
>> - name, sha1_to_hex(hash1));
>> - rewrite_here = (unsigned char *) oid->hash;
>> + die("entry %s in tree %s is not a tree", name,
>> + oid_to_hex(hash1));
>> + rewrite_here = (struct object_id *)oid;
>
> You don't need the typecast here anymore, do you?

Unfortunately, I do :(

Few lines above:
192: const struct object_id *oid;
194: oid = tree_entry_extract(, , );

Function tree_entry_extract returns const pointer, which leads to
compiler warning:
"assigning to 'struct object_id *' from 'const struct object_id *'
discards qualifiers".

On the other hand, if I change const qualifier for 'rewrite_here'
variable - warning will
appear in line 216:

216: oidcpy(rewrite_here, rewrite_with);

So the question here is rather: is it ok to overwrite buffer returned
by tree_entry_extract?

When writing this I opted to preserve cv-qualifiers despite changing
pointer type (which
implied preservation of typecast) - partly because parameter 'desc' of
tree_entry_extract
is NOT const (which suggests to me, that it's ok).

But this cast might be indication of unintended modification inside
tree description
structure and might lead to an error is some other place, if there's
an assumption, that
this buffer is not overwritable.

Maybe const should be removed from return type of tree_entry_extract (and maybe
from oid field of struct name_entry)?

I will give it some more thought - maybe oidcpy from line 216 could be replaced.

-- 
| ← Ceci n'est pas une pipe
Patryk Obara


Re: [PATCH v2 05/14] sha1_file: convert hash_sha1_file to object_id

2018-01-22 Thread Patryk Obara
On 22 January 2018 at 12:49, Duy Nguyen <pclo...@gmail.com> wrote:
> On Mon, Jan 22, 2018 at 12:04:28PM +0100, Patryk Obara wrote:
>> @@ -969,7 +969,7 @@ static int ident_to_worktree(const char *path, const 
>> char *src, size_t len,
>>
>>   /* step 4: substitute */
>>   strbuf_addstr(buf, "Id: ");
>> - strbuf_add(buf, sha1_to_hex(sha1), 40);
>> + strbuf_add(buf, sha1_to_hex(oid.hash), GIT_SHA1_HEXSZ);
>
> oid_to_hex()?

I didn't do it originally because the size of hash is explicitly
passed as the third parameter.
I should probably replace this line with:

strbuf_addstr(buf, oid_to_hex());

... since a hex representation is correctly 0-delimited anyway.
Will include in v3 unless there'll be some other suggestion :)

-- 
| ← Ceci n'est pas une pipe
Patryk Obara


[PATCH v2 09/14] notes: convert combine_notes_* to object_id

2018-01-22 Thread Patryk Obara
Convert the definition and declarations of combine_notes_* functions
to struct object_id and adjust usage of these functions.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 notes.c | 46 +++---
 notes.h | 25 +++--
 2 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/notes.c b/notes.c
index c7f21fae44..3f4f94507a 100644
--- a/notes.c
+++ b/notes.c
@@ -270,8 +270,8 @@ static int note_tree_insert(struct notes_tree *t, struct 
int_node *tree,
if (!oidcmp(>val_oid, >val_oid))
return 0;
 
-   ret = combine_notes(l->val_oid.hash,
-   entry->val_oid.hash);
+   ret = combine_notes(>val_oid,
+   >val_oid);
if (!ret && is_null_oid(>val_oid))
note_tree_remove(t, tree, n, entry);
free(entry);
@@ -786,8 +786,8 @@ static int prune_notes_helper(const struct object_id 
*object_oid,
return 0;
 }
 
-int combine_notes_concatenate(unsigned char *cur_sha1,
-   const unsigned char *new_sha1)
+int combine_notes_concatenate(struct object_id *cur_oid,
+ const struct object_id *new_oid)
 {
char *cur_msg = NULL, *new_msg = NULL, *buf;
unsigned long cur_len, new_len, buf_len;
@@ -795,18 +795,18 @@ int combine_notes_concatenate(unsigned char *cur_sha1,
int ret;
 
/* read in both note blob objects */
-   if (!is_null_sha1(new_sha1))
-   new_msg = read_sha1_file(new_sha1, _type, _len);
+   if (!is_null_oid(new_oid))
+   new_msg = read_sha1_file(new_oid->hash, _type, _len);
if (!new_msg || !new_len || new_type != OBJ_BLOB) {
free(new_msg);
return 0;
}
-   if (!is_null_sha1(cur_sha1))
-   cur_msg = read_sha1_file(cur_sha1, _type, _len);
+   if (!is_null_oid(cur_oid))
+   cur_msg = read_sha1_file(cur_oid->hash, _type, _len);
if (!cur_msg || !cur_len || cur_type != OBJ_BLOB) {
free(cur_msg);
free(new_msg);
-   hashcpy(cur_sha1, new_sha1);
+   oidcpy(cur_oid, new_oid);
return 0;
}
 
@@ -825,20 +825,20 @@ int combine_notes_concatenate(unsigned char *cur_sha1,
free(new_msg);
 
/* create a new blob object from buf */
-   ret = write_sha1_file(buf, buf_len, blob_type, cur_sha1);
+   ret = write_sha1_file(buf, buf_len, blob_type, cur_oid->hash);
free(buf);
return ret;
 }
 
-int combine_notes_overwrite(unsigned char *cur_sha1,
-   const unsigned char *new_sha1)
+int combine_notes_overwrite(struct object_id *cur_oid,
+   const struct object_id *new_oid)
 {
-   hashcpy(cur_sha1, new_sha1);
+   oidcpy(cur_oid, new_oid);
return 0;
 }
 
-int combine_notes_ignore(unsigned char *cur_sha1,
-   const unsigned char *new_sha1)
+int combine_notes_ignore(struct object_id *cur_oid,
+const struct object_id *new_oid)
 {
return 0;
 }
@@ -848,17 +848,17 @@ int combine_notes_ignore(unsigned char *cur_sha1,
  * newlines removed.
  */
 static int string_list_add_note_lines(struct string_list *list,
- const unsigned char *sha1)
+ const struct object_id *oid)
 {
char *data;
unsigned long len;
enum object_type t;
 
-   if (is_null_sha1(sha1))
+   if (is_null_oid(oid))
return 0;
 
/* read_sha1_file NUL-terminates */
-   data = read_sha1_file(sha1, , );
+   data = read_sha1_file(oid->hash, , );
if (t != OBJ_BLOB || !data || !len) {
free(data);
return t != OBJ_BLOB || !data;
@@ -884,17 +884,17 @@ static int string_list_join_lines_helper(struct 
string_list_item *item,
return 0;
 }
 
-int combine_notes_cat_sort_uniq(unsigned char *cur_sha1,
-   const unsigned char *new_sha1)
+int combine_notes_cat_sort_uniq(struct object_id *cur_oid,
+   const struct object_id *new_oid)
 {
struct string_list sort_uniq_list = STRING_LIST_INIT_DUP;
struct strbuf buf = STRBUF_INIT;
int ret = 1;
 
/* read both note blob objects into unique_lines */
-   if (string_list_add_note_lines(_uniq_list, cur_sha1))
+   if (string_list_add_note_lines(_uniq_list, cur_oid))
goto out;
-   if (string_list_add_note_lines(_uniq_list, new_sha1))
+   if (string_list_add_note_lines(_uniq_list, new_oid))
goto out;
string_list_remove_empty_items(_uni

[PATCH v2 14/14] sha1_file: rename hash_sha1_file_literally

2018-01-22 Thread Patryk Obara
This function was already converted to use struct object_id earlier.
---
 builtin/hash-object.c | 3 ++-
 cache.h   | 4 +++-
 sha1_file.c   | 5 +++--
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index c532ff9320..526da5c185 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -24,7 +24,8 @@ static int hash_literally(struct object_id *oid, int fd, 
const char *type, unsig
if (strbuf_read(, fd, 4096) < 0)
ret = -1;
else
-   ret = hash_sha1_file_literally(buf.buf, buf.len, type, oid, 
flags);
+   ret = hash_object_file_literally(buf.buf, buf.len, type, oid,
+flags);
strbuf_release();
return ret;
 }
diff --git a/cache.h b/cache.h
index 0a8be9c87f..6ef4248931 100644
--- a/cache.h
+++ b/cache.h
@@ -1243,7 +1243,9 @@ extern int hash_object_file(const void *buf, unsigned 
long len,
 extern int write_object_file(const void *buf, unsigned long len,
 const char *type, struct object_id *oid);
 
-extern int hash_sha1_file_literally(const void *buf, unsigned long len, const 
char *type, struct object_id *oid, unsigned flags);
+extern int hash_object_file_literally(const void *buf, unsigned long len,
+ const char *type, struct object_id *oid,
+ unsigned flags);
 
 extern int pretend_object_file(void *, unsigned long, enum object_type,
   struct object_id *oid);
diff --git a/sha1_file.c b/sha1_file.c
index 59238f5bea..34c041e8cd 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1652,8 +1652,9 @@ int write_object_file(const void *buf, unsigned long len, 
const char *type,
return write_loose_object(oid, hdr, hdrlen, buf, len, 0);
 }
 
-int hash_sha1_file_literally(const void *buf, unsigned long len, const char 
*type,
-struct object_id *oid, unsigned flags)
+int hash_object_file_literally(const void *buf, unsigned long len,
+  const char *type, struct object_id *oid,
+  unsigned flags)
 {
char *header;
int hdrlen, status = 0;
-- 
2.14.3



[PATCH v2 11/14] sha1_file: convert write_sha1_file to object_id

2018-01-22 Thread Patryk Obara
Convert the definition and declaration of write_sha1_file to
struct object_id and adjust usage of this function.

This commit also converts static function write_sha1_file_prepare, as it
is closely related.

Rename these functions to write_object_file and
write_object_file_prepare respectively.

Replace sha1_to_hex, hashcpy and hashclr with their oid equivalents
wherever possible.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 apply.c  |  8 
 builtin/checkout.c   |  3 +--
 builtin/mktag.c  |  6 +++---
 builtin/mktree.c | 10 +-
 builtin/notes.c  |  8 
 builtin/receive-pack.c   | 11 ++-
 builtin/replace.c|  2 +-
 builtin/tag.c|  2 +-
 builtin/unpack-objects.c |  9 ++---
 cache-tree.c |  5 +++--
 cache.h  |  4 +++-
 commit.c |  2 +-
 match-trees.c|  2 +-
 merge-recursive.c|  5 +++--
 notes-cache.c|  2 +-
 notes.c  |  9 -
 read-cache.c |  6 +++---
 sha1_file.c  | 29 +++--
 18 files changed, 65 insertions(+), 58 deletions(-)

diff --git a/apply.c b/apply.c
index 57ab8a8a29..4cd4504008 100644
--- a/apply.c
+++ b/apply.c
@@ -3554,7 +3554,7 @@ static int try_threeway(struct apply_state *state,
 
/* Preimage the patch was prepared for */
if (patch->is_new)
-   write_sha1_file("", 0, blob_type, pre_oid.hash);
+   write_object_file("", 0, blob_type, _oid);
else if (get_oid(patch->old_sha1_prefix, _oid) ||
 read_blob_object(, _oid, patch->old_mode))
return error(_("repository lacks the necessary blob to fall 
back on 3-way merge."));
@@ -3570,7 +3570,7 @@ static int try_threeway(struct apply_state *state,
return -1;
}
/* post_oid is theirs */
-   write_sha1_file(tmp_image.buf, tmp_image.len, blob_type, post_oid.hash);
+   write_object_file(tmp_image.buf, tmp_image.len, blob_type, _oid);
clear_image(_image);
 
/* our_oid is ours */
@@ -3583,7 +3583,7 @@ static int try_threeway(struct apply_state *state,
return error(_("cannot read the current contents of 
'%s'"),
 patch->old_name);
}
-   write_sha1_file(tmp_image.buf, tmp_image.len, blob_type, our_oid.hash);
+   write_object_file(tmp_image.buf, tmp_image.len, blob_type, _oid);
clear_image(_image);
 
/* in-core three-way merge between post and our using pre as base */
@@ -4291,7 +4291,7 @@ static int add_index_file(struct apply_state *state,
}
fill_stat_cache_info(ce, );
}
-   if (write_sha1_file(buf, size, blob_type, ce->oid.hash) < 0) {
+   if (write_object_file(buf, size, blob_type, >oid) < 0) {
free(ce);
return error(_("unable to create backing store "
   "for newly created file %s"), path);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 8bdc927d3f..ebb6b44660 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -227,8 +227,7 @@ static int checkout_merged(int pos, const struct checkout 
*state)
 * (it also writes the merge result to the object database even
 * when it may contain conflicts).
 */
-   if (write_sha1_file(result_buf.ptr, result_buf.size,
-   blob_type, oid.hash))
+   if (write_object_file(result_buf.ptr, result_buf.size, blob_type, ))
die(_("Unable to add merge result for '%s'"), path);
free(result_buf.ptr);
ce = make_cache_entry(mode, oid.hash, path, 2, 0);
diff --git a/builtin/mktag.c b/builtin/mktag.c
index 031b750f06..beb552847b 100644
--- a/builtin/mktag.c
+++ b/builtin/mktag.c
@@ -151,7 +151,7 @@ static int verify_tag(char *buffer, unsigned long size)
 int cmd_mktag(int argc, const char **argv, const char *prefix)
 {
struct strbuf buf = STRBUF_INIT;
-   unsigned char result_sha1[20];
+   struct object_id result;
 
if (argc != 1)
usage("git mktag");
@@ -165,10 +165,10 @@ int cmd_mktag(int argc, const char **argv, const char 
*prefix)
if (verify_tag(buf.buf, buf.len) < 0)
die("invalid tag signature file");
 
-   if (write_sha1_file(buf.buf, buf.len, tag_type, result_sha1) < 0)
+   if (write_object_file(buf.buf, buf.len, tag_type, ) < 0)
die("unable to write tag file");
 
strbuf_release();
-   printf("%s\n", sha1_to_hex(result_sha1));
+   printf("%s\n", oid_to_hex());
return 0;
 }
diff --git a/builtin/mktree.c b/builtin/mkt

[PATCH v2 07/14] match-trees: convert splice_tree to object_id

2018-01-22 Thread Patryk Obara
Convert the definition of static recursive splice_tree function to use
struct object_id and adjust single caller.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 match-trees.c | 42 --
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/match-trees.c b/match-trees.c
index 396b7338df..0f899a7212 100644
--- a/match-trees.c
+++ b/match-trees.c
@@ -161,19 +161,17 @@ static void match_trees(const struct object_id *hash1,
  * A tree "hash1" has a subdirectory at "prefix".  Come up with a
  * tree object by replacing it with another tree "hash2".
  */
-static int splice_tree(const unsigned char *hash1,
-  const char *prefix,
-  const unsigned char *hash2,
-  unsigned char *result)
+static int splice_tree(const struct object_id *hash1, const char *prefix,
+  const struct object_id *hash2, struct object_id *result)
 {
char *subpath;
int toplen;
char *buf;
unsigned long sz;
struct tree_desc desc;
-   unsigned char *rewrite_here;
-   const unsigned char *rewrite_with;
-   unsigned char subtree[20];
+   struct object_id *rewrite_here;
+   const struct object_id *rewrite_with;
+   struct object_id subtree;
enum object_type type;
int status;
 
@@ -182,9 +180,9 @@ static int splice_tree(const unsigned char *hash1,
if (*subpath)
subpath++;
 
-   buf = read_sha1_file(hash1, , );
+   buf = read_sha1_file(hash1->hash, , );
if (!buf)
-   die("cannot read tree %s", sha1_to_hex(hash1));
+   die("cannot read tree %s", oid_to_hex(hash1));
init_tree_desc(, buf, sz);
 
rewrite_here = NULL;
@@ -197,26 +195,26 @@ static int splice_tree(const unsigned char *hash1,
if (strlen(name) == toplen &&
!memcmp(name, prefix, toplen)) {
if (!S_ISDIR(mode))
-   die("entry %s in tree %s is not a tree",
-   name, sha1_to_hex(hash1));
-   rewrite_here = (unsigned char *) oid->hash;
+   die("entry %s in tree %s is not a tree", name,
+   oid_to_hex(hash1));
+   rewrite_here = (struct object_id *)oid;
break;
}
update_tree_entry();
}
if (!rewrite_here)
-   die("entry %.*s not found in tree %s",
-   toplen, prefix, sha1_to_hex(hash1));
+   die("entry %.*s not found in tree %s", toplen, prefix,
+   oid_to_hex(hash1));
if (*subpath) {
-   status = splice_tree(rewrite_here, subpath, hash2, subtree);
+   status = splice_tree(rewrite_here, subpath, hash2, );
if (status)
return status;
-   rewrite_with = subtree;
-   }
-   else
+   rewrite_with = 
+   } else {
rewrite_with = hash2;
-   hashcpy(rewrite_here, rewrite_with);
-   status = write_sha1_file(buf, sz, tree_type, result);
+   }
+   oidcpy(rewrite_here, rewrite_with);
+   status = write_sha1_file(buf, sz, tree_type, result->hash);
free(buf);
return status;
 }
@@ -280,7 +278,7 @@ void shift_tree(const struct object_id *hash1,
if (!*add_prefix)
return;
 
-   splice_tree(hash1->hash, add_prefix, hash2->hash, shifted->hash);
+   splice_tree(hash1, add_prefix, hash2, shifted);
 }
 
 /*
@@ -334,7 +332,7 @@ void shift_tree_by(const struct object_id *hash1,
 * shift tree2 down by adding shift_prefix above it
 * to match tree1.
 */
-   splice_tree(hash1->hash, shift_prefix, hash2->hash, 
shifted->hash);
+   splice_tree(hash1, shift_prefix, hash2, shifted);
else
/*
 * shift tree2 up by removing shift_prefix from it
-- 
2.14.3



[PATCH v2 03/14] sha1_file: convert pretend_sha1_file to object_id

2018-01-22 Thread Patryk Obara
Convert the declaration and definition of pretend_sha1_file to use
struct object_id and adjust all usages of this function.  Rename it to
pretend_object_file.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 Documentation/technical/api-object-access.txt |  2 +-
 blame.c   |  2 +-
 cache.h   |  5 -
 sha1_file.c   | 10 +-
 4 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/Documentation/technical/api-object-access.txt 
b/Documentation/technical/api-object-access.txt
index 03bb0e950d..a1162e5bcd 100644
--- a/Documentation/technical/api-object-access.txt
+++ b/Documentation/technical/api-object-access.txt
@@ -7,7 +7,7 @@ Talk about  and  family, things like
 * read_object_with_reference()
 * has_sha1_file()
 * write_sha1_file()
-* pretend_sha1_file()
+* pretend_object_file()
 * lookup_{object,commit,tag,blob,tree}
 * parse_{object,commit,tag,blob,tree}
 * Use of object flags
diff --git a/blame.c b/blame.c
index 2893f3c103..1fc22b304b 100644
--- a/blame.c
+++ b/blame.c
@@ -232,7 +232,7 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,
convert_to_git(_index, path, buf.buf, buf.len, , 0);
origin->file.ptr = buf.buf;
origin->file.size = buf.len;
-   pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_oid.hash);
+   pretend_object_file(buf.buf, buf.len, OBJ_BLOB, >blob_oid);
 
/*
 * Read the current index, replace the path entry with
diff --git a/cache.h b/cache.h
index d8b975a571..e4e03ac51d 100644
--- a/cache.h
+++ b/cache.h
@@ -1241,7 +1241,10 @@ extern int sha1_object_info(const unsigned char *, 
unsigned long *);
 extern int hash_sha1_file(const void *buf, unsigned long len, const char 
*type, unsigned char *sha1);
 extern int write_sha1_file(const void *buf, unsigned long len, const char 
*type, unsigned char *return_sha1);
 extern int hash_sha1_file_literally(const void *buf, unsigned long len, const 
char *type, struct object_id *oid, unsigned flags);
-extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned 
char *);
+
+extern int pretend_object_file(void *, unsigned long, enum object_type,
+  struct object_id *oid);
+
 extern int force_object_loose(const unsigned char *sha1, time_t mtime);
 extern int git_open_cloexec(const char *name, int flags);
 #define git_open(name) git_open_cloexec(name, O_RDONLY)
diff --git a/sha1_file.c b/sha1_file.c
index 3da70ac650..830b93b428 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1312,13 +1312,13 @@ static void *read_object(const unsigned char *sha1, 
enum object_type *type,
return content;
 }
 
-int pretend_sha1_file(void *buf, unsigned long len, enum object_type type,
- unsigned char *sha1)
+int pretend_object_file(void *buf, unsigned long len, enum object_type type,
+   struct object_id *oid)
 {
struct cached_object *co;
 
-   hash_sha1_file(buf, len, typename(type), sha1);
-   if (has_sha1_file(sha1) || find_cached_object(sha1))
+   hash_sha1_file(buf, len, typename(type), oid->hash);
+   if (has_sha1_file(oid->hash) || find_cached_object(oid->hash))
return 0;
ALLOC_GROW(cached_objects, cached_object_nr + 1, cached_object_alloc);
co = _objects[cached_object_nr++];
@@ -1326,7 +1326,7 @@ int pretend_sha1_file(void *buf, unsigned long len, enum 
object_type type,
co->type = type;
co->buf = xmalloc(len);
memcpy(co->buf, buf, len);
-   hashcpy(co->sha1, sha1);
+   hashcpy(co->sha1, oid->hash);
return 0;
 }
 
-- 
2.14.3



[PATCH v2 00/14] Some fixes and bunch of object_id conversions

2018-01-22 Thread Patryk Obara
Compared to v1:

Following brian's suggestion I renamed following functions and struct
names to indicate, that they are no longer intended for sha1 algorithm
only:

struct sha1_stat -> struct oid_stat
pretend_sha1_file-> pretend_object_file
write_sha1_file  -> write_object_file
hash_sha1_file   -> hash_object_file
hash_sha1_file_literally -> hash_object_file_literally

Added two more patches converting some more functions to struct object_id
and one with pure function rename.

Patryk Obara (14):
  http-push: improve error log
  clang-format: adjust penalty for return type line break
  sha1_file: convert pretend_sha1_file to object_id
  dir: convert struct sha1_stat to use object_id
  sha1_file: convert hash_sha1_file to object_id
  cache: clear whole hash buffer with oidclr
  match-trees: convert splice_tree to object_id
  commit: convert commit_tree* to object_id
  notes: convert combine_notes_* to object_id
  notes: convert write_notes_tree to object_id
  sha1_file: convert write_sha1_file to object_id
  sha1_file: convert force_object_loose to object_id
  sha1_file: convert write_loose_object to object_id
  sha1_file: rename hash_sha1_file_literally

 .clang-format |   2 +-
 Documentation/technical/api-object-access.txt |   2 +-
 apply.c   |  12 ++--
 blame.c   |   2 +-
 builtin/am.c  |   4 +-
 builtin/checkout.c|   3 +-
 builtin/commit-tree.c |   4 +-
 builtin/commit.c  |   5 +-
 builtin/hash-object.c |   3 +-
 builtin/index-pack.c  |   5 +-
 builtin/merge.c   |   8 +--
 builtin/mktag.c   |   6 +-
 builtin/mktree.c  |  10 +--
 builtin/notes.c   |   8 +--
 builtin/pack-objects.c|   2 +-
 builtin/receive-pack.c|  11 +--
 builtin/replace.c |   4 +-
 builtin/tag.c |   2 +-
 builtin/unpack-objects.c  |  11 +--
 cache-tree.c  |  16 ++---
 cache.h   |  25 ---
 commit.c  |  15 ++--
 commit.h  |  11 +--
 convert.c |   6 +-
 diffcore-rename.c |   4 +-
 dir.c |  56 +++
 dir.h |  12 ++--
 http-push.c   |   4 ++
 log-tree.c|   2 +-
 match-trees.c |  42 ++-
 merge-recursive.c |   5 +-
 notes-cache.c |   8 +--
 notes-merge.c |   9 ++-
 notes-utils.c |   9 +--
 notes-utils.h |   3 +-
 notes.c   |  63 
 notes.h   |  29 
 read-cache.c  |   6 +-
 sha1_file.c   | 100 ++
 t/helper/test-dump-untracked-cache.c  |   4 +-
 40 files changed, 279 insertions(+), 254 deletions(-)


base-commit: 59c276cf4da0705064c32c9dba54baefa282ea55
-- 
2.14.3



[PATCH v2 13/14] sha1_file: convert write_loose_object to object_id

2018-01-22 Thread Patryk Obara
Convert the definition and declaration of statis write_loose_object
function to struct object_id.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 sha1_file.c | 28 
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index d9ee966d74..59238f5bea 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1548,16 +1548,17 @@ static int create_tmpfile(struct strbuf *tmp, const 
char *filename)
return fd;
 }
 
-static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen,
- const void *buf, unsigned long len, time_t mtime)
+static int write_loose_object(const struct object_id *oid, char *hdr,
+ int hdrlen, const void *buf, unsigned long len,
+ time_t mtime)
 {
int fd, ret;
unsigned char compressed[4096];
git_zstream stream;
git_SHA_CTX c;
-   unsigned char parano_sha1[20];
+   struct object_id parano_oid;
static struct strbuf tmp_file = STRBUF_INIT;
-   const char *filename = sha1_file_name(sha1);
+   const char *filename = sha1_file_name(oid->hash);
 
fd = create_tmpfile(_file, filename);
if (fd < 0) {
@@ -1594,13 +1595,16 @@ static int write_loose_object(const unsigned char 
*sha1, char *hdr, int hdrlen,
} while (ret == Z_OK);
 
if (ret != Z_STREAM_END)
-   die("unable to deflate new object %s (%d)", sha1_to_hex(sha1), 
ret);
+   die("unable to deflate new object %s (%d)", oid_to_hex(oid),
+   ret);
ret = git_deflate_end_gently();
if (ret != Z_OK)
-   die("deflateEnd on object %s failed (%d)", sha1_to_hex(sha1), 
ret);
-   git_SHA1_Final(parano_sha1, );
-   if (hashcmp(sha1, parano_sha1) != 0)
-   die("confused by unstable object source data for %s", 
sha1_to_hex(sha1));
+   die("deflateEnd on object %s failed (%d)", oid_to_hex(oid),
+   ret);
+   git_SHA1_Final(parano_oid.hash, );
+   if (oidcmp(oid, _oid) != 0)
+   die("confused by unstable object source data for %s",
+   oid_to_hex(oid));
 
close_sha1_file(fd);
 
@@ -1645,7 +1649,7 @@ int write_object_file(const void *buf, unsigned long len, 
const char *type,
write_object_file_prepare(buf, len, type, oid, hdr, );
if (freshen_packed_object(oid->hash) || freshen_loose_object(oid->hash))
return 0;
-   return write_loose_object(oid->hash, hdr, hdrlen, buf, len, 0);
+   return write_loose_object(oid, hdr, hdrlen, buf, len, 0);
 }
 
 int hash_sha1_file_literally(const void *buf, unsigned long len, const char 
*type,
@@ -1663,7 +1667,7 @@ int hash_sha1_file_literally(const void *buf, unsigned 
long len, const char *typ
goto cleanup;
if (freshen_packed_object(oid->hash) || freshen_loose_object(oid->hash))
goto cleanup;
-   status = write_loose_object(oid->hash, header, hdrlen, buf, len, 0);
+   status = write_loose_object(oid, header, hdrlen, buf, len, 0);
 
 cleanup:
free(header);
@@ -1685,7 +1689,7 @@ int force_object_loose(const struct object_id *oid, 
time_t mtime)
if (!buf)
return error("cannot read sha1_file for %s", oid_to_hex(oid));
hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", typename(type), len) + 1;
-   ret = write_loose_object(oid->hash, hdr, hdrlen, buf, len, mtime);
+   ret = write_loose_object(oid, hdr, hdrlen, buf, len, mtime);
free(buf);
 
return ret;
-- 
2.14.3



[PATCH v2 12/14] sha1_file: convert force_object_loose to object_id

2018-01-22 Thread Patryk Obara
Convert the definition and declaration of force_object_loose to
struct object_id and adjust usage of this function.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 builtin/pack-objects.c |  2 +-
 cache.h|  3 ++-
 sha1_file.c| 10 +-
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 6b9cfc289d..f38197543d 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2768,7 +2768,7 @@ static void loosen_unused_packed_objects(struct rev_info 
*revs)
if (!packlist_find(_pack, oid.hash, NULL) &&
!has_sha1_pack_kept_or_nonlocal() &&
!loosened_object_can_be_discarded(, p->mtime))
-   if (force_object_loose(oid.hash, p->mtime))
+   if (force_object_loose(, p->mtime))
die("unable to force loose object");
}
}
diff --git a/cache.h b/cache.h
index d80141eb64..0a8be9c87f 100644
--- a/cache.h
+++ b/cache.h
@@ -1248,7 +1248,8 @@ extern int hash_sha1_file_literally(const void *buf, 
unsigned long len, const ch
 extern int pretend_object_file(void *, unsigned long, enum object_type,
   struct object_id *oid);
 
-extern int force_object_loose(const unsigned char *sha1, time_t mtime);
+extern int force_object_loose(const struct object_id *oid, time_t mtime);
+
 extern int git_open_cloexec(const char *name, int flags);
 #define git_open(name) git_open_cloexec(name, O_RDONLY)
 extern void *map_sha1_file(const unsigned char *sha1, unsigned long *size);
diff --git a/sha1_file.c b/sha1_file.c
index d1569b1b96..d9ee966d74 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1670,7 +1670,7 @@ int hash_sha1_file_literally(const void *buf, unsigned 
long len, const char *typ
return status;
 }
 
-int force_object_loose(const unsigned char *sha1, time_t mtime)
+int force_object_loose(const struct object_id *oid, time_t mtime)
 {
void *buf;
unsigned long len;
@@ -1679,13 +1679,13 @@ int force_object_loose(const unsigned char *sha1, 
time_t mtime)
int hdrlen;
int ret;
 
-   if (has_loose_object(sha1))
+   if (has_loose_object(oid->hash))
return 0;
-   buf = read_object(sha1, , );
+   buf = read_object(oid->hash, , );
if (!buf)
-   return error("cannot read sha1_file for %s", sha1_to_hex(sha1));
+   return error("cannot read sha1_file for %s", oid_to_hex(oid));
hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", typename(type), len) + 1;
-   ret = write_loose_object(sha1, hdr, hdrlen, buf, len, mtime);
+   ret = write_loose_object(oid->hash, hdr, hdrlen, buf, len, mtime);
free(buf);
 
return ret;
-- 
2.14.3



[PATCH v2 05/14] sha1_file: convert hash_sha1_file to object_id

2018-01-22 Thread Patryk Obara
Convert the declaration and definition of hash_sha1_file to use
struct object_id and adjust all function calls.

Rename this function to hash_object_file.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 apply.c  |  4 ++--
 builtin/index-pack.c |  5 ++---
 builtin/replace.c|  2 +-
 builtin/unpack-objects.c |  2 +-
 cache-tree.c | 11 +--
 cache.h  |  5 -
 convert.c|  6 +++---
 diffcore-rename.c|  4 ++--
 dir.c|  4 ++--
 log-tree.c   |  2 +-
 sha1_file.c  | 26 +-
 11 files changed, 36 insertions(+), 35 deletions(-)

diff --git a/apply.c b/apply.c
index 321a9fa68d..57ab8a8a29 100644
--- a/apply.c
+++ b/apply.c
@@ -3154,7 +3154,7 @@ static int apply_binary(struct apply_state *state,
 * See if the old one matches what the patch
 * applies to.
 */
-   hash_sha1_file(img->buf, img->len, blob_type, oid.hash);
+   hash_object_file(img->buf, img->len, blob_type, );
if (strcmp(oid_to_hex(), patch->old_sha1_prefix))
return error(_("the patch applies to '%s' (%s), "
   "which does not match the "
@@ -3199,7 +3199,7 @@ static int apply_binary(struct apply_state *state,
 name);
 
/* verify that the result matches */
-   hash_sha1_file(img->buf, img->len, blob_type, oid.hash);
+   hash_object_file(img->buf, img->len, blob_type, );
if (strcmp(oid_to_hex(), patch->new_sha1_prefix))
return error(_("binary patch to '%s' creates incorrect 
result (expecting %s, got %s)"),
name, patch->new_sha1_prefix, oid_to_hex());
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 4c51aec81f..7f5a95e6ff 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -958,9 +958,8 @@ static void resolve_delta(struct object_entry *delta_obj,
free(delta_data);
if (!result->data)
bad_object(delta_obj->idx.offset, _("failed to apply delta"));
-   hash_sha1_file(result->data, result->size,
-  typename(delta_obj->real_type),
-  delta_obj->idx.oid.hash);
+   hash_object_file(result->data, result->size,
+typename(delta_obj->real_type), _obj->idx.oid);
sha1_object(result->data, NULL, result->size, delta_obj->real_type,
_obj->idx.oid);
counter_lock();
diff --git a/builtin/replace.c b/builtin/replace.c
index 10078ae371..814bf6bfde 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -355,7 +355,7 @@ static void check_one_mergetag(struct commit *commit,
struct tag *tag;
int i;
 
-   hash_sha1_file(extra->value, extra->len, typename(OBJ_TAG), 
tag_oid.hash);
+   hash_object_file(extra->value, extra->len, typename(OBJ_TAG), _oid);
tag = lookup_tag(_oid);
if (!tag)
die(_("bad mergetag in commit '%s'"), ref);
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 62ea264c46..85a40d1af7 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -258,7 +258,7 @@ static void write_object(unsigned nr, enum object_type type,
} else {
struct object *obj;
int eaten;
-   hash_sha1_file(buf, size, typename(type), 
obj_list[nr].oid.hash);
+   hash_object_file(buf, size, typename(type), _list[nr].oid);
added_object(nr, type, buf, size);
obj = parse_object_buffer(_list[nr].oid, type, size, buf,
  );
diff --git a/cache-tree.c b/cache-tree.c
index e03e72c34a..6574eeb80d 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -400,15 +400,14 @@ static int update_one(struct cache_tree *it,
}
 
if (repair) {
-   unsigned char sha1[20];
-   hash_sha1_file(buffer.buf, buffer.len, tree_type, sha1);
-   if (has_sha1_file(sha1))
-   hashcpy(it->oid.hash, sha1);
+   struct object_id oid;
+   hash_object_file(buffer.buf, buffer.len, tree_type, );
+   if (has_sha1_file(oid.hash))
+   oidcpy(>oid, );
else
to_invalidate = 1;
} else if (dryrun)
-   hash_sha1_file(buffer.buf, buffer.len, tree_type,
-  it->oid.hash);
+   hash_object_file(buffer.buf, buffer.len, tree_type, >oid);
else if (write_sha1_file(buffer.buf, buffer.len, tree_type, 
it->oid.hash)) {
s

[PATCH v2 08/14] commit: convert commit_tree* to object_id

2018-01-22 Thread Patryk Obara
Convert the definitions and declarations of commit_tree and
commit_tree_extended to use struct object_id and adjust all usages of
these functions.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 builtin/am.c  |  4 ++--
 builtin/commit-tree.c |  4 ++--
 builtin/commit.c  |  5 +++--
 builtin/merge.c   |  8 
 commit.c  | 15 +++
 commit.h  | 11 ++-
 notes-cache.c |  4 ++--
 notes-merge.c |  9 -
 notes-utils.c |  7 ---
 notes-utils.h |  3 ++-
 10 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index acfe9d3c8c..6e6abb05cd 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1641,8 +1641,8 @@ static void do_commit(const struct am_state *state)
setenv("GIT_COMMITTER_DATE",
state->ignore_date ? "" : state->author_date, 1);
 
-   if (commit_tree(state->msg, state->msg_len, tree.hash, parents, 
commit.hash,
-   author, state->sign_commit))
+   if (commit_tree(state->msg, state->msg_len, , parents, ,
+   author, state->sign_commit))
die(_("failed to write commit object"));
 
reflog_msg = getenv("GIT_REFLOG_ACTION");
diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
index 2177251e24..e5bdf57b1e 100644
--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c
@@ -117,8 +117,8 @@ int cmd_commit_tree(int argc, const char **argv, const char 
*prefix)
die_errno("git commit-tree: failed to read");
}
 
-   if (commit_tree(buffer.buf, buffer.len, tree_oid.hash, parents,
-   commit_oid.hash, NULL, sign_commit)) {
+   if (commit_tree(buffer.buf, buffer.len, _oid, parents, _oid,
+   NULL, sign_commit)) {
strbuf_release();
return 1;
}
diff --git a/builtin/commit.c b/builtin/commit.c
index 8a87701414..c14878302e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1792,8 +1792,9 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
append_merge_tag_headers(parents, );
}
 
-   if (commit_tree_extended(sb.buf, sb.len, active_cache_tree->oid.hash,
-parents, oid.hash, author_ident.buf, sign_commit, 
extra)) {
+   if (commit_tree_extended(sb.buf, sb.len, _cache_tree->oid,
+parents, , author_ident.buf, sign_commit,
+extra)) {
rollback_index_files();
die(_("failed to write commit object"));
}
diff --git a/builtin/merge.c b/builtin/merge.c
index 30264cfd7c..92ba99a1a5 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -820,8 +820,8 @@ static int merge_trivial(struct commit *head, struct 
commit_list *remoteheads)
pptr = commit_list_append(head, pptr);
pptr = commit_list_append(remoteheads->item, pptr);
prepare_to_commit(remoteheads);
-   if (commit_tree(merge_msg.buf, merge_msg.len, result_tree.hash, parents,
-   result_commit.hash, NULL, sign_commit))
+   if (commit_tree(merge_msg.buf, merge_msg.len, _tree, parents,
+   _commit, NULL, sign_commit))
die(_("failed to write commit object"));
finish(head, remoteheads, _commit, "In-index merge");
drop_save();
@@ -845,8 +845,8 @@ static int finish_automerge(struct commit *head,
commit_list_insert(head, );
strbuf_addch(_msg, '\n');
prepare_to_commit(remoteheads);
-   if (commit_tree(merge_msg.buf, merge_msg.len, result_tree->hash, 
parents,
-   result_commit.hash, NULL, sign_commit))
+   if (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents,
+   _commit, NULL, sign_commit))
die(_("failed to write commit object"));
strbuf_addf(, "Merge made by the '%s' strategy.", wt_strategy);
finish(head, remoteheads, _commit, buf.buf);
diff --git a/commit.c b/commit.c
index cab8d4455b..760137e54b 100644
--- a/commit.c
+++ b/commit.c
@@ -1395,9 +1395,8 @@ void free_commit_extra_headers(struct commit_extra_header 
*extra)
}
 }
 
-int commit_tree(const char *msg, size_t msg_len,
-   const unsigned char *tree,
-   struct commit_list *parents, unsigned char *ret,
+int commit_tree(const char *msg, size_t msg_len, const struct object_id *tree,
+   struct commit_list *parents, struct object_id *ret,
const char *author, const char *sign_commit)
 {
struct commit_extra_header *extra = NULL, **tail = 
@@ -1526,8 +1525,8 @@ N_("Warning: commit message did not conform to UTF-8.\n"
"variable i18

[PATCH v2 01/14] http-push: improve error log

2018-01-22 Thread Patryk Obara
When git push fails due to server-side WebDAV error, it's not easy to
point to the main culprit.  Additional information about exact cURL
error and HTTP server response is helpful for debugging purpose.

New error log helped me pinpoint failing test t5540-http-push-webdav
to a missing Apache dependency in Fedora 27:
https://bugzilla.redhat.com/show_bug.cgi?id=1491151

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 http-push.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/http-push.c b/http-push.c
index 14435ab65d..0913f8ab86 100644
--- a/http-push.c
+++ b/http-push.c
@@ -915,6 +915,10 @@ static struct remote_lock *lock_remote(const char *path, 
long timeout)
lock->timeout = -1;
}
XML_ParserFree(parser);
+   } else {
+   fprintf(stderr,
+   "error: curl result=%d, HTTP code=%ld\n",
+   results.curl_result, results.http_code);
}
} else {
fprintf(stderr, "Unable to start LOCK request\n");
-- 
2.14.3



[PATCH v2 10/14] notes: convert write_notes_tree to object_id

2018-01-22 Thread Patryk Obara
Convert the definition and declaration of write_notes_tree to
struct object_id and adjust usage of this function.

Additionally, improve style of small part of this function, as old
formatting made it hard to understand at glance what this part of
code is doing.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 notes-cache.c |  2 +-
 notes-utils.c |  2 +-
 notes.c   | 16 +---
 notes.h   |  4 ++--
 4 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/notes-cache.c b/notes-cache.c
index d2f87147cc..010ad236cb 100644
--- a/notes-cache.c
+++ b/notes-cache.c
@@ -54,7 +54,7 @@ int notes_cache_write(struct notes_cache *c)
if (!c->tree.dirty)
return 0;
 
-   if (write_notes_tree(>tree, tree_oid.hash))
+   if (write_notes_tree(>tree, _oid))
return -1;
if (commit_tree(c->validity, strlen(c->validity), _oid, NULL,
_oid, NULL, NULL) < 0)
diff --git a/notes-utils.c b/notes-utils.c
index 058c642dac..02407fe2a7 100644
--- a/notes-utils.c
+++ b/notes-utils.c
@@ -12,7 +12,7 @@ void create_notes_commit(struct notes_tree *t, struct 
commit_list *parents,
 
assert(t->initialized);
 
-   if (write_notes_tree(t, tree_oid.hash))
+   if (write_notes_tree(t, _oid))
die("Failed to write notes tree to database");
 
if (!parents) {
diff --git a/notes.c b/notes.c
index 3f4f94507a..09ef1ce33a 100644
--- a/notes.c
+++ b/notes.c
@@ -1123,11 +1123,12 @@ int for_each_note(struct notes_tree *t, int flags, 
each_note_fn fn,
return for_each_note_helper(t, t->root, 0, 0, flags, fn, cb_data);
 }
 
-int write_notes_tree(struct notes_tree *t, unsigned char *result)
+int write_notes_tree(struct notes_tree *t, struct object_id *result)
 {
struct tree_write_stack root;
struct write_each_note_data cb_data;
int ret;
+   int flags;
 
if (!t)
t = _notes_tree;
@@ -1141,12 +1142,13 @@ int write_notes_tree(struct notes_tree *t, unsigned 
char *result)
cb_data.next_non_note = t->first_non_note;
 
/* Write tree objects representing current notes tree */
-   ret = for_each_note(t, FOR_EACH_NOTE_DONT_UNPACK_SUBTREES |
-   FOR_EACH_NOTE_YIELD_SUBTREES,
-   write_each_note, _data) ||
-   write_each_non_note_until(NULL, _data) ||
-   tree_write_stack_finish_subtree() ||
-   write_sha1_file(root.buf.buf, root.buf.len, tree_type, result);
+   flags = FOR_EACH_NOTE_DONT_UNPACK_SUBTREES |
+   FOR_EACH_NOTE_YIELD_SUBTREES;
+   ret = for_each_note(t, flags, write_each_note, _data) ||
+ write_each_non_note_until(NULL, _data) ||
+ tree_write_stack_finish_subtree() ||
+ write_sha1_file(root.buf.buf, root.buf.len, tree_type,
+ result->hash);
strbuf_release();
return ret;
 }
diff --git a/notes.h b/notes.h
index 88da38b5f4..0433f45db5 100644
--- a/notes.h
+++ b/notes.h
@@ -217,7 +217,7 @@ int for_each_note(struct notes_tree *t, int flags, 
each_note_fn fn,
  * Write the given notes_tree structure to the object database
  *
  * Creates a new tree object encapsulating the current state of the given
- * notes_tree, and stores its SHA1 into the 'result' argument.
+ * notes_tree, and stores its object id into the 'result' argument.
  *
  * Returns zero on success, non-zero on failure.
  *
@@ -225,7 +225,7 @@ int for_each_note(struct notes_tree *t, int flags, 
each_note_fn fn,
  * this function has returned zero. Please also remember to create a
  * corresponding commit object, and update the appropriate notes ref.
  */
-int write_notes_tree(struct notes_tree *t, unsigned char *result);
+int write_notes_tree(struct notes_tree *t, struct object_id *result);
 
 /* Flags controlling the operation of prune */
 #define NOTES_PRUNE_VERBOSE 1
-- 
2.14.3



[PATCH v2 02/14] clang-format: adjust penalty for return type line break

2018-01-22 Thread Patryk Obara
The penalty of 5 makes clang-format very eager to put even short type
declarations (e.g. "extern int") into a separate line, even when
breaking parameters list is sufficient.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 .clang-format | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.clang-format b/.clang-format
index 611ab4750b..12a89f95f9 100644
--- a/.clang-format
+++ b/.clang-format
@@ -163,7 +163,7 @@ PenaltyBreakComment: 10
 PenaltyBreakFirstLessLess: 0
 PenaltyBreakString: 10
 PenaltyExcessCharacter: 100
-PenaltyReturnTypeOnItsOwnLine: 5
+PenaltyReturnTypeOnItsOwnLine: 60
 
 # Don't sort #include's
 SortIncludes: false
-- 
2.14.3



[PATCH v2 04/14] dir: convert struct sha1_stat to use object_id

2018-01-22 Thread Patryk Obara
Convert the declaration of struct sha1_stat. Adjust all usages of this
struct and replace hash{clr,cmp,cpy} with oid{clr,cmp,cpy} wherever
possible.  Rename it to struct oid_stat.

Remove macro EMPTY_BLOB_SHA1_BIN, as it's no longer used.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 cache.h  |  2 --
 dir.c| 56 +---
 dir.h| 12 
 t/helper/test-dump-untracked-cache.c |  4 +--
 4 files changed, 34 insertions(+), 40 deletions(-)

diff --git a/cache.h b/cache.h
index e4e03ac51d..ed72933ba7 100644
--- a/cache.h
+++ b/cache.h
@@ -1047,8 +1047,6 @@ extern const struct object_id empty_tree_oid;
"\xe6\x9d\xe2\x9b\xb2\xd1\xd6\x43\x4b\x8b" \
"\x29\xae\x77\x5a\xd8\xc2\xe4\x8c\x53\x91"
 extern const struct object_id empty_blob_oid;
-#define EMPTY_BLOB_SHA1_BIN (empty_blob_oid.hash)
-
 
 static inline int is_empty_blob_sha1(const unsigned char *sha1)
 {
diff --git a/dir.c b/dir.c
index 7c4b45e30e..ef977b8657 100644
--- a/dir.c
+++ b/dir.c
@@ -233,10 +233,8 @@ int within_depth(const char *name, int namelen,
  *
  * Optionally updates the given sha1_stat with the given OID (when valid).
  */
-static int do_read_blob(const struct object_id *oid,
-   struct sha1_stat *sha1_stat,
-   size_t *size_out,
-   char **data_out)
+static int do_read_blob(const struct object_id *oid, struct oid_stat 
*sha1_stat,
+   size_t *size_out, char **data_out)
 {
enum object_type type;
unsigned long sz;
@@ -253,7 +251,7 @@ static int do_read_blob(const struct object_id *oid,
 
if (sha1_stat) {
memset(_stat->stat, 0, sizeof(sha1_stat->stat));
-   hashcpy(sha1_stat->sha1, oid->hash);
+   oidcpy(_stat->oid, oid);
}
 
if (sz == 0) {
@@ -654,9 +652,8 @@ void add_exclude(const char *string, const char *base,
 
 static int read_skip_worktree_file_from_index(const struct index_state *istate,
  const char *path,
- size_t *size_out,
- char **data_out,
- struct sha1_stat *sha1_stat)
+ size_t *size_out, char **data_out,
+ struct oid_stat *sha1_stat)
 {
int pos, len;
 
@@ -795,9 +792,8 @@ static int add_excludes_from_buffer(char *buf, size_t size,
  * ss_valid is non-zero, "ss" must contain good value as input.
  */
 static int add_excludes(const char *fname, const char *base, int baselen,
-   struct exclude_list *el,
-   struct index_state *istate,
-   struct sha1_stat *sha1_stat)
+   struct exclude_list *el, struct index_state *istate,
+   struct oid_stat *sha1_stat)
 {
struct stat st;
int r;
@@ -823,7 +819,7 @@ static int add_excludes(const char *fname, const char 
*base, int baselen,
if (size == 0) {
if (sha1_stat) {
fill_stat_data(_stat->stat, );
-   hashcpy(sha1_stat->sha1, EMPTY_BLOB_SHA1_BIN);
+   oidcpy(_stat->oid, _blob_oid);
sha1_stat->valid = 1;
}
close(fd);
@@ -847,10 +843,11 @@ static int add_excludes(const char *fname, const char 
*base, int baselen,
 !ce_stage(istate->cache[pos]) &&
 ce_uptodate(istate->cache[pos]) &&
 !would_convert_to_git(istate, fname))
-   hashcpy(sha1_stat->sha1,
-   istate->cache[pos]->oid.hash);
+   oidcpy(_stat->oid,
+  >cache[pos]->oid);
else
-   hash_sha1_file(buf, size, "blob", 
sha1_stat->sha1);
+   hash_sha1_file(buf, size, "blob",
+  sha1_stat->oid.hash);
fill_stat_data(_stat->stat, );
sha1_stat->valid = 1;
}
@@ -930,7 +927,7 @@ struct exclude_list *add_exclude_list(struct dir_struct 
*dir,
  * Used to set up core.excludesfile and .git/info/exclude lists.
  */
 static void add_excludes_from_file_1(struct dir_struct *dir, const char *fname,
-struct sha1_stat *sha1_stat)
+struct oid_stat *sha1_stat

[PATCH v2 06/14] cache: clear whole hash buffer with oidclr

2018-01-22 Thread Patryk Obara
As long as GIT_SHA1_RAWSZ is equal to GIT_MAX_RAWSZ there's no problem,
but when new hashing algorithm will be in place this memset will clear
only 20-byte prefix of hash buffer.

Alternatively, hashclr implementation could be adjusted, but this
function is almost removed from codebase already.  Separate
implementation of oidclr prevents potential buffer overrun in case
someone incorrectly used hashclr on object_id in future.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 cache.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cache.h b/cache.h
index 08f2b81e1b..d5d78d6a51 100644
--- a/cache.h
+++ b/cache.h
@@ -1029,7 +1029,7 @@ static inline void hashclr(unsigned char *hash)
 
 static inline void oidclr(struct object_id *oid)
 {
-   hashclr(oid->hash);
+   memset(oid->hash, 0, GIT_MAX_RAWSZ);
 }
 
 
-- 
2.14.3



Re: [PATCH 00/11] Some fixes and bunch of object_id conversions

2018-01-21 Thread Patryk Obara
On 20 January 2018 at 21:58, brian m. carlson
<sand...@crustytoothpaste.net> wrote:

> When I've made changes to the sha1_file functions, I've traditionally
> moved them away from using "sha1_file" to "object_file" to ensure that
> we make it a bit more obvious that they handle object_id structs and
> aren't limited to SHA-1.  For consistency, it might be nice to make that
> change.

I will address this in V2 and add several more conversions, that
are sufficient to make git hash-object -w work with sha-256 (which I will
send as separate RFC thread).

I started converting functions needed for cat-file, but number of places,
that still needs to be touched up grew quite rapidly, so it'll take some
time :).

-- 
| ← Ceci n'est pas une pipe
Patryk Obara


Re: [PATCH 11/11] sha1_file: convert write_sha1_file to object_id

2018-01-21 Thread Patryk Obara
On 20 January 2018 at 21:44, brian m. carlson
<sand...@crustytoothpaste.net> wrote:
> On Thu, Jan 18, 2018 at 03:51:03PM +0100, Patryk Obara wrote:
>> diff --git a/sha1_file.c b/sha1_file.c
>> index 88b960316c..b7baf69041 100644
>> --- a/sha1_file.c
>> +++ b/sha1_file.c
>> @@ -1420,8 +1420,8 @@ void *read_object_with_reference(const unsigned char 
>> *sha1,
>>  }
>>
>>  static void write_sha1_file_prepare(const void *buf, unsigned long len,
>> -const char *type, unsigned char *sha1,
>> -char *hdr, int *hdrlen)
>> + const char *type, struct object_id *oid,
>> + char *hdr, int *hdrlen)
>
> It looks like the indentation has changed here.  Was that intentional?

Yes. After every commit, I am running clang-format on lines, that I
touch to verify formatting and
decide if I want to include formatting fixes or not. In this case,
function parameters were
aligned using spaces only (which is arguably hard to see in the mail)
- now they are aligned to first
parameter (at least in my editor and in gitk :)).

-- 
| ← Ceci n'est pas une pipe
Patryk Obara


[PATCH 03/11] sha1_file: convert pretend_sha1_file to object_id

2018-01-18 Thread Patryk Obara
Convert the declaration and definition of pretend_sha1_file to use
struct object_id and adjust all usages of this function.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 blame.c | 2 +-
 cache.h | 3 ++-
 sha1_file.c | 8 
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/blame.c b/blame.c
index 2893f3c103..2ad656c1be 100644
--- a/blame.c
+++ b/blame.c
@@ -232,7 +232,7 @@ static struct commit *fake_working_tree_commit(struct 
diff_options *opt,
convert_to_git(_index, path, buf.buf, buf.len, , 0);
origin->file.ptr = buf.buf;
origin->file.size = buf.len;
-   pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_oid.hash);
+   pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, >blob_oid);
 
/*
 * Read the current index, replace the path entry with
diff --git a/cache.h b/cache.h
index d8b975a571..8ed75d7260 100644
--- a/cache.h
+++ b/cache.h
@@ -1241,7 +1241,8 @@ extern int sha1_object_info(const unsigned char *, 
unsigned long *);
 extern int hash_sha1_file(const void *buf, unsigned long len, const char 
*type, unsigned char *sha1);
 extern int write_sha1_file(const void *buf, unsigned long len, const char 
*type, unsigned char *return_sha1);
 extern int hash_sha1_file_literally(const void *buf, unsigned long len, const 
char *type, struct object_id *oid, unsigned flags);
-extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned 
char *);
+extern int pretend_sha1_file(void *, unsigned long, enum object_type,
+struct object_id *oid);
 extern int force_object_loose(const unsigned char *sha1, time_t mtime);
 extern int git_open_cloexec(const char *name, int flags);
 #define git_open(name) git_open_cloexec(name, O_RDONLY)
diff --git a/sha1_file.c b/sha1_file.c
index 3da70ac650..dc8adb9d17 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1313,12 +1313,12 @@ static void *read_object(const unsigned char *sha1, 
enum object_type *type,
 }
 
 int pretend_sha1_file(void *buf, unsigned long len, enum object_type type,
- unsigned char *sha1)
+ struct object_id *oid)
 {
struct cached_object *co;
 
-   hash_sha1_file(buf, len, typename(type), sha1);
-   if (has_sha1_file(sha1) || find_cached_object(sha1))
+   hash_sha1_file(buf, len, typename(type), oid->hash);
+   if (has_sha1_file(oid->hash) || find_cached_object(oid->hash))
return 0;
ALLOC_GROW(cached_objects, cached_object_nr + 1, cached_object_alloc);
co = _objects[cached_object_nr++];
@@ -1326,7 +1326,7 @@ int pretend_sha1_file(void *buf, unsigned long len, enum 
object_type type,
co->type = type;
co->buf = xmalloc(len);
memcpy(co->buf, buf, len);
-   hashcpy(co->sha1, sha1);
+   hashcpy(co->sha1, oid->hash);
return 0;
 }
 
-- 
2.14.3



[PATCH 05/11] sha1_file: convert hash_sha1_file to object_id

2018-01-18 Thread Patryk Obara
Convert the declaration and definition of hash_sha1_file to use
struct object_id and adjust all function calls.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 apply.c  |  4 ++--
 builtin/index-pack.c |  3 +--
 builtin/replace.c|  2 +-
 builtin/unpack-objects.c |  2 +-
 cache-tree.c | 11 +--
 cache.h  |  3 ++-
 convert.c|  6 +++---
 diffcore-rename.c|  2 +-
 dir.c|  2 +-
 log-tree.c   |  2 +-
 sha1_file.c  | 22 +++---
 11 files changed, 29 insertions(+), 30 deletions(-)

diff --git a/apply.c b/apply.c
index 321a9fa68d..d6cbb4dc66 100644
--- a/apply.c
+++ b/apply.c
@@ -3154,7 +3154,7 @@ static int apply_binary(struct apply_state *state,
 * See if the old one matches what the patch
 * applies to.
 */
-   hash_sha1_file(img->buf, img->len, blob_type, oid.hash);
+   hash_sha1_file(img->buf, img->len, blob_type, );
if (strcmp(oid_to_hex(), patch->old_sha1_prefix))
return error(_("the patch applies to '%s' (%s), "
   "which does not match the "
@@ -3199,7 +3199,7 @@ static int apply_binary(struct apply_state *state,
 name);
 
/* verify that the result matches */
-   hash_sha1_file(img->buf, img->len, blob_type, oid.hash);
+   hash_sha1_file(img->buf, img->len, blob_type, );
if (strcmp(oid_to_hex(), patch->new_sha1_prefix))
return error(_("binary patch to '%s' creates incorrect 
result (expecting %s, got %s)"),
name, patch->new_sha1_prefix, oid_to_hex());
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 4c51aec81f..f7c69008f1 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -959,8 +959,7 @@ static void resolve_delta(struct object_entry *delta_obj,
if (!result->data)
bad_object(delta_obj->idx.offset, _("failed to apply delta"));
hash_sha1_file(result->data, result->size,
-  typename(delta_obj->real_type),
-  delta_obj->idx.oid.hash);
+  typename(delta_obj->real_type), _obj->idx.oid);
sha1_object(result->data, NULL, result->size, delta_obj->real_type,
_obj->idx.oid);
counter_lock();
diff --git a/builtin/replace.c b/builtin/replace.c
index 10078ae371..7267d06b17 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -355,7 +355,7 @@ static void check_one_mergetag(struct commit *commit,
struct tag *tag;
int i;
 
-   hash_sha1_file(extra->value, extra->len, typename(OBJ_TAG), 
tag_oid.hash);
+   hash_sha1_file(extra->value, extra->len, typename(OBJ_TAG), _oid);
tag = lookup_tag(_oid);
if (!tag)
die(_("bad mergetag in commit '%s'"), ref);
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 62ea264c46..2e494e2a4b 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -258,7 +258,7 @@ static void write_object(unsigned nr, enum object_type type,
} else {
struct object *obj;
int eaten;
-   hash_sha1_file(buf, size, typename(type), 
obj_list[nr].oid.hash);
+   hash_sha1_file(buf, size, typename(type), _list[nr].oid);
added_object(nr, type, buf, size);
obj = parse_object_buffer(_list[nr].oid, type, size, buf,
  );
diff --git a/cache-tree.c b/cache-tree.c
index e03e72c34a..0df09180ee 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -400,15 +400,14 @@ static int update_one(struct cache_tree *it,
}
 
if (repair) {
-   unsigned char sha1[20];
-   hash_sha1_file(buffer.buf, buffer.len, tree_type, sha1);
-   if (has_sha1_file(sha1))
-   hashcpy(it->oid.hash, sha1);
+   struct object_id oid;
+   hash_sha1_file(buffer.buf, buffer.len, tree_type, );
+   if (has_sha1_file(oid.hash))
+   oidcpy(>oid, );
else
to_invalidate = 1;
} else if (dryrun)
-   hash_sha1_file(buffer.buf, buffer.len, tree_type,
-  it->oid.hash);
+   hash_sha1_file(buffer.buf, buffer.len, tree_type, >oid);
else if (write_sha1_file(buffer.buf, buffer.len, tree_type, 
it->oid.hash)) {
strbuf_release();
return -1;
diff --git a/cache.h b/cache.h
index 04af1d69e4..b953d9f82c 100644
--- a/cache.h
+++ b/cache.h
@@ -1236,7 +1236,8 @@

[PATCH 08/11] commit: convert commit_tree* to object_id

2018-01-18 Thread Patryk Obara
Convert the definitions and declarations of commit_tree and
commit_tree_extended to use struct object_id and adjust all usages of
these functions.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 builtin/am.c  |  4 ++--
 builtin/commit-tree.c |  4 ++--
 builtin/commit.c  |  5 +++--
 builtin/merge.c   |  8 
 commit.c  | 15 +++
 commit.h  | 11 ++-
 notes-cache.c |  4 ++--
 notes-merge.c |  9 -
 notes-utils.c |  7 ---
 notes-utils.h |  3 ++-
 10 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index acfe9d3c8c..6e6abb05cd 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1641,8 +1641,8 @@ static void do_commit(const struct am_state *state)
setenv("GIT_COMMITTER_DATE",
state->ignore_date ? "" : state->author_date, 1);
 
-   if (commit_tree(state->msg, state->msg_len, tree.hash, parents, 
commit.hash,
-   author, state->sign_commit))
+   if (commit_tree(state->msg, state->msg_len, , parents, ,
+   author, state->sign_commit))
die(_("failed to write commit object"));
 
reflog_msg = getenv("GIT_REFLOG_ACTION");
diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
index 2177251e24..e5bdf57b1e 100644
--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c
@@ -117,8 +117,8 @@ int cmd_commit_tree(int argc, const char **argv, const char 
*prefix)
die_errno("git commit-tree: failed to read");
}
 
-   if (commit_tree(buffer.buf, buffer.len, tree_oid.hash, parents,
-   commit_oid.hash, NULL, sign_commit)) {
+   if (commit_tree(buffer.buf, buffer.len, _oid, parents, _oid,
+   NULL, sign_commit)) {
strbuf_release();
return 1;
}
diff --git a/builtin/commit.c b/builtin/commit.c
index 8a87701414..c14878302e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1792,8 +1792,9 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
append_merge_tag_headers(parents, );
}
 
-   if (commit_tree_extended(sb.buf, sb.len, active_cache_tree->oid.hash,
-parents, oid.hash, author_ident.buf, sign_commit, 
extra)) {
+   if (commit_tree_extended(sb.buf, sb.len, _cache_tree->oid,
+parents, , author_ident.buf, sign_commit,
+extra)) {
rollback_index_files();
die(_("failed to write commit object"));
}
diff --git a/builtin/merge.c b/builtin/merge.c
index 30264cfd7c..92ba99a1a5 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -820,8 +820,8 @@ static int merge_trivial(struct commit *head, struct 
commit_list *remoteheads)
pptr = commit_list_append(head, pptr);
pptr = commit_list_append(remoteheads->item, pptr);
prepare_to_commit(remoteheads);
-   if (commit_tree(merge_msg.buf, merge_msg.len, result_tree.hash, parents,
-   result_commit.hash, NULL, sign_commit))
+   if (commit_tree(merge_msg.buf, merge_msg.len, _tree, parents,
+   _commit, NULL, sign_commit))
die(_("failed to write commit object"));
finish(head, remoteheads, _commit, "In-index merge");
drop_save();
@@ -845,8 +845,8 @@ static int finish_automerge(struct commit *head,
commit_list_insert(head, );
strbuf_addch(_msg, '\n');
prepare_to_commit(remoteheads);
-   if (commit_tree(merge_msg.buf, merge_msg.len, result_tree->hash, 
parents,
-   result_commit.hash, NULL, sign_commit))
+   if (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents,
+   _commit, NULL, sign_commit))
die(_("failed to write commit object"));
strbuf_addf(, "Merge made by the '%s' strategy.", wt_strategy);
finish(head, remoteheads, _commit, buf.buf);
diff --git a/commit.c b/commit.c
index cab8d4455b..760137e54b 100644
--- a/commit.c
+++ b/commit.c
@@ -1395,9 +1395,8 @@ void free_commit_extra_headers(struct commit_extra_header 
*extra)
}
 }
 
-int commit_tree(const char *msg, size_t msg_len,
-   const unsigned char *tree,
-   struct commit_list *parents, unsigned char *ret,
+int commit_tree(const char *msg, size_t msg_len, const struct object_id *tree,
+   struct commit_list *parents, struct object_id *ret,
const char *author, const char *sign_commit)
 {
struct commit_extra_header *extra = NULL, **tail = 
@@ -1526,8 +1525,8 @@ N_("Warning: commit message did not conform to UTF-8.\n"
"variable i18

[PATCH 00/11] Some fixes and bunch of object_id conversions

2018-01-18 Thread Patryk Obara
This patch series puts some mundane groundwork for experimentation with a new
hashing algorithm in git-hash-object.

Some time has passed since my last patches, and  I see, that work on new
hashing algorithm progressed nicely since then:

* brian m. carlson implemented vtable for hash algorithm selection and pushed
the repository format front - thanks to him it's now quite easy to
experimentally replace hashing functions and, e.g. do some performance testing.

* _a lot of people_ contributed to the transition plan, and now it's available
in text format in Documentation dir - Thank you all! It's much more readable
this way.

Patch 1 is not directly related to object_id conversions but helped with
debugging t5540, which kept failing on master for me (spoiler: it was Fedora
fault).  It helps with debugging of failing git-push over HTTP in case of
internal server error, so I think it might be worthwhile.

Patch 2 is a small adjustment to .clang-format, which prevents unnecessary
line breaks after function return type.

Patch 6 is a tiny fix in oidclr function.

All other patches are progressive conversions to struct object_id with some
formatting fixes sprinkled in. These should be somewhat uncontroversial, I hope.

Patryk Obara (11):
  http-push: improve error log
  clang-format: adjust penalty for return type line break
  sha1_file: convert pretend_sha1_file to object_id
  dir: convert struct sha1_stat to use object_id
  sha1_file: convert hash_sha1_file to object_id
  cache: clear whole hash buffer with oidclr
  match-trees: convert splice_tree to object_id
  commit: convert commit_tree* to object_id
  notes: convert combine_notes_* to object_id
  notes: convert write_notes_tree to object_id
  sha1_file: convert write_sha1_file to object_id

 .clang-format|  2 +-
 apply.c  | 12 +++
 blame.c  |  2 +-
 builtin/am.c |  4 +--
 builtin/checkout.c   |  3 +-
 builtin/commit-tree.c|  4 +--
 builtin/commit.c |  5 +--
 builtin/index-pack.c |  3 +-
 builtin/merge.c  |  8 ++---
 builtin/mktag.c  |  6 ++--
 builtin/mktree.c | 10 +++---
 builtin/notes.c  |  8 ++---
 builtin/receive-pack.c   | 11 ---
 builtin/replace.c|  4 +--
 builtin/tag.c|  2 +-
 builtin/unpack-objects.c | 11 ---
 cache-tree.c | 13 
 cache.h  | 13 
 commit.c | 13 
 commit.h | 11 ---
 convert.c|  6 ++--
 diffcore-rename.c|  2 +-
 dir.c| 31 +-
 dir.h|  2 +-
 http-push.c  |  4 +++
 log-tree.c   |  2 +-
 match-trees.c| 40 +++
 merge-recursive.c|  5 +--
 notes-cache.c|  8 ++---
 notes-merge.c|  9 +++---
 notes-utils.c|  9 +++---
 notes-utils.h|  3 +-
 notes.c  | 63 ++--
 notes.h  | 29 ++---
 read-cache.c |  6 ++--
 sha1_file.c  | 51 +++--
 t/helper/test-dump-untracked-cache.c |  4 +--
 37 files changed, 217 insertions(+), 202 deletions(-)

-- 
2.14.3



[PATCH 10/11] notes: convert write_notes_tree to object_id

2018-01-18 Thread Patryk Obara
Convert the definition and declaration of write_notes_tree to
struct object_id and adjust usage of this function.

Additionally, improve style of small part of this function, as old
formatting made it hard to understand at glance what this part of
code is doing.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 notes-cache.c |  2 +-
 notes-utils.c |  2 +-
 notes.c   | 16 +---
 notes.h   |  4 ++--
 4 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/notes-cache.c b/notes-cache.c
index d2f87147cc..010ad236cb 100644
--- a/notes-cache.c
+++ b/notes-cache.c
@@ -54,7 +54,7 @@ int notes_cache_write(struct notes_cache *c)
if (!c->tree.dirty)
return 0;
 
-   if (write_notes_tree(>tree, tree_oid.hash))
+   if (write_notes_tree(>tree, _oid))
return -1;
if (commit_tree(c->validity, strlen(c->validity), _oid, NULL,
_oid, NULL, NULL) < 0)
diff --git a/notes-utils.c b/notes-utils.c
index 058c642dac..02407fe2a7 100644
--- a/notes-utils.c
+++ b/notes-utils.c
@@ -12,7 +12,7 @@ void create_notes_commit(struct notes_tree *t, struct 
commit_list *parents,
 
assert(t->initialized);
 
-   if (write_notes_tree(t, tree_oid.hash))
+   if (write_notes_tree(t, _oid))
die("Failed to write notes tree to database");
 
if (!parents) {
diff --git a/notes.c b/notes.c
index 3f4f94507a..09ef1ce33a 100644
--- a/notes.c
+++ b/notes.c
@@ -1123,11 +1123,12 @@ int for_each_note(struct notes_tree *t, int flags, 
each_note_fn fn,
return for_each_note_helper(t, t->root, 0, 0, flags, fn, cb_data);
 }
 
-int write_notes_tree(struct notes_tree *t, unsigned char *result)
+int write_notes_tree(struct notes_tree *t, struct object_id *result)
 {
struct tree_write_stack root;
struct write_each_note_data cb_data;
int ret;
+   int flags;
 
if (!t)
t = _notes_tree;
@@ -1141,12 +1142,13 @@ int write_notes_tree(struct notes_tree *t, unsigned 
char *result)
cb_data.next_non_note = t->first_non_note;
 
/* Write tree objects representing current notes tree */
-   ret = for_each_note(t, FOR_EACH_NOTE_DONT_UNPACK_SUBTREES |
-   FOR_EACH_NOTE_YIELD_SUBTREES,
-   write_each_note, _data) ||
-   write_each_non_note_until(NULL, _data) ||
-   tree_write_stack_finish_subtree() ||
-   write_sha1_file(root.buf.buf, root.buf.len, tree_type, result);
+   flags = FOR_EACH_NOTE_DONT_UNPACK_SUBTREES |
+   FOR_EACH_NOTE_YIELD_SUBTREES;
+   ret = for_each_note(t, flags, write_each_note, _data) ||
+ write_each_non_note_until(NULL, _data) ||
+ tree_write_stack_finish_subtree() ||
+ write_sha1_file(root.buf.buf, root.buf.len, tree_type,
+ result->hash);
strbuf_release();
return ret;
 }
diff --git a/notes.h b/notes.h
index 88da38b5f4..0433f45db5 100644
--- a/notes.h
+++ b/notes.h
@@ -217,7 +217,7 @@ int for_each_note(struct notes_tree *t, int flags, 
each_note_fn fn,
  * Write the given notes_tree structure to the object database
  *
  * Creates a new tree object encapsulating the current state of the given
- * notes_tree, and stores its SHA1 into the 'result' argument.
+ * notes_tree, and stores its object id into the 'result' argument.
  *
  * Returns zero on success, non-zero on failure.
  *
@@ -225,7 +225,7 @@ int for_each_note(struct notes_tree *t, int flags, 
each_note_fn fn,
  * this function has returned zero. Please also remember to create a
  * corresponding commit object, and update the appropriate notes ref.
  */
-int write_notes_tree(struct notes_tree *t, unsigned char *result);
+int write_notes_tree(struct notes_tree *t, struct object_id *result);
 
 /* Flags controlling the operation of prune */
 #define NOTES_PRUNE_VERBOSE 1
-- 
2.14.3



[PATCH 09/11] notes: convert combine_notes_* to object_id

2018-01-18 Thread Patryk Obara
Convert the definition and declarations of combine_notes_* functions
to struct object_id and adjust usage of these functions.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 notes.c | 46 +++---
 notes.h | 25 +++--
 2 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/notes.c b/notes.c
index c7f21fae44..3f4f94507a 100644
--- a/notes.c
+++ b/notes.c
@@ -270,8 +270,8 @@ static int note_tree_insert(struct notes_tree *t, struct 
int_node *tree,
if (!oidcmp(>val_oid, >val_oid))
return 0;
 
-   ret = combine_notes(l->val_oid.hash,
-   entry->val_oid.hash);
+   ret = combine_notes(>val_oid,
+   >val_oid);
if (!ret && is_null_oid(>val_oid))
note_tree_remove(t, tree, n, entry);
free(entry);
@@ -786,8 +786,8 @@ static int prune_notes_helper(const struct object_id 
*object_oid,
return 0;
 }
 
-int combine_notes_concatenate(unsigned char *cur_sha1,
-   const unsigned char *new_sha1)
+int combine_notes_concatenate(struct object_id *cur_oid,
+ const struct object_id *new_oid)
 {
char *cur_msg = NULL, *new_msg = NULL, *buf;
unsigned long cur_len, new_len, buf_len;
@@ -795,18 +795,18 @@ int combine_notes_concatenate(unsigned char *cur_sha1,
int ret;
 
/* read in both note blob objects */
-   if (!is_null_sha1(new_sha1))
-   new_msg = read_sha1_file(new_sha1, _type, _len);
+   if (!is_null_oid(new_oid))
+   new_msg = read_sha1_file(new_oid->hash, _type, _len);
if (!new_msg || !new_len || new_type != OBJ_BLOB) {
free(new_msg);
return 0;
}
-   if (!is_null_sha1(cur_sha1))
-   cur_msg = read_sha1_file(cur_sha1, _type, _len);
+   if (!is_null_oid(cur_oid))
+   cur_msg = read_sha1_file(cur_oid->hash, _type, _len);
if (!cur_msg || !cur_len || cur_type != OBJ_BLOB) {
free(cur_msg);
free(new_msg);
-   hashcpy(cur_sha1, new_sha1);
+   oidcpy(cur_oid, new_oid);
return 0;
}
 
@@ -825,20 +825,20 @@ int combine_notes_concatenate(unsigned char *cur_sha1,
free(new_msg);
 
/* create a new blob object from buf */
-   ret = write_sha1_file(buf, buf_len, blob_type, cur_sha1);
+   ret = write_sha1_file(buf, buf_len, blob_type, cur_oid->hash);
free(buf);
return ret;
 }
 
-int combine_notes_overwrite(unsigned char *cur_sha1,
-   const unsigned char *new_sha1)
+int combine_notes_overwrite(struct object_id *cur_oid,
+   const struct object_id *new_oid)
 {
-   hashcpy(cur_sha1, new_sha1);
+   oidcpy(cur_oid, new_oid);
return 0;
 }
 
-int combine_notes_ignore(unsigned char *cur_sha1,
-   const unsigned char *new_sha1)
+int combine_notes_ignore(struct object_id *cur_oid,
+const struct object_id *new_oid)
 {
return 0;
 }
@@ -848,17 +848,17 @@ int combine_notes_ignore(unsigned char *cur_sha1,
  * newlines removed.
  */
 static int string_list_add_note_lines(struct string_list *list,
- const unsigned char *sha1)
+ const struct object_id *oid)
 {
char *data;
unsigned long len;
enum object_type t;
 
-   if (is_null_sha1(sha1))
+   if (is_null_oid(oid))
return 0;
 
/* read_sha1_file NUL-terminates */
-   data = read_sha1_file(sha1, , );
+   data = read_sha1_file(oid->hash, , );
if (t != OBJ_BLOB || !data || !len) {
free(data);
return t != OBJ_BLOB || !data;
@@ -884,17 +884,17 @@ static int string_list_join_lines_helper(struct 
string_list_item *item,
return 0;
 }
 
-int combine_notes_cat_sort_uniq(unsigned char *cur_sha1,
-   const unsigned char *new_sha1)
+int combine_notes_cat_sort_uniq(struct object_id *cur_oid,
+   const struct object_id *new_oid)
 {
struct string_list sort_uniq_list = STRING_LIST_INIT_DUP;
struct strbuf buf = STRBUF_INIT;
int ret = 1;
 
/* read both note blob objects into unique_lines */
-   if (string_list_add_note_lines(_uniq_list, cur_sha1))
+   if (string_list_add_note_lines(_uniq_list, cur_oid))
goto out;
-   if (string_list_add_note_lines(_uniq_list, new_sha1))
+   if (string_list_add_note_lines(_uniq_list, new_oid))
goto out;
string_list_remove_empty_items(_uni

[PATCH 02/11] clang-format: adjust penalty for return type line break

2018-01-18 Thread Patryk Obara
The penalty of 5 makes clang-format very eager to put even short type
declarations (e.g. "extern int") into a separate line, even when
breaking parameters list is sufficient.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 .clang-format | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.clang-format b/.clang-format
index 611ab4750b..12a89f95f9 100644
--- a/.clang-format
+++ b/.clang-format
@@ -163,7 +163,7 @@ PenaltyBreakComment: 10
 PenaltyBreakFirstLessLess: 0
 PenaltyBreakString: 10
 PenaltyExcessCharacter: 100
-PenaltyReturnTypeOnItsOwnLine: 5
+PenaltyReturnTypeOnItsOwnLine: 60
 
 # Don't sort #include's
 SortIncludes: false
-- 
2.14.3



[PATCH 11/11] sha1_file: convert write_sha1_file to object_id

2018-01-18 Thread Patryk Obara
Convert the definition and declaration of write_sha1_file to
struct object_id and adjust usage of this function.

This commit also converts static function write_sha1_file_prepare, as it
is closely related.

Replace sha1_to_hex, hashcpy and hashclr with their oid equivalents
wherever possible.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 apply.c  |  8 
 builtin/checkout.c   |  3 +--
 builtin/mktag.c  |  6 +++---
 builtin/mktree.c | 10 +-
 builtin/notes.c  |  8 
 builtin/receive-pack.c   | 11 ++-
 builtin/replace.c|  2 +-
 builtin/tag.c|  2 +-
 builtin/unpack-objects.c |  9 ++---
 cache-tree.c |  2 +-
 cache.h  |  3 ++-
 commit.c |  2 +-
 match-trees.c|  2 +-
 merge-recursive.c|  5 +++--
 notes-cache.c|  2 +-
 notes.c  |  9 -
 read-cache.c |  6 +++---
 sha1_file.c  | 25 +
 18 files changed, 60 insertions(+), 55 deletions(-)

diff --git a/apply.c b/apply.c
index d6cbb4dc66..4951f8a130 100644
--- a/apply.c
+++ b/apply.c
@@ -3554,7 +3554,7 @@ static int try_threeway(struct apply_state *state,
 
/* Preimage the patch was prepared for */
if (patch->is_new)
-   write_sha1_file("", 0, blob_type, pre_oid.hash);
+   write_sha1_file("", 0, blob_type, _oid);
else if (get_oid(patch->old_sha1_prefix, _oid) ||
 read_blob_object(, _oid, patch->old_mode))
return error(_("repository lacks the necessary blob to fall 
back on 3-way merge."));
@@ -3570,7 +3570,7 @@ static int try_threeway(struct apply_state *state,
return -1;
}
/* post_oid is theirs */
-   write_sha1_file(tmp_image.buf, tmp_image.len, blob_type, post_oid.hash);
+   write_sha1_file(tmp_image.buf, tmp_image.len, blob_type, _oid);
clear_image(_image);
 
/* our_oid is ours */
@@ -3583,7 +3583,7 @@ static int try_threeway(struct apply_state *state,
return error(_("cannot read the current contents of 
'%s'"),
 patch->old_name);
}
-   write_sha1_file(tmp_image.buf, tmp_image.len, blob_type, our_oid.hash);
+   write_sha1_file(tmp_image.buf, tmp_image.len, blob_type, _oid);
clear_image(_image);
 
/* in-core three-way merge between post and our using pre as base */
@@ -4291,7 +4291,7 @@ static int add_index_file(struct apply_state *state,
}
fill_stat_cache_info(ce, );
}
-   if (write_sha1_file(buf, size, blob_type, ce->oid.hash) < 0) {
+   if (write_sha1_file(buf, size, blob_type, >oid) < 0) {
free(ce);
return error(_("unable to create backing store "
   "for newly created file %s"), path);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 8bdc927d3f..a98e88288e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -227,8 +227,7 @@ static int checkout_merged(int pos, const struct checkout 
*state)
 * (it also writes the merge result to the object database even
 * when it may contain conflicts).
 */
-   if (write_sha1_file(result_buf.ptr, result_buf.size,
-   blob_type, oid.hash))
+   if (write_sha1_file(result_buf.ptr, result_buf.size, blob_type, ))
die(_("Unable to add merge result for '%s'"), path);
free(result_buf.ptr);
ce = make_cache_entry(mode, oid.hash, path, 2, 0);
diff --git a/builtin/mktag.c b/builtin/mktag.c
index 031b750f06..d4044da3e4 100644
--- a/builtin/mktag.c
+++ b/builtin/mktag.c
@@ -151,7 +151,7 @@ static int verify_tag(char *buffer, unsigned long size)
 int cmd_mktag(int argc, const char **argv, const char *prefix)
 {
struct strbuf buf = STRBUF_INIT;
-   unsigned char result_sha1[20];
+   struct object_id result;
 
if (argc != 1)
usage("git mktag");
@@ -165,10 +165,10 @@ int cmd_mktag(int argc, const char **argv, const char 
*prefix)
if (verify_tag(buf.buf, buf.len) < 0)
die("invalid tag signature file");
 
-   if (write_sha1_file(buf.buf, buf.len, tag_type, result_sha1) < 0)
+   if (write_sha1_file(buf.buf, buf.len, tag_type, ) < 0)
die("unable to write tag file");
 
strbuf_release();
-   printf("%s\n", sha1_to_hex(result_sha1));
+   printf("%s\n", oid_to_hex());
return 0;
 }
diff --git a/builtin/mktree.c b/builtin/mktree.c
index da0fd8cd70..1988f5396d 100644
--- a/builtin/mktree.c
+++ b/builtin/mktree.c
@@ -40,7 +40,7 @@ stat

[PATCH 01/11] http-push: improve error log

2018-01-18 Thread Patryk Obara
When git push fails due to server-side WebDAV error, it's not easy to
point to the main culprit.  Additional information about exact cURL
error and HTTP server response is helpful for debugging purpose.

New error log helped me pinpoint failing test t5540-http-push-webdav
to a missing Apache dependency in Fedora 27:
https://bugzilla.redhat.com/show_bug.cgi?id=1491151

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 http-push.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/http-push.c b/http-push.c
index 14435ab65d..0913f8ab86 100644
--- a/http-push.c
+++ b/http-push.c
@@ -915,6 +915,10 @@ static struct remote_lock *lock_remote(const char *path, 
long timeout)
lock->timeout = -1;
}
XML_ParserFree(parser);
+   } else {
+   fprintf(stderr,
+   "error: curl result=%d, HTTP code=%ld\n",
+   results.curl_result, results.http_code);
}
} else {
fprintf(stderr, "Unable to start LOCK request\n");
-- 
2.14.3



[PATCH 04/11] dir: convert struct sha1_stat to use object_id

2018-01-18 Thread Patryk Obara
Convert the declaration of struct sha1_stat. Adjust all usages of this
struct and replace hash{clr,cmp,cpy} with oid{clr,cmp,cpy} wherever
possible.

Remove macro EMPTY_BLOB_SHA1_BIN, as it's no longer used.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 cache.h  |  2 --
 dir.c| 31 ---
 dir.h|  2 +-
 t/helper/test-dump-untracked-cache.c |  4 ++--
 4 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/cache.h b/cache.h
index 8ed75d7260..04af1d69e4 100644
--- a/cache.h
+++ b/cache.h
@@ -1047,8 +1047,6 @@ extern const struct object_id empty_tree_oid;
"\xe6\x9d\xe2\x9b\xb2\xd1\xd6\x43\x4b\x8b" \
"\x29\xae\x77\x5a\xd8\xc2\xe4\x8c\x53\x91"
 extern const struct object_id empty_blob_oid;
-#define EMPTY_BLOB_SHA1_BIN (empty_blob_oid.hash)
-
 
 static inline int is_empty_blob_sha1(const unsigned char *sha1)
 {
diff --git a/dir.c b/dir.c
index 7c4b45e30e..effa531d35 100644
--- a/dir.c
+++ b/dir.c
@@ -253,7 +253,7 @@ static int do_read_blob(const struct object_id *oid,
 
if (sha1_stat) {
memset(_stat->stat, 0, sizeof(sha1_stat->stat));
-   hashcpy(sha1_stat->sha1, oid->hash);
+   oidcpy(_stat->oid, oid);
}
 
if (sz == 0) {
@@ -823,7 +823,7 @@ static int add_excludes(const char *fname, const char 
*base, int baselen,
if (size == 0) {
if (sha1_stat) {
fill_stat_data(_stat->stat, );
-   hashcpy(sha1_stat->sha1, EMPTY_BLOB_SHA1_BIN);
+   oidcpy(_stat->oid, _blob_oid);
sha1_stat->valid = 1;
}
close(fd);
@@ -847,10 +847,11 @@ static int add_excludes(const char *fname, const char 
*base, int baselen,
 !ce_stage(istate->cache[pos]) &&
 ce_uptodate(istate->cache[pos]) &&
 !would_convert_to_git(istate, fname))
-   hashcpy(sha1_stat->sha1,
-   istate->cache[pos]->oid.hash);
+   oidcpy(_stat->oid,
+  >cache[pos]->oid);
else
-   hash_sha1_file(buf, size, "blob", 
sha1_stat->sha1);
+   hash_sha1_file(buf, size, "blob",
+  sha1_stat->oid.hash);
fill_stat_data(_stat->stat, );
sha1_stat->valid = 1;
}
@@ -1223,7 +1224,7 @@ static void prep_exclude(struct dir_struct *dir,
}
 
/* Try to read per-directory file */
-   hashclr(sha1_stat.sha1);
+   oidclr(_stat.oid);
sha1_stat.valid = 0;
if (dir->exclude_per_dir &&
/*
@@ -1269,9 +1270,9 @@ static void prep_exclude(struct dir_struct *dir,
 * order, though, if you do that.
 */
if (untracked &&
-   hashcmp(sha1_stat.sha1, untracked->exclude_sha1)) {
+   hashcmp(sha1_stat.oid.hash, untracked->exclude_sha1)) {
invalidate_gitignore(dir->untracked, untracked);
-   hashcpy(untracked->exclude_sha1, sha1_stat.sha1);
+   hashcpy(untracked->exclude_sha1, sha1_stat.oid.hash);
}
dir->exclude_stack = stk;
current = stk->baselen;
@@ -2228,13 +2229,13 @@ static struct untracked_cache_dir 
*validate_untracked_cache(struct dir_struct *d
 
/* Validate $GIT_DIR/info/exclude and core.excludesfile */
root = dir->untracked->root;
-   if (hashcmp(dir->ss_info_exclude.sha1,
-   dir->untracked->ss_info_exclude.sha1)) {
+   if (oidcmp(>ss_info_exclude.oid,
+  >untracked->ss_info_exclude.oid)) {
invalidate_gitignore(dir->untracked, root);
dir->untracked->ss_info_exclude = dir->ss_info_exclude;
}
-   if (hashcmp(dir->ss_excludes_file.sha1,
-   dir->untracked->ss_excludes_file.sha1)) {
+   if (oidcmp(>ss_excludes_file.oid,
+  >untracked->ss_excludes_file.oid)) {
invalidate_gitignore(dir->untracked, root);
dir->untracked->ss_excludes_file = dir->ss_excludes_file;
}
@@ -2638,8 +2639,8 @@ void write_untracked_extension(struct strbuf *out, struct 
untracked_cache *untra
FLEX_ALLOC_MEM(ouc, ex

[PATCH 06/11] cache: clear whole hash buffer with oidclr

2018-01-18 Thread Patryk Obara
As long as GIT_SHA1_RAWSZ is equal to GIT_MAX_RAWSZ there's no problem,
but when new hashing algorithm will be in place this memset will clear
only 20-byte prefix of hash buffer.

Alternatively, hashclr implementation could be adjusted, but this
function is almost removed from codebase already.  Separate
implementation of oidclr prevents potential buffer overrun in case
someone incorrectly used hashclr on object_id in future.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 cache.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cache.h b/cache.h
index b953d9f82c..cf98573805 100644
--- a/cache.h
+++ b/cache.h
@@ -1029,7 +1029,7 @@ static inline void hashclr(unsigned char *hash)
 
 static inline void oidclr(struct object_id *oid)
 {
-   hashclr(oid->hash);
+   memset(oid->hash, 0, GIT_MAX_RAWSZ);
 }
 
 
-- 
2.14.3



[PATCH 07/11] match-trees: convert splice_tree to object_id

2018-01-18 Thread Patryk Obara
Convert the definition of static recursive splice_tree function to use
struct object_id and adjust single caller.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 match-trees.c | 42 --
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/match-trees.c b/match-trees.c
index 396b7338df..0f899a7212 100644
--- a/match-trees.c
+++ b/match-trees.c
@@ -161,19 +161,17 @@ static void match_trees(const struct object_id *hash1,
  * A tree "hash1" has a subdirectory at "prefix".  Come up with a
  * tree object by replacing it with another tree "hash2".
  */
-static int splice_tree(const unsigned char *hash1,
-  const char *prefix,
-  const unsigned char *hash2,
-  unsigned char *result)
+static int splice_tree(const struct object_id *hash1, const char *prefix,
+  const struct object_id *hash2, struct object_id *result)
 {
char *subpath;
int toplen;
char *buf;
unsigned long sz;
struct tree_desc desc;
-   unsigned char *rewrite_here;
-   const unsigned char *rewrite_with;
-   unsigned char subtree[20];
+   struct object_id *rewrite_here;
+   const struct object_id *rewrite_with;
+   struct object_id subtree;
enum object_type type;
int status;
 
@@ -182,9 +180,9 @@ static int splice_tree(const unsigned char *hash1,
if (*subpath)
subpath++;
 
-   buf = read_sha1_file(hash1, , );
+   buf = read_sha1_file(hash1->hash, , );
if (!buf)
-   die("cannot read tree %s", sha1_to_hex(hash1));
+   die("cannot read tree %s", oid_to_hex(hash1));
init_tree_desc(, buf, sz);
 
rewrite_here = NULL;
@@ -197,26 +195,26 @@ static int splice_tree(const unsigned char *hash1,
if (strlen(name) == toplen &&
!memcmp(name, prefix, toplen)) {
if (!S_ISDIR(mode))
-   die("entry %s in tree %s is not a tree",
-   name, sha1_to_hex(hash1));
-   rewrite_here = (unsigned char *) oid->hash;
+   die("entry %s in tree %s is not a tree", name,
+   oid_to_hex(hash1));
+   rewrite_here = (struct object_id *)oid;
break;
}
update_tree_entry();
}
if (!rewrite_here)
-   die("entry %.*s not found in tree %s",
-   toplen, prefix, sha1_to_hex(hash1));
+   die("entry %.*s not found in tree %s", toplen, prefix,
+   oid_to_hex(hash1));
if (*subpath) {
-   status = splice_tree(rewrite_here, subpath, hash2, subtree);
+   status = splice_tree(rewrite_here, subpath, hash2, );
if (status)
return status;
-   rewrite_with = subtree;
-   }
-   else
+   rewrite_with = 
+   } else {
rewrite_with = hash2;
-   hashcpy(rewrite_here, rewrite_with);
-   status = write_sha1_file(buf, sz, tree_type, result);
+   }
+   oidcpy(rewrite_here, rewrite_with);
+   status = write_sha1_file(buf, sz, tree_type, result->hash);
free(buf);
return status;
 }
@@ -280,7 +278,7 @@ void shift_tree(const struct object_id *hash1,
if (!*add_prefix)
return;
 
-   splice_tree(hash1->hash, add_prefix, hash2->hash, shifted->hash);
+   splice_tree(hash1, add_prefix, hash2, shifted);
 }
 
 /*
@@ -334,7 +332,7 @@ void shift_tree_by(const struct object_id *hash1,
 * shift tree2 down by adding shift_prefix above it
 * to match tree1.
 */
-   splice_tree(hash1->hash, shift_prefix, hash2->hash, 
shifted->hash);
+   splice_tree(hash1, shift_prefix, hash2, shifted);
else
/*
 * shift tree2 up by removing shift_prefix from it
-- 
2.14.3



[PATCH 5/6] sha1_file: convert hash_sha1_file_literally to struct object_id

2017-08-20 Thread Patryk Obara
Convert all remaining callers as well.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 builtin/hash-object.c | 2 +-
 cache.h   | 2 +-
 sha1_file.c   | 8 
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index 8a58ce0..c532ff9 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -24,7 +24,7 @@ static int hash_literally(struct object_id *oid, int fd, 
const char *type, unsig
if (strbuf_read(, fd, 4096) < 0)
ret = -1;
else
-   ret = hash_sha1_file_literally(buf.buf, buf.len, type, 
oid->hash, flags);
+   ret = hash_sha1_file_literally(buf.buf, buf.len, type, oid, 
flags);
strbuf_release();
return ret;
 }
diff --git a/cache.h b/cache.h
index eaf3603..237adb5 100644
--- a/cache.h
+++ b/cache.h
@@ -1199,7 +1199,7 @@ static inline const unsigned char 
*lookup_replace_object(const unsigned char *sh
 extern int sha1_object_info(const unsigned char *, unsigned long *);
 extern int hash_sha1_file(const void *buf, unsigned long len, const char 
*type, unsigned char *sha1);
 extern int write_sha1_file(const void *buf, unsigned long len, const char 
*type, unsigned char *return_sha1);
-extern int hash_sha1_file_literally(const void *buf, unsigned long len, const 
char *type, unsigned char *sha1, unsigned flags);
+extern int hash_sha1_file_literally(const void *buf, unsigned long len, const 
char *type, struct object_id *oid, unsigned flags);
 extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned 
char *);
 extern int force_object_loose(const unsigned char *sha1, time_t mtime);
 extern int git_open_cloexec(const char *name, int flags);
diff --git a/sha1_file.c b/sha1_file.c
index 11995e5..3e2ef4e 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3437,7 +3437,7 @@ int write_sha1_file(const void *buf, unsigned long len, 
const char *type, unsign
 }
 
 int hash_sha1_file_literally(const void *buf, unsigned long len, const char 
*type,
-unsigned char *sha1, unsigned flags)
+ struct object_id *oid, unsigned flags)
 {
char *header;
int hdrlen, status = 0;
@@ -3445,13 +3445,13 @@ int hash_sha1_file_literally(const void *buf, unsigned 
long len, const char *typ
/* type string, SP, %lu of the length plus NUL must fit this */
hdrlen = strlen(type) + 32;
header = xmalloc(hdrlen);
-   write_sha1_file_prepare(buf, len, type, sha1, header, );
+   write_sha1_file_prepare(buf, len, type, oid->hash, header, );
 
if (!(flags & HASH_WRITE_OBJECT))
goto cleanup;
-   if (freshen_packed_object(sha1) || freshen_loose_object(sha1))
+   if (freshen_packed_object(oid->hash) || freshen_loose_object(oid->hash))
goto cleanup;
-   status = write_loose_object(sha1, header, hdrlen, buf, len, 0);
+   status = write_loose_object(oid->hash, header, hdrlen, buf, len, 0);
 
 cleanup:
free(header);
-- 
2.9.5



[PATCH 4/6] sha1_file: convert index_fd to struct object_id

2017-08-20 Thread Patryk Obara
Convert all remaining callers as well.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 builtin/difftool.c|  2 +-
 builtin/hash-object.c |  2 +-
 builtin/replace.c |  2 +-
 cache.h   |  2 +-
 read-cache.c  |  2 +-
 sha1_file.c   | 14 +++---
 6 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/builtin/difftool.c b/builtin/difftool.c
index 8864d84..b2d3ba7 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -111,7 +111,7 @@ static int use_wt_file(const char *workdir, const char 
*name,
int fd = open(buf.buf, O_RDONLY);
 
if (fd >= 0 &&
-   !index_fd(wt_oid.hash, fd, , OBJ_BLOB, name, 0)) {
+   !index_fd(_oid, fd, , OBJ_BLOB, name, 0)) {
if (is_null_oid(oid)) {
oidcpy(oid, _oid);
use = 1;
diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index 1c0f0f3..8a58ce0 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -38,7 +38,7 @@ static void hash_fd(int fd, const char *type, const char 
*path, unsigned flags,
if (fstat(fd, ) < 0 ||
(literally
 ? hash_literally(, fd, type, flags)
-: index_fd(oid.hash, fd, , type_from_string(type), path, 
flags)))
+: index_fd(, fd, , type_from_string(type), path, flags)))
die((flags & HASH_WRITE_OBJECT)
? "Unable to add %s to database"
: "Unable to hash %s", path);
diff --git a/builtin/replace.c b/builtin/replace.c
index f4a85a1..3e71a77 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -269,7 +269,7 @@ static void import_object(struct object_id *oid, enum 
object_type type,
 
if (fstat(fd, ) < 0)
die_errno("unable to fstat %s", filename);
-   if (index_fd(oid->hash, fd, , type, NULL, flags) < 0)
+   if (index_fd(oid, fd, , type, NULL, flags) < 0)
die("unable to write object to database");
/* index_fd close()s fd for us */
}
diff --git a/cache.h b/cache.h
index 380868d..eaf3603 100644
--- a/cache.h
+++ b/cache.h
@@ -684,7 +684,7 @@ extern int ie_modified(const struct index_state *, const 
struct cache_entry *, s
 
 #define HASH_WRITE_OBJECT 1
 #define HASH_FORMAT_CHECK 2
-extern int index_fd(unsigned char *sha1, int fd, struct stat *st, enum 
object_type type, const char *path, unsigned flags);
+extern int index_fd(struct object_id *oid, int fd, struct stat *st, enum 
object_type type, const char *path, unsigned flags);
 extern int index_path(struct object_id *oid, const char *path, struct stat 
*st, unsigned flags);
 
 /*
diff --git a/read-cache.c b/read-cache.c
index 17f19a1..9b41058 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -161,7 +161,7 @@ static int ce_compare_data(const struct cache_entry *ce, 
struct stat *st)
 
if (fd >= 0) {
struct object_id oid;
-   if (!index_fd(oid.hash, fd, st, OBJ_BLOB, ce->name, 0))
+   if (!index_fd(, fd, st, OBJ_BLOB, ce->name, 0))
match = oidcmp(, >oid);
/* index_fd() closed the file descriptor already */
}
diff --git a/sha1_file.c b/sha1_file.c
index 6a2a48b..11995e5 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3662,8 +3662,8 @@ static int index_stream(unsigned char *sha1, int fd, 
size_t size,
return index_bulk_checkin(sha1, fd, size, type, path, flags);
 }
 
-int index_fd(unsigned char *sha1, int fd, struct stat *st,
-enum object_type type, const char *path, unsigned flags)
+int index_fd(struct object_id *oid, int fd, struct stat *st,
+ enum object_type type, const char *path, unsigned flags)
 {
int ret;
 
@@ -3672,15 +3672,15 @@ int index_fd(unsigned char *sha1, int fd, struct stat 
*st,
 * die() for large files.
 */
if (type == OBJ_BLOB && path && would_convert_to_git_filter_fd(path))
-   ret = index_stream_convert_blob(sha1, fd, path, flags);
+   ret = index_stream_convert_blob(oid->hash, fd, path, flags);
else if (!S_ISREG(st->st_mode))
-   ret = index_pipe(sha1, fd, type, path, flags);
+   ret = index_pipe(oid->hash, fd, type, path, flags);
else if (st->st_size <= big_file_threshold || type != OBJ_BLOB ||
 (path && would_convert_to_git(_index, path)))
-   ret = index_core(sha1, fd, xsize_t(st->st_size), type, path,
+   ret = index_core(oid->hash, fd, xsize_t(st->st_size), type, 
path,
 flags);
else
-   ret = index_stream(sha1, fd, xsize_t(st->st_size), type, path,
+   ret = index_stream(oid->has

[PATCH 2/6] read-cache: convert to struct object_id

2017-08-20 Thread Patryk Obara
Replace hashcmp with oidcmp.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 read-cache.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index acfb028..7285608 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -160,9 +160,9 @@ static int ce_compare_data(const struct cache_entry *ce, 
struct stat *st)
int fd = git_open_cloexec(ce->name, O_RDONLY);
 
if (fd >= 0) {
-   unsigned char sha1[20];
-   if (!index_fd(sha1, fd, st, OBJ_BLOB, ce->name, 0))
-   match = hashcmp(sha1, ce->oid.hash);
+   struct object_id oid;
+   if (!index_fd(oid.hash, fd, st, OBJ_BLOB, ce->name, 0))
+   match = oidcmp(, >oid);
/* index_fd() closed the file descriptor already */
}
return match;
-- 
2.9.5



[PATCH 6/6] sha1_file: convert index_stream to struct object_id

2017-08-20 Thread Patryk Obara
Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 sha1_file.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 3e2ef4e..8d6960a 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3655,11 +3655,11 @@ static int index_core(unsigned char *sha1, int fd, 
size_t size,
  * binary blobs, they generally do not want to get any conversion, and
  * callers should avoid this code path when filters are requested.
  */
-static int index_stream(unsigned char *sha1, int fd, size_t size,
+static int index_stream(struct object_id *oid, int fd, size_t size,
enum object_type type, const char *path,
unsigned flags)
 {
-   return index_bulk_checkin(sha1, fd, size, type, path, flags);
+   return index_bulk_checkin(oid->hash, fd, size, type, path, flags);
 }
 
 int index_fd(struct object_id *oid, int fd, struct stat *st,
@@ -3680,7 +3680,7 @@ int index_fd(struct object_id *oid, int fd, struct stat 
*st,
ret = index_core(oid->hash, fd, xsize_t(st->st_size), type, 
path,
 flags);
else
-   ret = index_stream(oid->hash, fd, xsize_t(st->st_size), type, 
path,
+   ret = index_stream(oid, fd, xsize_t(st->st_size), type, path,
   flags);
close(fd);
return ret;
-- 
2.9.5



[PATCH 3/6] sha1_file: convert index_path to struct object_id

2017-08-20 Thread Patryk Obara
Convert all remaining callers as well.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 builtin/update-index.c |  2 +-
 cache.h|  2 +-
 diff.c |  2 +-
 notes-merge.c  |  2 +-
 read-cache.c   |  2 +-
 sha1_file.c| 10 +-
 6 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 56721cf..d562f2e 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -280,7 +280,7 @@ static int add_one_path(const struct cache_entry *old, 
const char *path, int len
fill_stat_cache_info(ce, st);
ce->ce_mode = ce_mode_from_stat(old, st->st_mode);
 
-   if (index_path(ce->oid.hash, path, st,
+   if (index_path(>oid, path, st,
   info_only ? 0 : HASH_WRITE_OBJECT)) {
free(ce);
return -1;
diff --git a/cache.h b/cache.h
index 1c69d2a..380868d 100644
--- a/cache.h
+++ b/cache.h
@@ -685,7 +685,7 @@ extern int ie_modified(const struct index_state *, const 
struct cache_entry *, s
 #define HASH_WRITE_OBJECT 1
 #define HASH_FORMAT_CHECK 2
 extern int index_fd(unsigned char *sha1, int fd, struct stat *st, enum 
object_type type, const char *path, unsigned flags);
-extern int index_path(unsigned char *sha1, const char *path, struct stat *st, 
unsigned flags);
+extern int index_path(struct object_id *oid, const char *path, struct stat 
*st, unsigned flags);
 
 /*
  * Record to sd the data from st that we use to check whether a file
diff --git a/diff.c b/diff.c
index 9c38258..65f8d13 100644
--- a/diff.c
+++ b/diff.c
@@ -3246,7 +3246,7 @@ static void diff_fill_oid_info(struct diff_filespec *one)
}
if (lstat(one->path, ) < 0)
die_errno("stat '%s'", one->path);
-   if (index_path(one->oid.hash, one->path, , 0))
+   if (index_path(>oid, one->path, , 0))
die("cannot hash %s", one->path);
}
}
diff --git a/notes-merge.c b/notes-merge.c
index c12b354..744c685 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -709,7 +709,7 @@ int notes_merge_commit(struct notes_merge_options *o,
/* write file as blob, and add to partial_tree */
if (stat(path.buf, ))
die_errno("Failed to stat '%s'", path.buf);
-   if (index_path(blob_oid.hash, path.buf, , HASH_WRITE_OBJECT))
+   if (index_path(_oid, path.buf, , HASH_WRITE_OBJECT))
die("Failed to write blob object from '%s'", path.buf);
if (add_note(partial_tree, _oid, _oid, NULL))
die("Failed to add resolved note '%s' to notes tree",
diff --git a/read-cache.c b/read-cache.c
index 7285608..17f19a1 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -689,7 +689,7 @@ int add_to_index(struct index_state *istate, const char 
*path, struct stat *st,
return 0;
}
if (!intent_only) {
-   if (index_path(ce->oid.hash, path, st, HASH_WRITE_OBJECT)) {
+   if (index_path(>oid, path, st, HASH_WRITE_OBJECT)) {
free(ce);
return error("unable to index file %s", path);
}
diff --git a/sha1_file.c b/sha1_file.c
index b60ae15..6a2a48b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3686,7 +3686,7 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st,
return ret;
 }
 
-int index_path(unsigned char *sha1, const char *path, struct stat *st, 
unsigned flags)
+int index_path(struct object_id *oid, const char *path, struct stat *st, 
unsigned flags)
 {
int fd;
struct strbuf sb = STRBUF_INIT;
@@ -3696,7 +3696,7 @@ int index_path(unsigned char *sha1, const char *path, 
struct stat *st, unsigned
fd = open(path, O_RDONLY);
if (fd < 0)
return error_errno("open(\"%s\")", path);
-   if (index_fd(sha1, fd, st, OBJ_BLOB, path, flags) < 0)
+   if (index_fd(oid->hash, fd, st, OBJ_BLOB, path, flags) < 0)
return error("%s: failed to insert into database",
 path);
break;
@@ -3704,14 +3704,14 @@ int index_path(unsigned char *sha1, const char *path, 
struct stat *st, unsigned
if (strbuf_readlink(, path, st->st_size))
return error_errno("readlink(\"%s\")", path);
if (!(flags & HASH_WRITE_OBJECT))
-   hash_sha1_file(sb.buf, sb.len, blob_type, sha1);
-   else if (write_sha1_file(sb.buf, sb.len, blob_type, sha1))
+   hash_sha1_file(sb.buf, sb.len

[PATCH 1/6] builtin/hash-object: convert to struct object_id

2017-08-20 Thread Patryk Obara
Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 builtin/hash-object.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index d04baf9..1c0f0f3 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -16,7 +16,7 @@
  * needs to bypass the data conversion performed by, and the type
  * limitation imposed by, index_fd() and its callees.
  */
-static int hash_literally(unsigned char *sha1, int fd, const char *type, 
unsigned flags)
+static int hash_literally(struct object_id *oid, int fd, const char *type, 
unsigned flags)
 {
struct strbuf buf = STRBUF_INIT;
int ret;
@@ -24,7 +24,7 @@ static int hash_literally(unsigned char *sha1, int fd, const 
char *type, unsigne
if (strbuf_read(, fd, 4096) < 0)
ret = -1;
else
-   ret = hash_sha1_file_literally(buf.buf, buf.len, type, sha1, 
flags);
+   ret = hash_sha1_file_literally(buf.buf, buf.len, type, 
oid->hash, flags);
strbuf_release();
return ret;
 }
@@ -33,16 +33,16 @@ static void hash_fd(int fd, const char *type, const char 
*path, unsigned flags,
int literally)
 {
struct stat st;
-   unsigned char sha1[20];
+   struct object_id oid;
 
if (fstat(fd, ) < 0 ||
(literally
-? hash_literally(sha1, fd, type, flags)
-: index_fd(sha1, fd, , type_from_string(type), path, flags)))
+? hash_literally(, fd, type, flags)
+: index_fd(oid.hash, fd, , type_from_string(type), path, 
flags)))
die((flags & HASH_WRITE_OBJECT)
? "Unable to add %s to database"
: "Unable to hash %s", path);
-   printf("%s\n", sha1_to_hex(sha1));
+   printf("%s\n", oid_to_hex());
maybe_flush_or_die(stdout, "hash to stdout");
 }
 
-- 
2.9.5



[PATCH 0/6] Convert hash-object to struct object_id

2017-08-20 Thread Patryk Obara
This enabled conversion of few functions in sha1_file, which
had almost all callers converted already.

I hope I'm not stepping on anyone's toes with this patch series.
If I do - is there some email thread or document in which I can
coordinate with other developers, regarding which code regions
are being converted to struct object_id next?

I focused on this builtin in particular because it's probably
the smallest functionality, that can be converted to different
hashing algorithm, at least partly.

Patryk Obara (6):
  builtin/hash-object: convert to struct object_id
  read-cache: convert to struct object_id
  sha1_file: convert index_path to struct object_id
  sha1_file: convert index_fd to struct object_id
  sha1_file: convert hash_sha1_file_literally to struct object_id
  sha1_file: convert index_stream to struct object_id

 builtin/difftool.c |  2 +-
 builtin/hash-object.c  | 12 ++--
 builtin/replace.c  |  2 +-
 builtin/update-index.c |  2 +-
 cache.h|  6 +++---
 diff.c |  2 +-
 notes-merge.c  |  2 +-
 read-cache.c   |  8 
 sha1_file.c| 34 +-
 9 files changed, 35 insertions(+), 35 deletions(-)

-- 
2.9.5



Re: Please fix the useless email prompts

2017-08-19 Thread Patryk Obara
Why you can't just set username as name and username@hostname as mail?
You'll do it once and it will be preserved for future. If you use
various accounts for testing, use --system flag for config to store
the values in /etc. If you don't want to modify the environment, use
--local (or no flag) to preserve name in your cloned repository only.

-- 
| ← Ceci n'est pas une pipe
Patryk Obara


Re: [PATCH v4 4/4] commit: rewrite read_graft_line

2017-08-18 Thread Patryk Obara
Ok, so that's an option - in this instance free is not actually needed
because it can be triggered only in phase 0, but it would add a bit of
robustness.

-- 
| ← Ceci n'est pas une pipe
Patryk Obara


Re: [PATCH v4 4/4] commit: rewrite read_graft_line

2017-08-18 Thread Patryk Obara
Actually, I don't think I needed to remove free(graft) line, but I don't
know if freeing NULL is considered ok in git code. Let me know if I
should bring it back, please.

-- 
| ← Ceci n'est pas une pipe
Patryk Obara


[PATCH v4 0/4] Modernize read_graft_line implementation

2017-08-18 Thread Patryk Obara
Changes since v3:

- Commit replacing raw buffer does not store temporary pointer to
  strbuf internals any more.

- Commit message of patch 4 explains all alternative approaches
  considered so far.

- Patch 4 uses two-phases to parse graft line, without code repetition.

I have my reservations about patch 4 from readability standpoint
(it's not immediately clear why parsing code can skip freeing of graft
in phase 2), but this implementation seems to address every issue raised
in review so far. If you'll prefer me to go back to impementation
from v3, I have it prepared ;)


Patryk Obara (4):
  sha1_file: fix definition of null_sha1
  commit: replace the raw buffer with strbuf in read_graft_line
  commit: allocate array using object_id size
  commit: rewrite read_graft_line

 builtin/blame.c |  2 +-
 commit.c| 45 +
 commit.h|  2 +-
 sha1_file.c |  2 +-
 4 files changed, 28 insertions(+), 23 deletions(-)

-- 
2.9.5



[PATCH v4 4/4] commit: rewrite read_graft_line

2017-08-18 Thread Patryk Obara
Old implementation determined number of hashes by dividing length of
line by length of hash, which works only if all hash representations
have same length.

New graft line parser works in two phases:

  1. In first phase line is scanned to verify correctness and compute
 number of hashes, then graft struct is allocated.

  2. In second phase line is scanned again to fill up already allocated
 graft struct.

This way graft parsing code can support different sizes of hashes
without any further code adaptations.

A number of alternative implementations were considered and discarded:

  - Modifying graft structure to store oid_array instead of FLEXI_ARRAY
indicates undesirable usage of struct to readers.

  - Parsing into temporary string_list or oid_array complicates code
by adding more return paths, as these structures needs to be
cleared before returning from function.

  - Determining number of hashes by counting separators might cause
maintenance issues, if this function needs to be modified in future
again.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 commit.c | 35 ---
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/commit.c b/commit.c
index 436eb34..3eefd9d 100644
--- a/commit.c
+++ b/commit.c
@@ -137,32 +137,37 @@ int register_commit_graft(struct commit_graft *graft, int 
ignore_dups)
 struct commit_graft *read_graft_line(struct strbuf *line)
 {
/* The format is just "Commit Parent1 Parent2 ...\n" */
-   int i;
+   int i, phase;
+   const char *tail = NULL;
struct commit_graft *graft = NULL;
-   const int entry_size = GIT_SHA1_HEXSZ + 1;
+   struct object_id dummy_oid, *oid;
 
strbuf_rtrim(line);
if (!line->len || line->buf[0] == '#')
return NULL;
-   if ((line->len + 1) % entry_size)
-   goto bad_graft_data;
-   i = (line->len + 1) / entry_size - 1;
-   graft = xmalloc(st_add(sizeof(*graft),
-  st_mult(sizeof(struct object_id), i)));
-   graft->nr_parent = i;
-   if (get_oid_hex(line->buf, >oid))
-   goto bad_graft_data;
-   for (i = GIT_SHA1_HEXSZ; i < line->len; i += entry_size) {
-   if (line->buf[i] != ' ')
-   goto bad_graft_data;
-   if (get_sha1_hex(line->buf + i + 1, 
graft->parent[i/entry_size].hash))
+   /*
+* phase 0 verifies line, counts hashes in line and allocates graft
+* phase 1 fills graft
+*/
+   for (phase = 0; phase < 2; phase++) {
+   oid = graft ? >oid : _oid;
+   if (parse_oid_hex(line->buf, oid, ))
goto bad_graft_data;
+   for (i = 0; *tail != '\0'; i++) {
+   oid = graft ? >parent[i] : _oid;
+   if (!isspace(*tail++) || parse_oid_hex(tail, oid, 
))
+   goto bad_graft_data;
+   }
+   if (!graft) {
+   graft = xmalloc(st_add(sizeof(*graft),
+  st_mult(sizeof(struct 
object_id), i)));
+   graft->nr_parent = i;
+   }
}
return graft;
 
 bad_graft_data:
error("bad graft data: %s", line->buf);
-   free(graft);
return NULL;
 }
 
-- 
2.9.5



[PATCH v4 1/4] sha1_file: fix definition of null_sha1

2017-08-18 Thread Patryk Obara
The array is declared in cache.h as:

  extern const unsigned char null_sha1[GIT_MAX_RAWSZ];

Definition in sha1_file.c must match.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 sha1_file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sha1_file.c b/sha1_file.c
index b60ae15..f5b5bec 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -32,7 +32,7 @@
 #define SZ_FMT PRIuMAX
 static inline uintmax_t sz_fmt(size_t s) { return s; }
 
-const unsigned char null_sha1[20];
+const unsigned char null_sha1[GIT_MAX_RAWSZ];
 const struct object_id null_oid;
 const struct object_id empty_tree_oid = {
EMPTY_TREE_SHA1_BIN_LITERAL
-- 
2.9.5



[PATCH v4 3/4] commit: allocate array using object_id size

2017-08-18 Thread Patryk Obara
struct commit_graft aggregates an array of object_id's, which have
size >= GIT_MAX_RAWSZ bytes. This change prevents memory allocation
error when size of object_id is larger than GIT_SHA1_RAWSZ.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 commit.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/commit.c b/commit.c
index 1a0a9f2..436eb34 100644
--- a/commit.c
+++ b/commit.c
@@ -147,7 +147,8 @@ struct commit_graft *read_graft_line(struct strbuf *line)
if ((line->len + 1) % entry_size)
goto bad_graft_data;
i = (line->len + 1) / entry_size - 1;
-   graft = xmalloc(st_add(sizeof(*graft), st_mult(GIT_SHA1_RAWSZ, i)));
+   graft = xmalloc(st_add(sizeof(*graft),
+  st_mult(sizeof(struct object_id), i)));
graft->nr_parent = i;
if (get_oid_hex(line->buf, >oid))
goto bad_graft_data;
-- 
2.9.5



[PATCH v4 2/4] commit: replace the raw buffer with strbuf in read_graft_line

2017-08-18 Thread Patryk Obara
This simplifies function declaration and allows for use of strbuf_rtrim
instead of modifying buffer directly.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 builtin/blame.c |  2 +-
 commit.c| 23 +++
 commit.h|  2 +-
 3 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index bda1a78..d4472e9 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -488,7 +488,7 @@ static int read_ancestry(const char *graft_file)
return -1;
while (!strbuf_getwholeline(, fp, '\n')) {
/* The format is just "Commit Parent1 Parent2 ...\n" */
-   struct commit_graft *graft = read_graft_line(buf.buf, buf.len);
+   struct commit_graft *graft = read_graft_line();
if (graft)
register_commit_graft(graft, 0);
}
diff --git a/commit.c b/commit.c
index 8b28415..1a0a9f2 100644
--- a/commit.c
+++ b/commit.c
@@ -134,34 +134,33 @@ int register_commit_graft(struct commit_graft *graft, int 
ignore_dups)
return 0;
 }
 
-struct commit_graft *read_graft_line(char *buf, int len)
+struct commit_graft *read_graft_line(struct strbuf *line)
 {
/* The format is just "Commit Parent1 Parent2 ...\n" */
int i;
struct commit_graft *graft = NULL;
const int entry_size = GIT_SHA1_HEXSZ + 1;
 
-   while (len && isspace(buf[len-1]))
-   buf[--len] = '\0';
-   if (buf[0] == '#' || buf[0] == '\0')
+   strbuf_rtrim(line);
+   if (!line->len || line->buf[0] == '#')
return NULL;
-   if ((len + 1) % entry_size)
+   if ((line->len + 1) % entry_size)
goto bad_graft_data;
-   i = (len + 1) / entry_size - 1;
+   i = (line->len + 1) / entry_size - 1;
graft = xmalloc(st_add(sizeof(*graft), st_mult(GIT_SHA1_RAWSZ, i)));
graft->nr_parent = i;
-   if (get_oid_hex(buf, >oid))
+   if (get_oid_hex(line->buf, >oid))
goto bad_graft_data;
-   for (i = GIT_SHA1_HEXSZ; i < len; i += entry_size) {
-   if (buf[i] != ' ')
+   for (i = GIT_SHA1_HEXSZ; i < line->len; i += entry_size) {
+   if (line->buf[i] != ' ')
goto bad_graft_data;
-   if (get_sha1_hex(buf + i + 1, graft->parent[i/entry_size].hash))
+   if (get_sha1_hex(line->buf + i + 1, 
graft->parent[i/entry_size].hash))
goto bad_graft_data;
}
return graft;
 
 bad_graft_data:
-   error("bad graft data: %s", buf);
+   error("bad graft data: %s", line->buf);
free(graft);
return NULL;
 }
@@ -174,7 +173,7 @@ static int read_graft_file(const char *graft_file)
return -1;
while (!strbuf_getwholeline(, fp, '\n')) {
/* The format is just "Commit Parent1 Parent2 ...\n" */
-   struct commit_graft *graft = read_graft_line(buf.buf, buf.len);
+   struct commit_graft *graft = read_graft_line();
if (!graft)
continue;
if (register_commit_graft(graft, 1))
diff --git a/commit.h b/commit.h
index 6d857f0..baecc0a 100644
--- a/commit.h
+++ b/commit.h
@@ -247,7 +247,7 @@ struct commit_graft {
 };
 typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);
 
-struct commit_graft *read_graft_line(char *buf, int len);
+struct commit_graft *read_graft_line(struct strbuf *line);
 int register_commit_graft(struct commit_graft *, int);
 struct commit_graft *lookup_commit_graft(const struct object_id *oid);
 
-- 
2.9.5



Re: [PATCH v3 4/4] commit: rewrite read_graft_line

2017-08-18 Thread Patryk Obara
Ah! I presumed two separate loops, one throwing away oids and second
one actually filling a table - this makes more sense. I was just about
to send v4, but will rewrite the last patch and we'll see how it looks
like.
-- 
| ← Ceci n'est pas une pipe
Patryk Obara


Re: [PATCH v3 4/4] commit: rewrite read_graft_line

2017-08-18 Thread Patryk Obara
Jeff King <p...@peff.net> wrote:
>
> So we're probably fine. The two parsing passes are right next to each
> other and are sufficiently simple and strict that we don't have to
> worry about them diverging.

That was my conclusion as well. I added comment before the first pass and
avoided any "cleverness" to make it perfectly clear to a reader.

> We'd reject such an input totally (though as an interesting side effect,
> you can convince the parser to allocate 20x as much RAM as you send it;
> one oid for each space).

Grafts are not populated during clone operation, so it really would be user
making his life miserable. I could allocate FLEXI_ARRAY of size
min(n, line->len / (GIT_*MIN*_HEXSZ+1)) instead… but I think it's not even
worth the cost of making the code more complicated (and I don't want
to reintroduce these size macros in here.

We _could_ put an artificial limit on graft parents, though (e.g. 10) and
display an error message urging the user to stop using grafts?

> The single-pass alternative would probably be to read into a dynamic
> structure like an oid_array, and then copy the result into the flex
> structure.

Before sending v3 I tried two other alternative implementations (perhaps I
should've listed them in the v3 cover letter):

  1. Using string_list_split_in_place. I resigned from this approach as soon
 as I noticed, that line->buf needs to be preserved for possible
 error message. string_list_split would have no benefits over using
 oid_array.

  2. Parsing into temporary oid_array and then copying memory to FLEXI_ARRAY.
 Throw-away oid_array still needs to be cleaned, which means we have
 new/different return path (one before xmalloc and one after xmalloc),
 which means "bad_graft_data" label needs to be changed into "cleanup"
 label (or removed), which means error description needs be conditionally
 put in earlier code… and at this point, I decided these changes are not
 making code cleaner nor more readable at all :)

Junio C Hamano <gits...@pobox.com> wrote:
>
> If I were doing the two-pass thing, I'd probably write a for loop
> that runs exactly twice, where the first iteration parses into a
> single throw-away oid struct only to count, and the second iteration
> parses the same input into the allocated array of oid struct.  That
> way, you do not have to worry about two phrases going out of sync.

Two passes would still differ in error handling due to xmalloc between them…

-- 
| ← Ceci n'est pas une pipe
Patryk Obara


Re: [PATCH v3 2/4] commit: replace the raw buffer with strbuf in read_graft_line

2017-08-18 Thread Patryk Obara
Jeff King <p...@peff.net> wrote:

> AFAICT this is only here to avoid having to s/buf/line->buf/ in the rest
> of the function. But I think we should just make that change (you
> already did in some of the spots). And IMHO we should do the same for
> line->len. When there are two names for the same value, it increases the
> chances of a bug where the two end up diverging.

My motivation was rather to keep patch(es) as small as possible because every
line using buf will be replaced in a later patch in series. But it will make
commit better (it will stand on its own), so why not to do it? :)

> (…) I think short-circuiting like:
>
>   if (!line->len || line->buf[0] == '#')
>
> is better (I also think "!" instead of "== 0" is our usual style, but
> that's much less important).

Ah, I only replaced comparison to NULL terminator with length check because
I thought it better shows intention of the code and I didn't notice, that
reversing order will result in better code overall.

I will include both changes in v4.

-- 
| ← Ceci n'est pas une pipe
Patryk Obara


[PATCH v3 3/4] commit: allocate array using object_id size

2017-08-17 Thread Patryk Obara
struct commit_graft aggregates an array of object_id's, which have
size >= GIT_MAX_RAWSZ bytes. This change prevents memory allocation
error when size of object_id is larger than GIT_SHA1_RAWSZ.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 commit.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/commit.c b/commit.c
index 019e733..61528a5 100644
--- a/commit.c
+++ b/commit.c
@@ -149,7 +149,8 @@ struct commit_graft *read_graft_line(struct strbuf *line)
if ((len + 1) % entry_size)
goto bad_graft_data;
i = (len + 1) / entry_size - 1;
-   graft = xmalloc(st_add(sizeof(*graft), st_mult(GIT_SHA1_RAWSZ, i)));
+   graft = xmalloc(st_add(sizeof(*graft),
+  st_mult(sizeof(struct object_id), i)));
graft->nr_parent = i;
if (get_oid_hex(buf, >oid))
goto bad_graft_data;
-- 
2.9.5



[PATCH v3 4/4] commit: rewrite read_graft_line

2017-08-17 Thread Patryk Obara
Determine the number of object_id's to parse in a single graft line by
counting separators (whitespace characters) instead of dividing by
length of hash representation.

This way graft parsing code can support different sizes of hashes
without any further code adaptations.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 commit.c | 28 +---
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/commit.c b/commit.c
index 61528a5..46ee2db 100644
--- a/commit.c
+++ b/commit.c
@@ -137,29 +137,27 @@ int register_commit_graft(struct commit_graft *graft, int 
ignore_dups)
 struct commit_graft *read_graft_line(struct strbuf *line)
 {
/* The format is just "Commit Parent1 Parent2 ...\n" */
-   int i, len;
-   char *buf = line->buf;
+   int i, n;
+   const char *tail = NULL;
struct commit_graft *graft = NULL;
-   const int entry_size = GIT_SHA1_HEXSZ + 1;
 
strbuf_rtrim(line);
if (line->buf[0] == '#' || line->len == 0)
return NULL;
-   len = line->len;
-   if ((len + 1) % entry_size)
-   goto bad_graft_data;
-   i = (len + 1) / entry_size - 1;
+   /* count number of blanks to determine size of array to allocate */
+   for (i = 0, n = 0; i < line->len; i++)
+   if (isspace(line->buf[i]))
+   n++;
graft = xmalloc(st_add(sizeof(*graft),
-  st_mult(sizeof(struct object_id), i)));
-   graft->nr_parent = i;
-   if (get_oid_hex(buf, >oid))
+  st_mult(sizeof(struct object_id), n)));
+   graft->nr_parent = n;
+   if (parse_oid_hex(line->buf, >oid, ))
goto bad_graft_data;
-   for (i = GIT_SHA1_HEXSZ; i < len; i += entry_size) {
-   if (buf[i] != ' ')
-   goto bad_graft_data;
-   if (get_sha1_hex(buf + i + 1, graft->parent[i/entry_size].hash))
+   for (i = 0; i < graft->nr_parent; i++)
+   if (!isspace(*tail++) || parse_oid_hex(tail, >parent[i], 
))
goto bad_graft_data;
-   }
+   if (tail[0] != '\0')
+   goto bad_graft_data;
return graft;
 
 bad_graft_data:
-- 
2.9.5



[PATCH v3 1/4] sha1_file: fix definition of null_sha1

2017-08-17 Thread Patryk Obara
The array is declared in cache.h as:

  extern const unsigned char null_sha1[GIT_MAX_RAWSZ];

Definition in sha1_file.c must match.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 sha1_file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sha1_file.c b/sha1_file.c
index b60ae15..f5b5bec 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -32,7 +32,7 @@
 #define SZ_FMT PRIuMAX
 static inline uintmax_t sz_fmt(size_t s) { return s; }
 
-const unsigned char null_sha1[20];
+const unsigned char null_sha1[GIT_MAX_RAWSZ];
 const struct object_id null_oid;
 const struct object_id empty_tree_oid = {
EMPTY_TREE_SHA1_BIN_LITERAL
-- 
2.9.5



[PATCH v3 2/4] commit: replace the raw buffer with strbuf in read_graft_line

2017-08-17 Thread Patryk Obara
This simplifies function declaration and allows for use of strbuf_rtrim
instead of modifying buffer directly.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 builtin/blame.c |  2 +-
 commit.c| 15 ---
 commit.h|  2 +-
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index bda1a78..d4472e9 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -488,7 +488,7 @@ static int read_ancestry(const char *graft_file)
return -1;
while (!strbuf_getwholeline(, fp, '\n')) {
/* The format is just "Commit Parent1 Parent2 ...\n" */
-   struct commit_graft *graft = read_graft_line(buf.buf, buf.len);
+   struct commit_graft *graft = read_graft_line();
if (graft)
register_commit_graft(graft, 0);
}
diff --git a/commit.c b/commit.c
index 8b28415..019e733 100644
--- a/commit.c
+++ b/commit.c
@@ -134,17 +134,18 @@ int register_commit_graft(struct commit_graft *graft, int 
ignore_dups)
return 0;
 }
 
-struct commit_graft *read_graft_line(char *buf, int len)
+struct commit_graft *read_graft_line(struct strbuf *line)
 {
/* The format is just "Commit Parent1 Parent2 ...\n" */
-   int i;
+   int i, len;
+   char *buf = line->buf;
struct commit_graft *graft = NULL;
const int entry_size = GIT_SHA1_HEXSZ + 1;
 
-   while (len && isspace(buf[len-1]))
-   buf[--len] = '\0';
-   if (buf[0] == '#' || buf[0] == '\0')
+   strbuf_rtrim(line);
+   if (line->buf[0] == '#' || line->len == 0)
return NULL;
+   len = line->len;
if ((len + 1) % entry_size)
goto bad_graft_data;
i = (len + 1) / entry_size - 1;
@@ -161,7 +162,7 @@ struct commit_graft *read_graft_line(char *buf, int len)
return graft;
 
 bad_graft_data:
-   error("bad graft data: %s", buf);
+   error("bad graft data: %s", line->buf);
free(graft);
return NULL;
 }
@@ -174,7 +175,7 @@ static int read_graft_file(const char *graft_file)
return -1;
while (!strbuf_getwholeline(, fp, '\n')) {
/* The format is just "Commit Parent1 Parent2 ...\n" */
-   struct commit_graft *graft = read_graft_line(buf.buf, buf.len);
+   struct commit_graft *graft = read_graft_line();
if (!graft)
continue;
if (register_commit_graft(graft, 1))
diff --git a/commit.h b/commit.h
index 6d857f0..baecc0a 100644
--- a/commit.h
+++ b/commit.h
@@ -247,7 +247,7 @@ struct commit_graft {
 };
 typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);
 
-struct commit_graft *read_graft_line(char *buf, int len);
+struct commit_graft *read_graft_line(struct strbuf *line);
 int register_commit_graft(struct commit_graft *, int);
 struct commit_graft *lookup_commit_graft(const struct object_id *oid);
 
-- 
2.9.5



[PATCH v3 0/4] Modernize read_graft_line implementation

2017-08-17 Thread Patryk Obara
Changes since v2:
- commit implementing free_graft dropped (no longer needed)
- several more lines moved from last commit to commit replacing
  raw buffer with strbuf
- fix for memory allocation separated from change in hash
  parsing
- commit_graft struct uses FLEX_ARRAY again, meaning free_graft,
  memset, nor oid_array were not needed after all

Patryk Obara (4):
  sha1_file: fix definition of null_sha1
  commit: replace the raw buffer with strbuf in read_graft_line
  commit: allocate array using object_id size
  commit: rewrite read_graft_line

 builtin/blame.c |  2 +-
 commit.c| 38 +++---
 commit.h|  2 +-
 sha1_file.c |  2 +-
 4 files changed, 22 insertions(+), 22 deletions(-)

-- 
2.9.5

base-commit: b3622a4ee94e4916cd05e6d96e41eeb36b941182


Re: [PATCH v2 4/4] commit: rewrite read_graft_line

2017-08-17 Thread Patryk Obara
Understood :) - yes, oid_array is not completely necessary here - and it
gives the wrong impression about usage of this struct.

FLEX_ARRAY will be brought back in v3.

On Thu, Aug 17, 2017 at 11:20 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Patryk Obara <patryk.ob...@gmail.com> writes:
>
>> The previous implementation of read_graft_line used calculations based
>> on GIT_SHA1_RAWSZ and GIT_SHA1_HEXSZ to determine the number of commit
>> ids in a single graft line.  New implementation does not depend on these
>> constants, so it adapts to any object_id buffer size.
>>
>> To make this possible, FLEX_ARRAY of object_id in struct was replaced
>> by an oid_array.
>
> There is a leap in logic between the two paragraphs.  Your use of
> parse_oid_hex() is good.  But I do not think moving the array body
> to outside commit_graft structure and forcing it to be separately
> allocated is necessary or beneficial.  When we got a single line, we
> know how many fake parents a child described by that graft line has,
> and you can still use of FLEX_ARRAY to avoid separate allocation
> and need for separate freeing of it.



-- 
| ← Ceci n'est pas une pipe
Patryk Obara


Re: [PATCH 3/5] commit: replace the raw buffer with strbuf in read_graft_line

2017-08-17 Thread Patryk Obara
> In fact, the former is already how we represent the list of fake
> parents in the commit_graft structure, so I think patch 5/5 in this
> series does two unrelated things, one of which is bad (i.e. use of
> parse_oid_hex() is good; turning the FLEX_ARRAY at the end into a
> oid_array that requires a separate allocation of the array is bad).

Agreed; I already split patch 5 into two separate changes (one fixing
memory allocation issue, one parsing object_ids into FLEX_ARRAY,
without modifying graft struct). In result patch 4 (free_graft) can
be dropped. I will send these changes as v3.


On Thu, Aug 17, 2017 at 11:17 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Jeff King <p...@peff.net> writes:
>
>> I'd expect most of the GIT_MAX constants to eventually go away in favor
>> of "struct object_id", but that will still be using the same "big enough
>> to hold any hash" size under the hood.
>
> Indeed.  It is good to see major contributors are in agreement ;-)
> I'd expect that an array of "struct object_id" would be how a fixed
> number of object names would be represented, i.e.
>
> struct object_id thing[num_elements];
>
> instead of an array of uchar that is MAX bytes long, i.e.
>
> unsigned char name[GIT_MAX_RAWSZ][num_elements];
>
> In fact, the former is already how we represent the list of fake
> parents in the commit_graft structure, so I think patch 5/5 in this
> series does two unrelated things, one of which is bad (i.e. use of
> parse_oid_hex() is good; turning the FLEX_ARRAY at the end into a
> oid_array that requires a separate allocation of the array is bad).
>
>> Agreed. Most code should be dealing with the abstract concept of a hash
>> and shouldn't have to care about the size. I really like parse_oid_hex()
>> for that reason (and I think parsing is the main place we've found that
>> needs to care).
>
> Yes.



-- 
| ← Ceci n'est pas une pipe
Patryk Obara


[PATCH v2 2/4] commit: replace the raw buffer with strbuf in read_graft_line

2017-08-16 Thread Patryk Obara
This simplifies function declaration and allows for use of strbuf_rtrim
instead of modifying buffer directly.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 builtin/blame.c |  2 +-
 commit.c| 11 ++-
 commit.h|  2 +-
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index bda1a78..d4472e9 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -488,7 +488,7 @@ static int read_ancestry(const char *graft_file)
return -1;
while (!strbuf_getwholeline(, fp, '\n')) {
/* The format is just "Commit Parent1 Parent2 ...\n" */
-   struct commit_graft *graft = read_graft_line(buf.buf, buf.len);
+   struct commit_graft *graft = read_graft_line();
if (graft)
register_commit_graft(graft, 0);
}
diff --git a/commit.c b/commit.c
index 8b28415..499fb14 100644
--- a/commit.c
+++ b/commit.c
@@ -134,15 +134,16 @@ int register_commit_graft(struct commit_graft *graft, int 
ignore_dups)
return 0;
 }
 
-struct commit_graft *read_graft_line(char *buf, int len)
+struct commit_graft *read_graft_line(struct strbuf *line)
 {
/* The format is just "Commit Parent1 Parent2 ...\n" */
-   int i;
+   int i, len;
+   char *buf = line->buf;
struct commit_graft *graft = NULL;
const int entry_size = GIT_SHA1_HEXSZ + 1;
 
-   while (len && isspace(buf[len-1]))
-   buf[--len] = '\0';
+   strbuf_rtrim(line);
+   len = line->len;
if (buf[0] == '#' || buf[0] == '\0')
return NULL;
if ((len + 1) % entry_size)
@@ -174,7 +175,7 @@ static int read_graft_file(const char *graft_file)
return -1;
while (!strbuf_getwholeline(, fp, '\n')) {
/* The format is just "Commit Parent1 Parent2 ...\n" */
-   struct commit_graft *graft = read_graft_line(buf.buf, buf.len);
+   struct commit_graft *graft = read_graft_line();
if (!graft)
continue;
if (register_commit_graft(graft, 1))
diff --git a/commit.h b/commit.h
index 6d857f0..baecc0a 100644
--- a/commit.h
+++ b/commit.h
@@ -247,7 +247,7 @@ struct commit_graft {
 };
 typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);
 
-struct commit_graft *read_graft_line(char *buf, int len);
+struct commit_graft *read_graft_line(struct strbuf *line);
 int register_commit_graft(struct commit_graft *, int);
 struct commit_graft *lookup_commit_graft(const struct object_id *oid);
 
-- 
2.9.5



[PATCH v2 4/4] commit: rewrite read_graft_line

2017-08-16 Thread Patryk Obara
The previous implementation of read_graft_line used calculations based
on GIT_SHA1_RAWSZ and GIT_SHA1_HEXSZ to determine the number of commit
ids in a single graft line.  New implementation does not depend on these
constants, so it adapts to any object_id buffer size.

To make this possible, FLEX_ARRAY of object_id in struct was replaced
by an oid_array.

Code allocating graft now needs to use memset to zero the memory before
use to start with oid_array in a consistent state.

Updates free_graft function implemented in the previous patch to
properly cleanup an oid_array storing parents.

Signed-off-by: Patryk Obara <patryk.ob...@gmail.com>
---
 commit.c  | 39 +--
 commit.h  |  2 +-
 shallow.c |  1 +
 3 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/commit.c b/commit.c
index 4d23e72..8bdce36 100644
--- a/commit.c
+++ b/commit.c
@@ -111,6 +111,7 @@ static int commit_graft_pos(const unsigned char *sha1)
 
 static void free_commit_graft(struct commit_graft *graft)
 {
+   oid_array_clear(>parents);
free(graft);
 }
 
@@ -139,35 +140,37 @@ int register_commit_graft(struct commit_graft *graft, int 
ignore_dups)
return 0;
 }
 
+static int parse_next_oid_hex(const char *buf, struct object_id *oid, const 
char **end)
+{
+   while (isspace(buf[0]))
+   buf++;
+   return parse_oid_hex(buf, oid, end);
+}
+
 struct commit_graft *read_graft_line(struct strbuf *line)
 {
/* The format is just "Commit Parent1 Parent2 ...\n" */
-   int i, len;
-   char *buf = line->buf;
struct commit_graft *graft = NULL;
-   const int entry_size = GIT_SHA1_HEXSZ + 1;
+   struct object_id oid;
+   const char *tail = NULL;
 
strbuf_rtrim(line);
-   len = line->len;
-   if (buf[0] == '#' || buf[0] == '\0')
+   if (line->buf[0] == '#' || line->len == 0)
return NULL;
-   if ((len + 1) % entry_size)
+   graft = xmalloc(sizeof(*graft));
+   memset(graft, 0, sizeof(*graft));
+   if (parse_oid_hex(line->buf, >oid, ))
goto bad_graft_data;
-   i = (len + 1) / entry_size - 1;
-   graft = xmalloc(st_add(sizeof(*graft), st_mult(GIT_SHA1_RAWSZ, i)));
-   graft->nr_parent = i;
-   if (get_oid_hex(buf, >oid))
+   while (!parse_next_oid_hex(tail, , ))
+   oid_array_append(>parents, );
+   if (tail[0] != '\0')
goto bad_graft_data;
-   for (i = GIT_SHA1_HEXSZ; i < len; i += entry_size) {
-   if (buf[i] != ' ')
-   goto bad_graft_data;
-   if (get_sha1_hex(buf + i + 1, graft->parent[i/entry_size].hash))
-   goto bad_graft_data;
-   }
+   graft->nr_parent = graft->parents.nr;
+
return graft;
 
 bad_graft_data:
-   error("bad graft data: %s", buf);
+   error("bad graft data: %s", line->buf);
free_commit_graft(graft);
return NULL;
 }
@@ -363,7 +366,7 @@ int parse_commit_buffer(struct commit *item, const void 
*buffer, unsigned long s
int i;
struct commit *new_parent;
for (i = 0; i < graft->nr_parent; i++) {
-   new_parent = lookup_commit(>parent[i]);
+   new_parent = lookup_commit(>parents.oid[i]);
if (!new_parent)
continue;
pptr = _list_insert(new_parent, pptr)->next;
diff --git a/commit.h b/commit.h
index baecc0a..96ff375 100644
--- a/commit.h
+++ b/commit.h
@@ -243,7 +243,7 @@ void sort_in_topological_order(struct commit_list **, enum 
rev_sort_order);
 struct commit_graft {
struct object_id oid;
int nr_parent; /* < 0 if shallow commit */
-   struct object_id parent[FLEX_ARRAY]; /* more */
+   struct oid_array parents;
 };
 typedef int (*each_commit_graft_fn)(const struct commit_graft *, void *);
 
diff --git a/shallow.c b/shallow.c
index f5591e5..892cd90 100644
--- a/shallow.c
+++ b/shallow.c
@@ -33,6 +33,7 @@ int register_shallow(const struct object_id *oid)
xmalloc(sizeof(struct commit_graft));
struct commit *commit = lookup_commit(oid);
 
+   memset(graft, 0, sizeof(*graft));
oidcpy(>oid, oid);
graft->nr_parent = -1;
if (commit && commit->object.parsed)
-- 
2.9.5



  1   2   >