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

On Wed, May 17, 2017 at 08:15:03PM +0200, Johannes Sixt wrote:

Am 17.05.2017 um 16:26 schrieb Ben Peart:

quoted

On 5/16/2017 3:13 PM, Johannes Sixt wrote:

quoted

Am 16.05.2017 um 19:17 schrieb Ben Peart:

quoted

OK, now I'm confused as to the best path for adding a get_be64.  This
one is trivial:

#define get_be64(p)    ntohll(*(uint64_t *)(p))

I cringe when I see a cast like this. Unless you can guarantee that p is
char* (bare or signed or unsigned), you fall pray to strict aliasing
violations, aka undefined behavior. And I'm not even mentioning correct
alignment, yet.

Note, this macro is only used where the CPU architecture is OK with
unaligned memory access.

I'm not worried about the unaligned memory access: It either works, or we
get a SIGBUS. The undefined behavior is more worrisome because the code may
work or not, and we can never be sure which it is.

I don't think there's much we can do, though. That's how all of the
get_be* macros are designed to work (and there's really no point in
using them on something that isn't a char pointer).

I agree it would be nice to have some type safety there if we can get
it, though. I wonder if:

  static inline uint32_t get_be32(unsigned char *p)
  {
	return ntohl(*(unsigned int *)p);
  }

would generate the same code. It does mean we may have problems between
signed/unsigned buffers, though.

-Peff