# [PATCH 2/2] Use fixed-size integers for .idx file I/O 6 messages from 2007-01-18 to 2007-01-18. Participants: Junio C Hamano, Morten Welinder, Simon 'corecode' Schubert, Johannes Schindelin, Shawn O. Pearce. Thread: https://gitlist.dev/t/6420 ## Junio C Hamano, 2007-01-18 07:24 Subject: [PATCH 2/2] Use fixed-size integers for .idx file I/O Message-ID: <7vd55col04.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vd55col04.fsf%40assigned-by-dhcp.cox.net ``` This attempts to finish what Simon started in the previous commit. Signed-off-by: Junio C Hamano --- * I do not think this should have any platform dependent impact as long as 1/2 is Ok, but for completeness's sake here it is. builtin-pack-objects.c | 6 +++--- cache.h | 2 +- sha1_file.c | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index 42dd8c8..3824ee3 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -569,7 +569,7 @@ static void write_index_file(void) sha1_to_hex(object_list_sha1), "idx"); struct object_entry **list = sorted_by_sha; struct object_entry **last = list + nr_result; - unsigned int array[256]; + uint32_t array[256]; /* * Write the first-level table (the list is sorted, @@ -587,7 +587,7 @@ static void write_index_file(void) array[i] = htonl(next - sorted_by_sha); list = next; } - sha1write(f, array, 256 * sizeof(int)); + sha1write(f, array, 256 * 4); /* * Write the actual SHA1 entries.. @@ -595,7 +595,7 @@ static void write_index_file(void) list = sorted_by_sha; for (i = 0; i < nr_result; i++) { struct object_entry *entry = *list++; - unsigned int offset = htonl(entry->offset); + uint32_t offset = htonl(entry->offset); sha1write(f, &offset, 4); sha1write(f, entry->sha1, 20); } diff --git a/cache.h b/cache.h index fda3f8e..e6e19bd 100644 --- a/cache.h +++ b/cache.h @@ -351,7 +351,7 @@ struct pack_window { extern struct packed_git { struct packed_git *next; struct pack_window *windows; - unsigned int *index_base; + uint32_t *index_base; off_t index_size; off_t pack_size; int pack_fd; diff --git a/sha1_file.c b/sha1_file.c index 0b70545..3025440 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -435,7 +435,7 @@ static int check_packed_git_idx(const char *path, unsigned long *idx_size_, void **idx_map_) { void *idx_map; - unsigned int *index; + uint32_t *index; unsigned long idx_size; int nr, i; int fd = open(path, O_RDONLY); @@ -1351,7 +1351,7 @@ int nth_packed_object_sha1(const struct packed_git *p, int n, unsigned long find_pack_entry_one(const unsigned char *sha1, struct packed_git *p) { - unsigned int *level1_ofs = p->index_base; + uint32_t *level1_ofs = p->index_base; int hi = ntohl(level1_ofs[*sha1]); int lo = ((*sha1 == 0x0) ? 0 : ntohl(level1_ofs[*sha1 - 1])); void *index = p->index_base + 256; @@ -1360,7 +1360,7 @@ unsigned long find_pack_entry_one(const unsigned char *sha1, int mi = (lo + hi) / 2; int cmp = hashcmp((unsigned char *)index + (24 * mi) + 4, sha1); if (!cmp) - return ntohl(*((unsigned int *) ((char *) index + (24 * mi)))); + return ntohl(*((uint32_t *)((char *)index + (24 * mi)))); if (cmp > 0) hi = mi; else -- 1.5.0.rc1.g05cf8 ``` ## Morten Welinder, 2007-01-18 14:51 Subject: Re: [PATCH 2/2] Use fixed-size integers for .idx file I/O Message-ID: <118833cc0701180651w3b5ac164m4e396399f1d58cb7@mail.gmail.com> URL: https://gitlist.dev/e/118833cc0701180651w3b5ac164m4e396399f1d58cb7%40mail.gmail.com In-Reply-To: <7vd55col04.fsf@assigned-by-dhcp.cox.net> ``` > - return ntohl(*((unsigned int *) ((char *) index + (24 * mi)))); > + return ntohl(*((uint32_t *)((char *)index + (24 * mi)))); Is that pointer gymnastics guaranteed to work? I.e., how do we know that we can access an uint32_t (or unsigned) at such an address? M. ``` ## Simon 'corecode' Schubert, 2007-01-18 14:56 Subject: Re: [PATCH 2/2] Use fixed-size integers for .idx file I/O Message-ID: <45AF8A86.7070101@fs.ei.tum.de> URL: https://gitlist.dev/e/45AF8A86.7070101%40fs.ei.tum.de In-Reply-To: <118833cc0701180651w3b5ac164m4e396399f1d58cb7@mail.gmail.com> ``` Morten Welinder wrote: >> - return ntohl(*((unsigned int *) ((char *) >> index + (24 * mi)))); >> + return ntohl(*((uint32_t *)((char *)index + >> (24 * mi)))); > > Is that pointer gymnastics guaranteed to work? I.e., how do we know > that we can access an uint32_t (or unsigned) at such an address? if index is always aligned to a 4-byte boundary, this is safe. apart from that, the problem already existed. cheers simon -- Serve - BSD +++ RENT this banner advert +++ ASCII Ribbon /"\ Work - Mac +++ space for low €€€ NOW!1 +++ Campaign \ / Party Enjoy Relax | http://dragonflybsd.org Against HTML \ Dude 2c 2 the max ! http://golden-apple.biz Mail + News / \ ``` ## Johannes Schindelin, 2007-01-18 15:15 Subject: Re: [PATCH 2/2] Use fixed-size integers for .idx file I/O Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0701181612440.22628%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <45AF8A86.7070101@fs.ei.tum.de> ``` Hi, On Thu, 18 Jan 2007, Simon 'corecode' Schubert wrote: > Morten Welinder wrote: > > > - return ntohl(*((unsigned int *) ((char *) index + > > > (24 * mi)))); > > > + return ntohl(*((uint32_t *)((char *)index + (24 * > > > mi)))); > > > > Is that pointer gymnastics guaranteed to work? I.e., how do we know > > that we can access an uint32_t (or unsigned) at such an address? > > if index is always aligned to a 4-byte boundary, this is safe. apart from > that, the problem already existed. index is assigned from p->index_base, which comes from check_packed_git_idx(), and there it comes from an xmmap(). AFAICT mmap() (and for NO_MMAP, malloc()) _always_ return aligned pointers, so there is no problem, as long as the alignment step divides 24. Ciao, Dscho ``` ## Shawn O. Pearce, 2007-01-18 15:21 Subject: Re: [PATCH 2/2] Use fixed-size integers for .idx file I/O Message-ID: <20070118152107.GA15428@spearce.org> URL: https://gitlist.dev/e/20070118152107.GA15428%40spearce.org In-Reply-To: <45AF8A86.7070101@fs.ei.tum.de> ``` Simon 'corecode' Schubert wrote: > Morten Welinder wrote: > >>- return ntohl(*((unsigned int *) ((char *) > >>index + (24 * mi)))); > >>+ return ntohl(*((uint32_t *)((char *)index + > >>(24 * mi)))); > > > >Is that pointer gymnastics guaranteed to work? I.e., how do we know > >that we can access an uint32_t (or unsigned) at such an address? > > if index is always aligned to a 4-byte boundary, this is safe. apart from > that, the problem already existed. Its always 4-byte aligned here. The index is mmap()'d as one huge chunk so the first byte of the index is page aligned. The index starts out with 256 4-byte words, then is composed of 24 byte units (20 byte SHA1, 4 byte offset). So no worries with the current file format, or code. Yes, we're taking the leap of faith that any currently-used processor will work on 32 bit unsigned integers if they are 4 byte aligned in memory. Maybe someone has a processor that this isn't true for, but a lot of other software would probably break on that same system... -- Shawn. ``` ## Johannes Schindelin, 2007-01-18 15:30 Subject: Re: [PATCH 2/2] Use fixed-size integers for .idx file I/O Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0701181630070.22628%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <20070118152107.GA15428@spearce.org> ``` Hi, On Thu, 18 Jan 2007, Shawn O. Pearce wrote: > Simon 'corecode' Schubert wrote: > > Morten Welinder wrote: > > >>- return ntohl(*((unsigned int *) ((char *) > > >>index + (24 * mi)))); > > >>+ return ntohl(*((uint32_t *)((char *)index + > > >>(24 * mi)))); > > > > > >Is that pointer gymnastics guaranteed to work? I.e., how do we know > > >that we can access an uint32_t (or unsigned) at such an address? > > > > if index is always aligned to a 4-byte boundary, this is safe. apart from > > that, the problem already existed. > > Its always 4-byte aligned here. The index is mmap()'d as one huge > chunk so the first byte of the index is page aligned. The index > starts out with 256 4-byte words, then is composed of 24 byte units > (20 byte SHA1, 4 byte offset). So no worries with the current file > format, or code. > > Yes, we're taking the leap of faith that any currently-used processor > will work on 32 bit unsigned integers if they are 4 byte aligned in > memory. Actually, it's even valid in 8 byte aligned mode. (24 is divisible by 8.) Ciao, Dscho ```