[FFmpeg-devel] [PATCH] avutil/cpu: remove the |checked| static variable

[Wan-Teh Chang]

Remove the |checked| variable because the invalid value of -1 for
|flags| can be used to indicate the same condition. Also rename |flags|
to |cpu_flags| because there are a local variable and a function
parameter named |flags| in the same file.

Add a test program libavutil/tests/cpu_init.c to check whether the
one-time initialization in av_get_cpu_flags() has data races.

Co-author: Dmitry Vyukov of Google

Signed-off-by: Wan-Teh Chang <wtc at google.com>
---
 libavutil/Makefile         |  1 +
 libavutil/cpu.c            | 40 ++++++++++++++------------
 libavutil/tests/cpu_init.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 94 insertions(+), 19 deletions(-)
 create mode 100644 libavutil/tests/cpu_init.c

diff --git a/libavutil/Makefile b/libavutil/Makefile
index 0fa90fe..732961a 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -187,6 +187,7 @@ TESTPROGS = adler32                                                     \
             camellia                                                    \
             color_utils                                                 \
             cpu                                                         \
+            cpu_init                                                    \
             crc                                                         \
             des                                                         \
             dict                                                        \
diff --git a/libavutil/cpu.c b/libavutil/cpu.c
index f5785fc..73317c4 100644
--- a/libavutil/cpu.c
+++ b/libavutil/cpu.c
@@ -44,7 +44,20 @@
 #include <unistd.h>
 #endif
 
-static int flags, checked;
+static int cpu_flags = -1;
+
+static int get_cpu_flags(void)
+{
+    if (ARCH_AARCH64)
+        return ff_get_cpu_flags_aarch64();
+    if (ARCH_ARM)
+        return ff_get_cpu_flags_arm();
+    if (ARCH_PPC)
+        return ff_get_cpu_flags_ppc();
+    if (ARCH_X86)
+        return ff_get_cpu_flags_x86();
+    return 0;
+}
 
 void av_force_cpu_flags(int arg){
     if (   (arg & ( AV_CPU_FLAG_3DNOW    |
@@ -69,33 +82,22 @@ void av_force_cpu_flags(int arg){
         arg |= AV_CPU_FLAG_MMX;
     }
 
-    flags   = arg;
-    checked = arg != -1;
+    cpu_flags = arg;
 }
 
 int av_get_cpu_flags(void)
 {
-    if (checked)
-        return flags;
-
-    if (ARCH_AARCH64)
-        flags = ff_get_cpu_flags_aarch64();
-    if (ARCH_ARM)
-        flags = ff_get_cpu_flags_arm();
-    if (ARCH_PPC)
-        flags = ff_get_cpu_flags_ppc();
-    if (ARCH_X86)
-        flags = ff_get_cpu_flags_x86();
-
-    checked = 1;
+    int flags = cpu_flags;
+    if (flags == -1) {
+        flags = get_cpu_flags();
+        cpu_flags = flags;
+    }
     return flags;
 }
 
 void av_set_cpu_flags_mask(int mask)
 {
-    checked       = 0;
-    flags         = av_get_cpu_flags() & mask;
-    checked       = 1;
+    cpu_flags = get_cpu_flags() & mask;
 }
 
 int av_parse_cpu_flags(const char *s)
diff --git a/libavutil/tests/cpu_init.c b/libavutil/tests/cpu_init.c
new file mode 100644
index 0000000..5a5ecda
--- /dev/null
+++ b/libavutil/tests/cpu_init.c
@@ -0,0 +1,72 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/*
+ * This test program tests whether the one-time initialization in
+ * av_get_cpu_flags() has data races.
+ */
+
+#include <stdio.h>
+#include <string.h>
+
+#include "config.h"
+
+#include "libavutil/cpu.h"
+#include "libavutil/thread.h"
+
+#if HAVE_PTHREADS
+static void *thread_main(void *arg)
+{
+    int *flags = arg;
+
+    *flags = av_get_cpu_flags();
+    return NULL;
+}
+#endif
+
+
+int main(int argc, char **argv)
+{
+#if HAVE_PTHREADS
+    int cpu_flags1;
+    int cpu_flags2;
+    int ret;
+    pthread_t thread1;
+    pthread_t thread2;
+
+    if ((ret = pthread_create(&thread1, NULL, thread_main, &cpu_flags1))) {
+        fprintf(stderr, "pthread_create failed: %s.\n", strerror(ret));
+        return 1;
+    }
+    if ((ret = pthread_create(&thread2, NULL, thread_main, &cpu_flags2))) {
+        fprintf(stderr, "pthread_create failed: %s.\n", strerror(ret));
+        return 1;
+    }
+    pthread_join(thread1, NULL);
+    pthread_join(thread2, NULL);
+
+    if (cpu_flags1 < 0)
+        return 2;
+    if (cpu_flags2 < 0)
+        return 2;
+    if (cpu_flags1 != cpu_flags2)
+        return 3;
+#endif
+
+    return 0;
+}
-- 
2.8.0.rc3.226.g39d4020