Re: [PATCH v3 06/24] multi-pack-index: load into memory

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/9/2018 3:08 PM, Junio C Hamano wrote:
Derrick Stolee <stolee@xxxxxxxxx> writes:
diff --git a/midx.h b/midx.h
index dbdbe9f873..2d83dd9ec1 100644
--- a/midx.h
+++ b/midx.h
@@ -1,6 +1,10 @@
  #ifndef __MIDX_H__
  #define __MIDX_H__
+struct multi_pack_index;
I actually was quite surprised that this struct is defined in
object-store.h and not here.  It feels the other way around.

The raw_object_store needs to know that such an in-core structure
might exist as an optional feature in an object store, but as an
optional feature, I suspect that it has a pointer to an instance of
multi_pack_index, instead of embedding the struct itself in it, so I
would have expected to see an "I am only telling you that there is a
struct with this name, but I am leaving it opaque as you do not have
any business looking inside the struct yourself.  You only need to
be aware of the type's existence and a pointer to it so that you can
call helpers that know what's inside and that should be sufficient
for your needs." decl like this in object-store.h and instead an
actual implementation in here.

I thought it natural to include the struct definition next to the definition for 'struct packed_git', but I like your separation of concerns. Perhaps we could move the packed_git definition to packfile.h as well (separately). Of course, that sounds like an unnecessary churn.

Thanks,

-Stolee




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux