Re: Troubles with ASN generated code

[Guy Harris <guy () alum mit edu>]

On May 19, 2017, at 9:38 AM, Jaap Keuter <jaap.keuter () xs4all nl> wrote:

> So, this change https://code.wireshark.org/review/#/c/21708/ appears to make the
> buildbot happy, the GCC6 builds still fails with
> (3346fc9c83719bce26cca31fc4b5d9139ab56d76)
>
> In file included from ./asn1/q932/packet-q932-template.c:33:0:
> ./asn1/q932/packet-q932-exp.h:26:27: error:
> ‘q932_PresentedNumberUnscreened_vals’ defined but not used
> [-Werror=unused-const-variable=]
> ./asn1/q932/packet-q932-exp.h:18:27: error: ‘q932_PresentedNumberScreened_vals’
> defined but not used [-Werror=unused-const-variable=]
> ./asn1/q932/packet-q932-exp.h:10:27: error:
> ‘q932_PresentedAddressUnscreened_vals’ defined but not used
> [-Werror=unused-const-variable=]
> ./asn1/q932/packet-q932-exp.h:2:27: error: ‘q932_PresentedAddressScreened_vals’
> defined but not used [-Werror=unused-const-variable=]

GCC 6 is probably correct.

*Defining* functions and data (except perhaps for inline functions) in a header file is generally Not The Right Thing 
To Do, because

        1) if only one source file includes the header, and there's no reason for other files to include it, the very 
existence of the header is misleading, as it implies that the stuff in the header is stuff being exported from the file;

        2) if more than one source file includes the header, you have multiple copies of the data or function, rather 
than sharing a single copy, which is arguably wasteful;

        3) if more than one file includes the header, and not all of them use all of the data or functions, you have 
multiple copies of the data or function *and some of them are unused*, which is *definitely* wasteful.

The only reason why you *might* want to do this for data is that, if you have a plugin module, and it has a data 
structure that points to one of the data structures mentioned in the header, the run-time loader on some platform you 
support might not do the load-time evaluation of the address of the structure required to make this work.  For example, 
if libwireshark exports a value_string table, I'm not sure all the platforms we support would allow a plugin dissector 
to point to that value_string from one of its header fields.

Given that we're already exporting value_string tables from libwireshark, either

        1) it works on all platforms

or

        2) we're not doing the stuff that doesn't work

so I wouldn't be opposed to doing with asn2wrs-generated dissectors the same way we do it for hand-written dissectors, 
i.e.:

        have the header file declare but *not* define the value_strings, and declare it with WS_DLL_PUBLIC rather than 
either static or extern;

        have the source files define the value strings, and define them with WS_DLL_PUBLIC_DEF.

Any value_string that asn2wrs is not *absolutely certain* will be used only by one source file should *not* be made 
static and should *not* be defined in a header file.  Any value string that asn2wrs *is* absolutely certain will be 
used only by one source file should be defined as static *in that source file*, *not* in a header file.
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe