Re: [PATCH] fix threaded grep for machines with only one cpu

Heiko Voigt [off-list ref] writes:

quoted

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.

quoted

+#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?