Heiko Voigt <hvoigt@xxxxxxxxxx> writes:
>> IOW, please try this patch. I am planning to queue it to 'maint' as part
>> of 1.7.0.1 if this is the right solution (which I obviously think it is).
>
> Yes your patch does it correctly I just verified that the segfaults are
> gone as well. I think your solution is even nicer than mine. Thanks.
>> +#define read_sha1_lock() do { if (use_threads) pthread_mutex_lock(&read_sha1_mutex); } while (0)
>> +#define read_sha1_unlock() do { if (use_threads) pthread_mutex_unlock(&read_sha1_mutex); } while (0)
>
> One minor thing: Would it not be even nicer having the while loop inside
> the if clause? E.g like this
>
> #define read_sha1_lock() if (use_threads) do { pthread_mutex_lock(&read_sha1_mutex); } while (0)
> #define read_sha1_unlock() if (use_threads) do { pthread_mutex_unlock(&read_sha1_mutex); } while (0)
No.
#define frotz() do { this compound stmt; } while (0)
is a common idiom to make a macro that expands to a compound stmt behave
as if it is a simple function call to avoid bugs when it is expanded,
regardless of in which context it is used by an unsuspecting caller.
Your rewrite is pointless because it is the same as saying
#define read_sha1_lock() if (use_threads) p_m_l(&r_s_m)
and that is exactly what the idiom's use of "do { } while (0)" is all
about.
Try this simple program.
-- >8 ---- >8 ---- >8 ---- >8 ---- >8 ---- >8 ---- >8 ---- >8 --
#include <stdio.h>
#define frotz() do { if (flag) printf("frotz"); } while (0)
#define xyzzy() if (flag) do { printf("xyzzy"); } while (0)
#define yomin() if (flag) printf("yomin")
void test(int foo, int bar, int baz, int flag)
{
if (foo)
frotz();
else if (bar)
xyzzy();
else if (baz)
yomin();
else
printf("huh?");
printf("\n");
}
int main(int ac, char **av)
{
int foo, bar, baz;
for (foo = 0; foo < 2; foo++)
for (bar = 0; bar < 2; bar++)
for (baz = 0; baz < 2; baz++) {
printf("%d %d %d ", foo, bar, baz);
test(foo, bar, baz, 1);
}
return 0;
}
-- >8 ---- >8 ---- >8 ---- >8 ---- >8 ---- >8 ---- >8 ---- >8 --
Textually the "test" function expands to this:
void test(int foo, int bar, int baz, int flag)
{
if (foo)
do { if (flag) printf("frotz"); } while (0);
else if (bar)
if (flag) do { printf("xyzzy"); } while (0);
else if (baz)
if (flag) printf("yomin");
else
printf("huh?");
printf("\n");
}
but if you properly indent it, it looks like this:
void test(int foo, int bar, int baz, int flag)
{
if (foo)
do {
if (flag)
printf("frotz");
} while (0);
else if (bar)
if (flag)
do {
printf("xyzzy");
} while (0);
else if (baz)
if (flag)
printf("yomin");
else
printf("huh?");
printf("\n");
}
Notice how your version (xyzzy) broke the cascade of if..elseif..else.
Don't they teach this in schools anymore?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html