Re: tools/make-dissector-reg.py

[Graham Bloice <graham.bloice () trihedral com>]

On 11 December 2013 13:10, Gisle Vanem <gvanem () yahoo no> 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

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe