[PATCH 3/5] scanner: introduce --object-type option
Emil Velikov
emil.l.velikov at gmail.com
Fri Jul 28 12:13:48 UTC 2017
On 27 July 2017 at 14:25, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> On Wed, 26 Jul 2017 14:56:19 +0100
> Emil Velikov <emil.l.velikov at gmail.com> wrote:
>
>> From: Emil Velikov <emil.velikov at collabora.com>
>>
>> The option is used to indicate how the code will be used - would it be a
>> part of shared or static one.
>>
>> In the former case one needs to export the specific symbols, although
>> normally people want to statically build the protocol code into their
>> project.
>>
>> If the option is missing a warning is emitted, to point people and do
>> the right thing.
>>
>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>> ---
>> src/scanner.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 56 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/scanner.c b/src/scanner.c
>> index c345ed6..cc45b74 100644
>> --- a/src/scanner.c
>> +++ b/src/scanner.c
>> @@ -57,6 +57,11 @@ enum side {
>> SERVER,
>> };
>>
>> +enum object_type {
>> + SHARED,
>> + STATIC,
>> +};
>
> Hi,
>
> I could go with this as well, as long as we have some solution to the
> original problem of conflicting interface names that Jonas is trying to
> solve.
>
i think the original patch from Jonas, addressed that issue nicely.
Let's keep that in the respective thread.
>> +
>> static int
>> usage(int ret)
>> {
>> @@ -70,6 +75,11 @@ usage(int ret)
>> fprintf(stderr, " -h, --help display this help and exit.\n"
>> " -v, --version print the wayland library version that\n"
>> " the scanner was built against.\n"
>> + " -t, --object-type=[static,shared]\n"
>> + " How is the resulting code going to be built/used\n"
>> + " static - standalone static object, used internally\n"
>> + " shared - shared library, to be used by multiple projects\n"
>> + " Using static is highly recommened.\n"
>> " -c, --include-core-only include the core version of the headers,\n"
>> " that is e.g. wayland-client-core.h instead\n"
>> " of wayland-client.h.\n");
>> @@ -1712,9 +1722,11 @@ emit_messages(struct wl_list *message_list,
>> printf("};\n\n");
>> }
>>
>> +
>> static void
>> -emit_code(struct protocol *protocol)
>> +emit_code(struct protocol *protocol, enum object_type obj_type)
>> {
>> + const char *symbol_visibility;
>> struct interface *i, *next;
>> struct wl_array types;
>> char **p, *prev;
>> @@ -1728,6 +1740,19 @@ emit_code(struct protocol *protocol)
>> "#include <stdint.h>\n"
>> "#include \"wayland-util.h\"\n\n");
>>
>> + /* When building a shared library symbols must be exported, otherwise
>> + * we want to have the symbols hidden. */
>> + if (obj_type == STATIC) {
>> + symbol_visibility = "WL_PRIVATE";
>> + printf("#if defined(__GNUC__) && __GNUC__ >= 4\n"
>> + "#define WL_PRIVATE __attribute__ ((visibility(\"hidden\")))\n"
>> + "#else\n"
>> + "#define WL_PRIVATE\n"
>> + "#endif\n\n");
>> + } else {
>> + symbol_visibility = "WL_EXPORT";
>> + }
>
> I wonder... would it be any better to just replace 'WL_EXPORT' and
> 'extern' with tokens:
>
> - LOCAL_INTERFACE_DECL for declarations of symbols that will also be
> defined in the generated code file
>
> - EXTERN_INTERFACE_DECL for declarations of symbols that the generated
> files will need but the code file will not define, that is,
> interfaces defined on other XML files.
>
> - LOCAL_INTERFACE_DEF for symbol definitions in the generated code file.
>
> The exported configuration would then be:
> LOCAL_INTERFACE_DECL=extern
> EXTERN_INTERFACE_DECL=extern
> LOCAL_INTERFACE_DEF=WL_EXPORT
>
> That would be far too flexible and no-one would use it right, right?
>
I did not introduce separate tokens, since those are (and should be)
used _only_ in the .c file.
Personally then do not seem necessary, If you prefer we can add them though.
>> +
>> wl_array_init(&types);
>> wl_list_for_each(i, &protocol->interface_list, link) {
>> emit_types_forward_declarations(protocol, &i->request_list, &types);
>> @@ -1757,10 +1782,10 @@ emit_code(struct protocol *protocol)
>> emit_messages(&i->request_list, i, "requests");
>> emit_messages(&i->event_list, i, "events");
>>
>> - printf("WL_EXPORT const struct wl_interface "
>> + printf("%s const struct wl_interface "
>> "%s_interface = {\n"
>> "\t\"%s\", %d,\n",
>> - i->name, i->name, i->version);
>> + symbol_visibility, i->name, i->name, i->version);
>>
>> if (!wl_list_empty(&i->request_list))
>> printf("\t%d, %s_requests,\n",
>> @@ -1790,6 +1815,24 @@ free_protocol(struct protocol *protocol)
>> free_description(protocol->description);
>> }
>>
>> +static enum object_type
>> +parse_obj_type(const char *obj_type_str)
>> +{
>> + if (!obj_type_str) {
>> + fprintf(stderr, "Warning: --object-type is not specified, assuming shared.\n");
>> + return SHARED;
>> + }
>> +
>> + if (strcmp(obj_type_str, "static") == 0)
>> + return STATIC;
>> +
>> + if (strcmp(obj_type_str, "shared") == 0)
>> + return SHARED;
>> +
>> + fprintf(stderr, "Error: invalid object type string '%s'\n", obj_type_str);
>> + usage(EXIT_FAILURE);
>> +}
>> +
>> int main(int argc, char *argv[])
>> {
>> struct parse_context ctx;
>> @@ -1802,6 +1845,8 @@ int main(int argc, char *argv[])
>> bool core_headers = false;
>> bool version = false;
>> bool fail = false;
>> + char *obj_type_str = NULL;
>> + enum object_type obj_type;
>> int opt;
>> enum {
>> CLIENT_HEADER,
>> @@ -1812,12 +1857,13 @@ int main(int argc, char *argv[])
>> static const struct option options[] = {
>> { "help", no_argument, NULL, 'h' },
>> { "version", no_argument, NULL, 'v' },
>> + { "object-type", required_argument, NULL, 't' },
>> { "include-core-only", no_argument, NULL, 'c' },
>> { 0, 0, NULL, 0 }
>> };
>>
>> while (1) {
>> - opt = getopt_long(argc, argv, "hvc", options, NULL);
>> + opt = getopt_long(argc, argv, "hvtc", options, NULL);
>
> Should be "hvt:c" I think?
>
Indeed. Thanks.
>>
>> if (opt == -1)
>> break;
>> @@ -1829,6 +1875,9 @@ int main(int argc, char *argv[])
>> case 'v':
>> version = true;
>> break;
>> + case 't':
>> + obj_type_str = optarg;
>> + break;
>> case 'c':
>> core_headers = true;
>> break;
>> @@ -1858,6 +1907,8 @@ int main(int argc, char *argv[])
>> else
>> usage(EXIT_FAILURE);
>>
>> + obj_type = parse_obj_type(obj_type_str);
>> +
>> if (argc == 3) {
>> input_filename = argv[1];
>> input = fopen(input_filename, "r");
>> @@ -1937,7 +1988,7 @@ int main(int argc, char *argv[])
>> emit_header(&protocol, SERVER);
>> break;
>> case CODE:
>> - emit_code(&protocol);
>> + emit_code(&protocol, obj_type);
>> break;
>> }
>>
>
> The patch looks pretty much correct to me, if we choose to go this way.
>
Glad to hear.
I'll let me know once you guys are settled in on the approach, and
I'll respin the series with all the comments addressed.
-Emil
More information about the wayland-devel
mailing list