Re: [PATCH 4/3] Introduce 'convert_path_to_git()'

From: Junio C Hamano <gitster@pobox.com>
Date: May 16, 2009, 06:40 UTC
Message-ID: <7vy6sxpn2q.fsf@alter.siamese.dyndns.org>
In-Reply-To: <alpine.LFD.2.01.0905151156090.3343@localhost.localdomain>

Linus Torvalds <torvalds@linux-foundation.org> writes:

Show 12 quoted lines

> +#ifdef FAST_UNALIGNED
> +	while (plen >= sizeof(unsigned long)) {
> +		unsigned long x = *(unsigned long *)path;
> +		if (x & (unsigned long) 0x8080808080808080ull)
> +			break;
> +		*(unsigned long *)result = x;
> +		path += sizeof(unsigned long);
> +		result += sizeof(unsigned long);
> +		plen -= sizeof(unsigned long);
> +		retval += sizeof(unsigned long);
> +	}
> +#endif

It somehow bothers me that the stride of this loop is protected from having different size of unsigned long from what the author of this function expected, but the literal constant used as the mask is not quite, which means that taken as a whole it does not work if your unsigned long is more than 8-bytes. No, I do not think it matters in practice, and I find it a neat trick to cast the unsigned long long literal down to unsigned long, but still it looks somewhat, eh, what would I say...

"Ugly" is not quite the word I am looking for. "My gut feels that there has to be a way to write this more cleanly, but I am frustrated that I cannot come up with one" might be the word...