Re: [BUG] git diff --no-index segfaults on large files (NULL object database)

[Jeff King <peff@peff.net>]

On Sun, Apr 05, 2026 at 01:07:27AM +0800, Tian Yuchen wrote:

> On 4/5/26 00:53, Luca Stefani wrote:
> > Thanks for looking into it.
> > Locally, I simply check against null storage and it works just fine,
> > flags is always 0 in my experiments so a check against
> > INDEX_WRITE_OBJECT also worked.
> > 
> > diff --git a/object-file.c b/object-file.c
> > index f0b029ff0b..68303aa99c 100644
> > --- a/object-file.c
> > +++ b/object-file.c
> > @@ -1654,7 +1654,8 @@ int index_fd(struct index_state *istate, struct
> > object_id *oid,
> >    } else if ((st->st_size >= 0 &&
> >        (size_t)st->st_size <=
> > repo_settings_get_big_file_threshold(istate->repo)) ||
> >       type != OBJ_BLOB ||
> > -    (path && would_convert_to_git(istate, path))) {
> > +    (path && would_convert_to_git(istate, path)) ||
> > +    !(flags & INDEX_WRITE_OBJECT)) {
> >    ret = index_core(istate, oid, fd, xsize_t(st->st_size),
> >    type, path, flags);
> >    } else {
> > 
> > Luca.
> 
> That looks good, almost exactly what I was about to send. I was mistaken—
> there isn’t a hash_write_object flag after all ;-)
> 
> It looks like this is your first time posting on the Git mailing list. Would
> you consider contributing this (as a patch)?s

Alternatively, should the odb transaction system be more forgiving here,
and act as a noop when there is no odb?

Bisecting the segfault yields ce1661f9da (odb: add transaction
interface, 2025-09-16). Before then, we passed around the
object_database itself, saw that its transaction field was NULL, and
returned immediately. After that commit, we pass the object_databse to
odb_transaction_begin(), which narrows it to odb->sources (which is
NULL) while passing to object_file_transaction_begin(). And then that
function looks at source->odb to go back to the object_database! But
the source being NULL, it segfaults.

Immediately after that commit, the switch from taking an odb to a source
is not helpful, though I think eventually it is used to set
transaction->base.source. But should the whole thing check for a NULL
source and return early? Or otherwise establish some kind of noop
transaction?

I haven't thought about the implications (nor even really looked at odb
transaction code before). But doing it that way would fix not only this
bug, but also other potential bugs throughout the code base when callers
start a noop transaction.

+cc Justin (author of ce1661f9da) for any thoughts.

-Peff