Re: [PATCH] block-sha1: Windows declares ntohl() in winsock2.h

From: Nicolas Pitre <nico@cam.org>
Date: Aug 18, 2009, 18:10 UTC
Message-ID: <alpine.LFD.2.00.0908181403190.6044@xanadu.home>
In-Reply-To: <bdca99240908180959h69f37671k4d526fbf4814e8d1@mail.gmail.com>

On Tue, 18 Aug 2009, Sebastian Schuberth wrote:

Show 11 quoted lines

> On Tue, Aug 18, 2009 at 18:43, Nicolas Pitre<nico@cam.org> wrote:
> 
> >> Well...  Given that git already uses ntohl/htonl quite extensively in
> >> its core already, I'd suggest making this more globally available
> >> instead.
> >
> > What about something like this?
> 
> I like the idea of making bswap available more globally, but I'm not
> sure if it's worth to introduce a new file for only that purpose.
> Isn't there already a central header for such things?

That central header is already quite crowded. A bit of isolation might not hurt.

Furthermore, other platforms might wish to add their own (re)definitions for those byte swap operations, so it has the potential to grow. I for example have a better implementation for ARM than what is provided by glibc. (Yeah yeah, maybe glibc should be fixed instead, but that reasoning goes for all those other libraries too).

> Moreover, including compat/bswap.h would only give you ntohl()/htonl()
> on one platform. For consistency, I'd expect to get those for any
> platform if I include compat/bswap.h, but maybe I'm not aware of some
> Git source code rules.

You get it by default for all platforms already by including git-compat-util.h. The compat/bswap.h is not meant to be included by random c files. If compat/bswap.h happens to contain a better version for your architecture then it'll override the default one.

> Finally, there's a typo in your comment saying "sinple" instead of "simple".

Thanks

Nicolas