[PATCH 3/5] scanner: introduce --object-type option

Pekka Paalanen ppaalanen at gmail.com
Thu Jul 27 13:25:31 UTC 2017


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.

> +
>  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?

> +
>  	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?

>  
>  		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.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20170727/1962b5cd/attachment.sig>


More information about the wayland-devel mailing list