Hi, On Fri, 18 Sep 2020, Junio C Hamano wrote: > Thomas Guyot-Sionnest <tguyot@xxxxxxxxx> writes: > > >> > - same_contents = oideq(&one->oid, &two->oid); > >> > + if (one->is_stdin && two->is_stdin) > >> > + same_contents = !strcmp(one->data, two->data); > >> > + else > >> > + same_contents = oideq(&one->oid, &two->oid); > >> > >> ...should this actually be checking the oid_valid flag in each filespec? > >> That would presumably cover the is_stdin case, too. I also wonder > >> whether range-diff ought to be using that flag instead of is_stdin. > > > > I considered that, but IIRC when run under a debugger oid_valid was > > set to 0 - it seemed to be used for something different that i'm not > > familiar with, maybe it's an indication the object is in git datastore > > (whereas with --no-index outside files will only be hashed for > > comparison). > > If it says !oid_valid, I think you are getting what you do want. I suspect the same. > The contents from the outside world, be it what was read from the > standard input or a pipe, a regular file that is not up-to-date with > the index, may not have a usable oid computed for it, and oid_valid > being false signals you that you need byte-for-byte comparison. As > suggested by Peff in another message, you can take that signal and > compare the size and then the contents with memcmp() to see if they > are the same. To complete the information: `struct diff_filespec`'s first attribute is `oid`, the object ID of the data. If it is left uninitialized (as is the case in `range-diff`'s case), `oid_valid` has to be 0 to prevent it from being used. I believe that that is exactly the reason why we want this: - same_contents = oideq(&one->oid, &two->oid); + same_contents = one->oid_valid && two->oid_valid ? oideq(&one->oid, &two->oid) : !strcmp(one->data, two->data); Ciao, Dscho
