On Wed, 26 Jul 2017 16:09:32 +0100 Emil Velikov <[email protected]> wrote:
> On 25 July 2017 at 10:24, Pekka Paalanen <[email protected]> wrote: > > On Tue, 25 Jul 2017 15:25:58 +0800 > > Jonas Ådahl <[email protected]> wrote: > > > >> On Mon, Jul 24, 2017 at 02:16:04PM +0300, Pekka Paalanen wrote: > >> > On Mon, 3 Jul 2017 17:16:45 +0800 > >> > Jonas Ådahl <[email protected]> wrote: > >> > > >> > > Two different protocols may use interfaces with identical names. > >> > > Implementing support for both those protocols would result in symbol > >> > > clashes, as wayland-scanner generates symbols from the interface names. > >> > > > >> > > Make it possible to avoiding these clashes by adding a way to add a > >> > > prefix to the symbols generated by wayland-scanner. Implementations > >> > > (servers and clients) can then use these prefix:ed symbols to implement > >> > > different objects with the same name. > >> > > > >> > > Signed-off-by: Jonas Ådahl <[email protected]> > >> > > --- > >> > > > >> > > Something like this would be needed if a compositor/client wants to > >> > > implement > >> > > xdg-shell unstable v5 alongside xdg-shell stable, unless we want to > >> > > rename all > >> > > our xdg-shell interfaces. Implementing xdg-shell unstable v6 alongside > >> > > xdg-shell stable does not have this issue. > >> > > > >> > > See issue raised here: > >> > > https://lists.freedesktop.org/archives/wayland-devel/2017-June/034380.html > >> > > > >> > > > >> > > Jonas > >> > > > >> > > > >> > > src/scanner.c | 94 > >> > > ++++++++++++++++++++++++++++++++++++++++++++++------------- > >> > > 1 file changed, 73 insertions(+), 21 deletions(-) > >> > > >> > > >> > Hi, > >> > > >> > while this seems to change the ABI symbol names, it does not change the > >> > names in the documentation, and it does not change the names of > >> > #defines of enums, or the inline functions. That means that this is not > >> > enough to fulfill the purpose: being able to use two similarly named > >> > but different protocols by adding a prefix. > >> > >> The idea I had was rather that one would avoid changing any names on > >> non-symbols. It'd still be possible to implement both by doing it in > >> separate C files. I can see the point in adding the prefix on everything > >> though, so I'll provide a patch for that. > >> > > > > Hi, > > > > recapping the discussion from IRC, we pretty much agreed that prefixing > > is not a nice solution. Jonas found out that we cannot actually prefix > > everything, because there usually are references to other protocol > > things (like you would never want to prefix wl_surface). So it becomes > > very hard to prefix things appropriately. > > > > The alternative we discussed is solving a different problem: scanner > > makes all the public symbols in the generated .c files WL_EXPORT, which > > makes them leak into DSO ABI, which is bad. In my opinion, it should > > have never happened in the first place. But we missed it, and now it has > > spread, so we cannot just fix scanner to stop exporting, the decision > > must be with the consumers. So we need a scanner option to stop > > exporting. > > > > Quentin proposed we add a scanner option > > --visibility={static|compiler|export}. It would affect all the symbols > > exported from the generated .c files as follows: > > > > - static: the symbols will be static. > > - compiler: the symbols will get whatever the default visibility is > > with the compiler, i.e. not explicitly static and not exported > > - export: the symbols are exported (this the old behaviour, and will be > > the default) > > > > Obviously, the only way to actually make use of the 'static' option is > > for the consumer to #include the generated .c file. It's ugly, yes, but > > it solves the conflicting symbol names issue Jonas was looking into. > > > > In my opinion, the prefixing approach where we still cannot prefix > > everything in a way that one could use conflicting protocols in the > > same compilation unit, and where e.g. the static inline functions are > > not prefixed, is more ugly than the 'static' option. > > > > We are going to need an option to stop the exports anyway, and it seems > > like we can piggyback on that solution for the problem underlying the > > prefixing proposal as well. > > > It sounds like Quentin proposal is trying to address two distinct > things at the same time: > - expose correct symbol visiblity > - avoid symbol crashes due to conflicting naming used for "different" > protocols Yes, it indeed is. > Former is addressed by swapping WL_EXPORT with WL_PRIVATE, or alike > While the latter can be tackled in a couple of says: > - manually update the sources to use separate distinct name for the protocol > - convince the generator (scanner) to produce ones for us > > I think convincing the scanner is a perfectly reasonable solution. Does that mean you are in favour of this patch 2/3 as is? Is it ok to prefix only the "ABI symbols" and leave everything else in the API, e.g. static inline functions, unprefixed? Thanks, pq
pgpVJvRNqyG_G.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
