[Graham Bloice <graham.bloice@xxxxxxxxxxxxx>]

On 11 December 2013 13:10, Gisle Vanem <gvanem@xxxxxxxx> wrote:

I saw the recent change to 'tools/make-dissector-reg.py' to allow
reading list of files from a file; the argument "dissectorsinfile". I find this a bit awkward. Isn't it better to use the more familiar response-file syntax? So the command to generate register.c would be something like:
 @$(PYTHON) "../../tools/make-dissector-reg.py" . dissectors \
  @$(some_temp_file_with_all_dissectors_src)

I started this off while working on the CMake builds for Visual Studio, due to a bug in Visual Studio where it would drop every 8192nd character from a long command line as used to be passed to make-dissector-reg. My hack was to produce a file containing the file list, and maybe due to my CMake ineptness it turned out to be one file per line.  The intent was that only CMake would use make-reg-dissector in this way, nmake, and automake builds would continue as before with all files passed on the command line.

The hack was picked up by others, and then modified for use python < 2.7 and committed to trunk. 

 

Not sure how to best produce this temp_file. But certainly not one
file per line as the snippet:
 files = [line.rstrip() for line in dissector_f]

indicates. 'dissector_f.read().split()' would be more robust I think.
So what about this patch:

--- orig/tools/make-dissector-reg.py        2013-12-10 21:58:26 +0000
+++ tools/make-dissector-reg.py   2013-12-11 13:00:59 +0000
@@ -42,7 +42,7 @@
 * Generated automatically from %s.
 */
""" % (sys.argv[0])
-elif registertype in ("dissectors", "dissectorsinfile"):
+elif registertype == "dissectors":
    final_filename = "register.c"
    cache_filename = "register-cache.pkl"
    preamble = """\
@@ -65,16 +65,18 @@

#
# All subsequent arguments are the files to scan
-# or the name of a file containing the files to scan
+# or the name of a '@response-file' containing the files to scan
#
-if registertype == "dissectorsinfile":
+if sys.argv[3][0] == '@':
+    resp_file = sys.argv[3][1:]
    try:
-        dissector_f = open(sys.argv[3])
+        file = open(resp_file)
    except IOError:
-        print(("Unable to open input file '%s'" % sys.argv[3]))
+        print(("Unable to open response-file '%s'" % resp_file))
        sys.exit(1)
-
-    files = [line.rstrip() for line in dissector_f]
+    files = file.read().split()
+    file.close()
else:
    files = sys.argv[3:]

--------------

BTW. You forgot a 'dissector_f.close()'.

So, unless you're interested in CMake builds for Visual Studio, you can effectively ignore the changes to make-dissector-reg, but if you feel the need to improve it, then the CMake will have to be fixed at the same time.

Graham