Switch back to subprocess.run (#908)

* helpers.py: switch back to subprocess.run (revert 346305fc)

* test_free: replace find|grep|xargs with python solution

tests/helpers.py

@@ -17,34 +17,29 @@ def run_subprocess(command, working_dir='.', env=None, expected_returncode=0, in
     Helper function to run a shell command and report success/failure
     depending on the exit status of the shell command.
     """
+    env_ = os.environ.copy()
     if env is not None:
-        env_ = os.environ.copy()
         env_.update(env)
-        env = env_
+    env = env_
 
     # Note we need to capture stdout/stderr from the subprocess,
     # then print it, which pytest will then capture and
     # buffer appropriately
     print(working_dir + " > " + " ".join(command))
-    proc = subprocess.Popen(
-        command,
-        stdin=subprocess.PIPE,
-        stdout=subprocess.PIPE,
-        stderr=subprocess.STDOUT,
-        cwd=working_dir,
-        env=env,
-    )
 
-    try:
-        out, _ = proc.communicate(input=input, timeout=480)
-    except subprocess.TimeoutExpired:
-        proc.kill()
-        out, _ = proc.communicate()
+    result = subprocess.run(
+            command,
+            input=input,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.STDOUT,
+            cwd=working_dir,
+            env=env,
+        )
 
-    if not(ignore_returncode) and (proc.returncode != expected_returncode):
-        print(out.decode('utf-8'))
-        assert False, "Got unexpected return code {}".format(proc.returncode)
-    return out.decode('utf-8')
+    if not(ignore_returncode) and (result.returncode != expected_returncode):
+        print(result.stdout.decode('utf-8'))
+        assert False, "Got unexpected return code {}".format(result.returncode)
+    return result.stdout.decode('utf-8')
 
 def available_kems_by_name():
     available_names = []

tests/test_code_conventions.py

@@ -1,7 +1,9 @@
 # SPDX-License-Identifier: MIT
 
 import helpers
+import os
 import pytest
+import re
 import sys
 
 # Ensure every key-exchange algorithm in the code
@@ -50,16 +52,21 @@ def test_spdx():
 @helpers.filtered_test
 @pytest.mark.skipif(sys.platform.startswith("win"), reason="Not needed on Windows")
 def test_free():
-    files = helpers.run_subprocess(['find', 'src', '-name', '*.c'])
-    files = helpers.run_subprocess(['grep', '-v', 'picnic/external'], input=files.encode('utf8'))
-    lines = helpers.run_subprocess(['xargs', 'grep', '[^_]free('], input=files.encode('utf8'))
-    lines = lines.split("\n")
+    c_files = []
+    for path, _, files in os.walk('src'):
+        if os.path.join('picnic','external') in path: continue
+        c_files += [os.path.join(path,f) for f in files if f[-2:] == '.c']
     okay = True
-    for line in lines:
-        if line == "": continue
-        if not('IGNORE free-check' in line):
-            okay = False
-            print("Suspicious `free` in " + line)
+    for fn in c_files:
+        with open(fn) as f:
+            # Find all lines that contain 'free(' but not '_free('
+            for no, line in enumerate(f,1):
+                if not re.match(r'^.*[^_]free\(.*$', line):
+                    continue
+                if 'IGNORE free-check' in line:
+                    continue
+                okay = False
+                print("Suspicious `free` in {}:{}:{}".format(fn,no,line))
     assert okay, "'free' is used in some files.  These should be changed to 'OQS_MEM_secure_free' or 'OQS_MEM_insecure_free' as appropriate. If you are sure you want to use 'free' in a particular spot, add the comment '// IGNORE free-check' on the line where 'free' occurs."
 
 if __name__ == "__main__":