Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > Yeah, I think that's even better, although... [snip] > > It's also kind of weird that map_loose_object_1() is a noop on a > > negative descriptor. That technically makes this correct, but I think it > > would be much less surprising to always take a valid descriptor, and > > this code should do: > > > > if (fd) > > return -1; > > return map_loose_object_1(r, path, fd, size); > > > > If we are going to make map_loose_object_1() less confusing (and I think > > that is worth doing), let's go all the way. > > ...maybe we should go further in the other direction. I.e. with my > earlier suggestion we're left with the mess that the "fd" ownership > isn't clear. With Peff's suggestion I think we can make the function always close the fd. If we find it not to be clear, we can rename the function to ..._close_fd() or something like that. Thanks to both of you for your suggestions.
