Re: [PATCH v1 2/5] Teach git to optionally utilize a file system monitor to speed up detecting new or changed files.

From: Ben Peart <peartben@gmail.com>
Date: May 16, 2017, 01:55 UTC
Message-ID: <2d965a87-36da-23b4-4bc5-97de47f3d7f7@gmail.com>
In-Reply-To: <20170516003414.yliltu5fsaudfhyu@sigill.intra.peff.net>

On 5/15/2017 8:34 PM, Jeff King wrote:

Show 23 quoted lines

> On Tue, May 16, 2017 at 12:22:14AM +0000, brian m. carlson wrote:
>
>> On Mon, May 15, 2017 at 03:13:44PM -0400, Ben Peart wrote:
>>> +	istate->last_update = (time_t)ntohll(*(uint64_t *)index);
>>> +	index += sizeof(uint64_t);
>>> +
>>> +	ewah_size = ntohl(*(uint32_t *)index);
>>> +	index += sizeof(uint32_t);
>>
>> To answer the question you asked in your cover letter, you cannot write
>> this unless you can guarantee (((uintptr_t)index & 7) == 0) is true.
>> Otherwise, this will produce a SIGBUS on SPARC, Alpha, MIPS, and some
>> ARM systems, and it will perform poorly on PowerPC and other ARM
>> systems[0].
>>
>> If you got that pointer from malloc and have only indexed multiples of 8
>> on it, you're good.  But if you're not sure, you probably want to use
>> memcpy.  If the compiler can determine that it's not necessary, it will
>> omit the copy and perform a direct load.
>
> I think get_be32() does exactly what we want for the ewah_size read. For
> the last_update one, we don't have a get_be64() yet, but it should be
> easy to make based on the 16/32 versions.

Thanks for the pointers. I'll update this to use the existing get_be32 and have created a get_be64 and will use that for the last_update.

>
> (I note also that time_t is not necessarily 64-bits in the first place,
> but David said something about this not really being a time_t).
>

The in memory representation is a time_t as that is the return value of time(NULL) but it is converted to/from a 64 bit value when written/read to the index extension so that the index format is the same no matter the native size of time_t.

> -Peff
>