Re: [PATCH] builtin/index-pack.c: Fix some pthread_t misuse

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 7, 2012, 19:15 UTC
Message-ID: <7vvcmgmde3.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4F57B04F.6050307@ramsay1.demon.co.uk>

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

Show 9 quoted lines

> However, pthread_t is intended to be an opaque (implementation defined)
> type. For example, an implementation may choose to use a structure to
> implement the type.  Therefore, assigning zero (or any other constant)
> to a pthread_t is not supported in general.
> ...
> Note that, for the same reason given above, you can not, in general,
> directly compare pthread_t handles with the built-in equality operator.
> In order to compare pthread_t's for equality, the POSIX standard requires
> the use of pthread_equal().

Thanks, the above analysis all sound sensible.

I do not think it matters in *this* case, but if a loop iterates over an array of things with a field of type pthread_t in it, whose element may or may not be valid, and wants to mark the validity of an element with the value of its pthread_t field, what is the proper way to do so? I.e.

	for (i = 0; i < ARRAY_SIZE(thread_data); i++) {
		if (pthread_invalid(thread_data[i].thread)
			continue; /* not used */
        	if (!pthread_equal(self, thread_data[i].thread))
                	continue; /* not me */
		/* ah, this is mine! */
                ...
	}

Perhaps the answer is "Don't do it" and that is perfectly fine, but does Nguyen's code rely on the final clean-up (assignment with 0 you are removing with this patch) to mark that these elements are no longer relevant?