Junio C Hamano <gitster@xxxxxxxxx> writes:
> "Eric W. Biederman" <ebiederm@xxxxxxxxx> writes:
>
>> Did you have any manual merge conflicts you had to resolve?
>> If so it is possible to see the merge result you had?
>
> The only merge-fix I had to apply to make everything compile was
> this:
>
> diff --git a/bloom.c b/bloom.c
> index ff131893cd..59eb0a0481 100644
> --- a/bloom.c
> +++ b/bloom.c
> @@ -278,7 +278,7 @@ static int has_entries_with_high_bit(struct repository *r, struct tree *t)
> struct tree_desc desc;
> struct name_entry entry;
>
> - init_tree_desc(&desc, t->buffer, t->size);
> + init_tree_desc(&desc, &t->object.oid, t->buffer, t->size);
> while (tree_entry(&desc, &entry)) {
> size_t i;
> for (i = 0; i < entry.pathlen; i++) {
>
> as one topic changed the function signature while the other topic
> added a new callsite.
>
> Everything else was pretty-much auto resolved, I think.
>
> Output from "git show --cc seen" matches my recollection. The above
> does appear as an evil merge.
Thanks, and I found all of this on your seen branch.
After tracking all of these down it appears all of the errors
came from my branch, I will be resending the patches as soon
as I finish going through the review comments.
Looking at the build errors pretty much all of the all of the
automatic test failures came from commit_tree_extended.
There was a strbuf that did
strbuf_init(&buf, 8192);
strbuf_init(&buf, 8192);
twice.
Plus there was another buffer that was allocated and not freed,
in commit_tree_extended.
The leaks were a bit tricky to track down as building with SANITIZE=leak
causes tests to fail somewhat randomly for me with "gcc (Debian
12.2.0-14) 12.2.0".
There was one smatch static-analysis error that suggested using
CALLOC_ARRAY instead of xcalloc.
The "win" build and "linux-gcc-default (ubuntu-lastest)" build failed
because of an over eager gcc warning -Werror=array-bounds.
Claiming:
In file included from /usr/include/string.h:535,
from git-compat-util.h:228,
from commit.c:1:
In function ‘memcpy’,
inlined from ‘oidcpy’ at hash-ll.h:272:2,
inlined from ‘commit_tree_extended’ at commit.c:1705:3:
##[error]/usr/include/x86_64-linux-gnu/bits/string_fortified.h:29:10: ‘__builtin_memcpy’ offset [0, 31] is out of the bounds [0, 0] [-Werror=array-bounds]
29 | return __builtin___memcpy_chk (__dest, __src, __len,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
30 | __glibc_objsize0 (__dest));
| ~~~~~~~~~~~~~~~~~~~~~~~~~~
Previously in commit_tree_extended the structure was:
while (parents) {
struct commit *parent = pop_commit(&parents);
strbuf_addf(&buffer, "parent %s\n",
oid_to_hex(&parent->object.oid));
}
brian had changed it to:
nparents = commit_list_count(parents);
parent_buf = xcalloc(nparents, sizeof(*parent_buf));
for (i = 0; i < nparents; i++) {
struct commit *parent = pop_commit(&parents);
oidcpy(&parent_buf[i], &parent->object.oid);
}
Which is perfectly sound code.
I changed the structure of the loop to:
nparents = commit_list_count(parents);
parent_buf = xcalloc(nparents, sizeof(*parent_buf));
i = 0;
while (parents) {
struct commit *parent = pop_commit(&parents);
oidcpy(&parent_buf[i++], &parent->object.oid);
}
And the "array-bounds" warning had no problems with the code.
So it looks like the error was actually that array-bounds thought
there was a potential NULL pointer dereference at which point
it would not have array bounds, and then it complained about
the array bounds, instead of the NULL pointer dereference.
I am going to fix the patch. If array-bounds causes further
problems you may want to think about disabling it.
Eric
