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

From: Jeff King <peff@peff.net>
Date: May 18, 2017, 04:52 UTC
Message-ID: <20170518045207.gd26sq5qdbxg6vnm@sigill.intra.peff.net>
In-Reply-To: <2e5ea82d-3b8f-f508-e2af-5f193241a573@kdbg.org>

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

Show 19 quoted lines

> Am 17.05.2017 um 16:26 schrieb Ben Peart:
> > On 5/16/2017 3:13 PM, Johannes Sixt wrote:
> > > Am 16.05.2017 um 19:17 schrieb Ben Peart:
> > > > 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