patch, 5 partsfix up 'jk/pack-bitmap' branch

Hi Jeff,

These patches fix various errors/warnings on the cygwin, MinGW and msvc builds, provoked by the jk/pack-bitmap branch.

Note that this does not fix all problems on the msvc build; I have a solution, but I don't like it. :-D So, I'm going to try a different fix. I had hoped to have done this by now, but ... (hopefully, sometime this weekend).

Note that I have only tested the cygwin and MinGW builds by running the t5310-pack-bitmaps.sh test, and only on a little-endian machine. (Torsten has tested the first patch on a big-endian)

Note also, that these patches are build on top of the 'pu' branch as of yesterday (pu @ 2b65d9ebc).

So, could you please squash these patches into the relevant commits on your branch.

Thanks!

ATB, Ramsay Jones

Ramsay Jones (5):
  compat/bswap.h: Fix build on cygwin, MinGW and msvc
  Makefile: Add object files in ewah/ to clean target
  khash.h: Spell the null pointer as NULL
  pack-objects: Limit visibility of 'indexed_commits' symbols
  ewah_bitmap.c: Fix printf format warnings on MinGW

 Makefile               |  5 +--
 builtin/pack-objects.c |  6 ++--
 compat/bswap.h         | 97 +++++++++++++++++++++++++++++++++++---------------
 ewah/ewah_bitmap.c     |  6 ++--
 khash.h                |  2 +-
 5 files changed, 79 insertions(+), 37 deletions(-)

On Thu, Nov 07, 2013 at 09:58:02PM +0000, Ramsay Jones wrote:

Thanks. Your timing is impeccable, as I was just sitting down to finalize the next re-roll. I'll add these in.

-Peff

Thank you Ramsay, all the patches look OK to me.

On Thu, Nov 7, 2013 at 11:19 PM, Jeff King <peff@peff.net> wrote:

On 2013-11-07 23.19, Jeff King wrote:

Side question: Do we have enough test coverage for htonll()/ntohll(), or do we want do the "module test" which I send a couple of days before ? /Torsten

On 08/11/13 17:10, Torsten Bögershausen wrote:

Yes, sorry for not bringing that up; I didn't get to try (or even look at) your test patch, because I couldn't 'git am' it. :(

I've been meaning to look into why that is, but just haven't had time yet. (I think it may have something to do with your name - I've noticed in the past that any mail with utf8 characters causes problems.)

However, I should have alerted Jeff to your patch; sorry about that!

ATB, Ramsay Jones

On Fri, Nov 08, 2013 at 06:10:30PM +0100, Torsten Bögershausen wrote:

The series adds tests for building and using the ewah bitmaps, which in turn rely on the htonll code. So they are being tested in the existing series.

-Peff

On 2013-11-08 23.29, Jeff King wrote:

You are thinking about t5310-pack-bitmaps.sh ? If I do like this in compat/bswap.h

# define ntohll(n) (n) # define htonll(n) (n) (on an Intel processor, little endian)

then t5310 passes, even if the uint64_t words are written in little endian to disc instead of big endian.

What do I miss? /torsten

Am 09.11.2013 12:35, schrieb Torsten Bögershausen:

Of course. You write little endian and also read back little endian; that should work just fine, no?

OTOH, if you write with Intel and read with PPC, then you should observe misbehavior with the above patch.

-- Hannes

Johannes Sixt <j6t@kdbg.org> writes:

Maybe you could check in a simple test that the bitmap for a very predictable pack (e.g. only one commit) has a certain content, by checking its hash. That would guard against accidental format changes.

On Sat, Nov 09, 2013 at 10:03:56PM +0100, Thomas Rast wrote:

Another option would be to cross-check reading and writing against JGit, which is valuable for other reasons besides checking endianness. I do not think most people will have JGit installed, but it's better than nothing (and if even one person runs it, we have a chance of finding a bug).

-Peff