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

From: Nicolas Pitre <nico@cam.org>
Date: Aug 18, 2009, 19:22 UTC
Message-ID: <alpine.LFD.2.00.0908181516510.6044@xanadu.home>
In-Reply-To: <7vk511dk11.fsf@alter.siamese.dyndns.org>

On Tue, 18 Aug 2009, Junio C Hamano wrote:

Show 17 quoted lines

> Nicolas Pitre <nico@cam.org> writes:
> 
> > On Tue, 18 Aug 2009, Junio C Hamano wrote:
> >
> >> For git's own use, I would be much happier with this change.
> >> 
> >> But given that there are some people wanting to snarf block-sha1/*.[ch]
> >> out to use them standalone, I have a slight hesitation against introducing
> >> the dependency to git-compat-util.h, making it unclear to them that all
> >> this file wants from outside are ntohl, htonl and memcpy.
> >
> > Should we really care to keep our code suboptimal just to make it 
> > readily reusable by other projects?  That seems a bit backward to me.
> 
> You are right; and I should give a bit more credit to their intelligence.
> The source (block-sha1/sha1.c) is short enough that they can figure this
> out for themselves even without any additional comments.

Well, I gave in and added a comment to the patch anyway, with more improvements in the case of constant values. Patch follows.

> Another issue, especially with your "openssl sha1 removal" patch, is if we
> can assume gcc everywhere.  As far as I can tell, block-sha1/sha1.c will
> be the first unconditional use of inline asm or statement expression on
> i386/amd64.  Are folks on Solaris and other platforms Ok with this?

I guess we can guard the first with ifdef(__GNUC__) which should help people with MSVC. That should take care of x86 at least.

Nicolas