Re: [PATCH] Introduce the authorizer protocol
Hi, On 25 November 2015 at 16:14, Pekka Paalanen <ppaala...@gmail.com> wrote: > On Tue, 24 Nov 2015 18:07:34 +0100 > Mariusz Ceier <mceier+wayl...@gmail.com> wrote: > > > Hi, > > > > On 24 November 2015 at 17:35, Giulio Camuffo <giuliocamu...@gmail.com> > > wrote: > > > > > 2015-11-24 18:16 GMT+02:00 Mariusz Ceier <mceier+wayl...@gmail.com>: > > > > Hi, > > > > How the clients will know: > > > > a) which interface is restricted and which is not ? > > > > > > It doesn't, but does it need to know it? It can still ask for > > > authorization even if the interface is not. > > > > > > > > I think it should - otherwise client, that wants to run on as many > > compositors as possible, would have to send authorize requests for every > > interface, since different compositors can implement different policies > and > > interface in one compositor can be unrestricted while in another > > restricted. > > Why should we separate restricted interfaces in the registry level? Or > at all? > > Because binding restricted interface kills client and since interface specification doesn't tell if that interface is restricted or not, binding would be a gamble for client. Also to authorize the client to use of restricted interface, authorizer object needs to be created - that means restricted interfaces can't be bound before authorizer object is announced, and that means restricted interfaces can't be announced before authorizer is announced, unless client can bind them later (which I don't think is a case). The interface specification needs to say whether it is restricted or > not. If a compositor does not care about restricting it, it'll just > always answer "yes." > Even if interface specification would include information whether it's restricted or not, that information would have to be included in protocol - since later versions of protocol can become restricted. > When you design a new protocol extension, I can't imagine that you > would not know if it needs to be restricted or not. Do you have any > example to the contrary? > I think it may be the case during development and even after it gets stabilized. It may be unrestricted at first, but then someone might find security bug (in design) or request requiring privileged access could be added - in these cases compositors probably would like to restrict access to that interface. In v1 of authorizer that would mean, that clients which assumed that specific interfaces are unrestricted would be killed by compositor. And in v2 authorizer clients would know that they need to send authorize request first - since interface wouldn't be available in wl_registry but only in wl_restricted_registry. > > > > b) that the compositor implements restricted interface ? should they > be > > > > visible in the registry ? > > > > > > Well, they are normal globals so they will are available in the > > > registry. If needed, this interface could > > > send out a list of the restricted ones, but i'm not sure it is. > > > > > > > > If they're normal globals - registry will contain a mix of restricted and > > unrestricted interfaces, and in current state clients won't know which is > > restricted and which is not, and binding to restricted interface will > kill > > client - are we playing russian roulette ? ;) > > Clients do not go binding random globals without knowing the > specification. > Still specification doesn't say which protocol is restricted and which is not. In v1 authorizer clients can't assume that specific interface is always unrestricted (it could become restricted in later version or compositor could implement different policy), so they would have to send authorize request for each interface they use. > > > I think something like wl_registry but for restricted interfaces only, > may > > be a better solution. > > I see no need for this. > > > Also since authorizer is another global, it would have to be announced > > before any restricted interface, right ? > > That requirement probably should be added to description. > > There is no such requirement. Clients *must* deal with arbitrary > announcements orders anyway. > > Except they would get killed for trying to bind restricted interface without sending authorize request - and they can't send it before getting authorizer object (in v1). So v1 introduces weak order here. > > Thanks, > pq > Mariusz Ceier ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC wayland-protocols v2] Introduce the restricted_registry protocol
Hi, It's missing README, but otherwise it looks good to me: Reviewed-by: Mariusz Ceier <mceier+wayl...@gmail.com> Mariusz Ceier On 25 November 2015 at 15:10, Giulio Camuffo <giuliocamu...@gmail.com> wrote: > This new extension is used by clients wanting to execute priviledged > actions such as taking a screenshot. > The usual way of granting special priviledged to apps is to fork and > exec them in the compositor, and then checking if the client is the > known one when it binds the restricted global interface. This works > but is quite limited, as it doesn't allow the compositor to ask the > user if the app is trusted, because it can't wait for the answer in > the bind function as that would block the compositor. > This new protocol instead allows the answer to come after some time > without blocking the compositor or the client. > --- > > v2: renamed to restricted_registry. > It now replaces wl_registry entirely for the restricted interfaces > and hence it makes it lets clients know for sure that the globals > in wl_registry are not restricted, and which are the restricted > ones. > > > Makefile.am| 1 + > .../restricted-registry/restricted-registry-v1.xml | 148 > + > 2 files changed, 149 insertions(+) > create mode 100644 unstable/restricted-registry/restricted-registry-v1.xml > > diff --git a/Makefile.am b/Makefile.am > index a32e977..bfe9a6a 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -5,6 +5,7 @@ unstable_protocols = > \ > unstable/text-input/text-input-unstable-v1.xml > \ > unstable/input-method/input-method-unstable-v1.xml > \ > unstable/xdg-shell/xdg-shell-unstable-v5.xml > \ > + unstable/authorizer/authorizer-unstable-v1.xml > $(NULL) > > nobase_dist_pkgdata_DATA = > \ > diff --git a/unstable/restricted-registry/restricted-registry-v1.xml > b/unstable/restricted-registry/restricted-registry-v1.xml > new file mode 100644 > index 000..8c9224f > --- /dev/null > +++ b/unstable/restricted-registry/restricted-registry-v1.xml > @@ -0,0 +1,148 @@ > + > + > + > + > +Copyright © 2015 Giulio Camuffo. > + > +Permission to use, copy, modify, distribute, and sell this > +software and its documentation for any purpose is hereby granted > +without fee, provided that the above copyright notice appear in > +all copies and that both that copyright notice and this permission > +notice appear in supporting documentation, and that the name of > +the copyright holders not be used in advertising or publicity > +pertaining to distribution of the software without specific, > +written prior permission. The copyright holders make no > +representations about the suitability of this software for any > +purpose. It is provided "as is" without express or implied > +warranty. > + > +THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS > +SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND > +FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY > +SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > +WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN > +AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, > +ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF > +THIS SOFTWARE. > + > + > + > + > + This global interface acts as a wl_registry for restricted > interfaces. > + > + The globals advertized through this extension are not available > + through the normal wl_registry, and are not bindable by all the > + clients. > + When the clients creates a new zwp_restricted_registry_v1 object, > + it will advertize the existing restricted globals through the global > + event. These globals are not immediately bindable, but the client > + needs to be authorized first. To do so, the clients should send > + the authorize request for each global it whishes to bind, and then > + notify the compositor it whishes to be authorized by using the > + commit_authorization request. The compositor will then process the > + request and then send the list of authorized globals with the > + global_authorized event, followed by the authorization_done event. > + The client can bind the authorized globals when they are advertized > + but trying to bind any other will trigger a protocol error killing > + the client. > + > + The list of restricted interfaces is compositor de
Re: [PATCH] Introduce the authorizer protocol
Hi, On 24 November 2015 at 17:35, Giulio Camuffo <giuliocamu...@gmail.com> wrote: > 2015-11-24 18:16 GMT+02:00 Mariusz Ceier <mceier+wayl...@gmail.com>: > > Hi, > > How the clients will know: > > a) which interface is restricted and which is not ? > > It doesn't, but does it need to know it? It can still ask for > authorization even if the interface is not. > > I think it should - otherwise client, that wants to run on as many compositors as possible, would have to send authorize requests for every interface, since different compositors can implement different policies and interface in one compositor can be unrestricted while in another restricted. > > b) that the compositor implements restricted interface ? should they be > > visible in the registry ? > > Well, they are normal globals so they will are available in the > registry. If needed, this interface could > send out a list of the restricted ones, but i'm not sure it is. > > If they're normal globals - registry will contain a mix of restricted and unrestricted interfaces, and in current state clients won't know which is restricted and which is not, and binding to restricted interface will kill client - are we playing russian roulette ? ;) I think something like wl_registry but for restricted interfaces only, may be a better solution. Also since authorizer is another global, it would have to be announced before any restricted interface, right ? That requirement probably should be added to description. > > > If the client would like to use many restricted interfaces, it would > have to > > issue multiple authorize requests - probably causing compositor to show > many > > popups/notifications. I think it would be better to send list of > interfaces > > and receive back a list of interfaces for which access was granted > instead. > > That's a valid point... i don't think wl_array works for strings > though, so instead i'd add a way to group requests together. > > > -- > Giulio > > > > > Mariusz Ceier > > > > > > On 24 November 2015 at 16:16, Giulio Camuffo <giuliocamu...@gmail.com> > > wrote: > >> > >> This new extension is used by clients wanting to execute priviledged > >> actions such as taking a screenshot. > >> The usual way of granting special priviledged to apps is to fork and > >> exec them in the compositor, and then checking if the client is the > >> known one when it binds the restricted global interface. This works > >> but is quite limited, as it doesn't allow the compositor to ask the > >> user if the app is trusted, because it can't wait for the answer in > >> the bind function as that would block the compositor. > >> This new protocol instead allows the answer to come after some time > >> without blocking the compositor or the client. > >> --- > >> > >> For reference, i've implemented this in orbital[0] and it's used by > >> the screenshooter tool[1]. The name is different but it works exaclty > >> the same as this one. > >> One thing missing is how the revoke authorization, if even want/need it? > >> > >> 0: > >> > https://github.com/giucam/orbital/blob/master/src/compositor/authorizer.cpp > >> 1: > >> > https://github.com/giucam/orbital/blob/master/src/screenshooter/main.cpp#L301 > >> > >> > >> Makefile.am| 1 + > >> unstable/authorizer/authorizer-unstable-v1.xml | 90 > >> ++ > >> 2 files changed, 91 insertions(+) > >> create mode 100644 unstable/authorizer/authorizer-unstable-v1.xml > >> > >> diff --git a/Makefile.am b/Makefile.am > >> index a32e977..bfe9a6a 100644 > >> --- a/Makefile.am > >> +++ b/Makefile.am > >> @@ -5,6 +5,7 @@ unstable_protocols = > >> \ > >> unstable/text-input/text-input-unstable-v1.xml > >> \ > >> unstable/input-method/input-method-unstable-v1.xml > >> \ > >> unstable/xdg-shell/xdg-shell-unstable-v5.xml > >> \ > >> + unstable/authorizer/authorizer-unstable-v1.xml > >> $(NULL) > >> > >> nobase_dist_pkgdata_DATA = > >> \ > >> diff --git a/unstable/authorizer/authorizer-unstable-v1.xml > >> b/unstable/authorizer/authorizer-unstable-v1.xml > >> new file mode 100644 > >> index 000..f10dd0e > >> --- /dev/null > >> +++ b/unstable/authorizer/authorizer-unstable-v1.xml > >> @@ -0,0 +1,90 @@ > >> + > >> + > >> +
Re: [PATCH] Introduce the authorizer protocol
Hi, How the clients will know: a) which interface is restricted and which is not ? b) that the compositor implements restricted interface ? should they be visible in the registry ? If the client would like to use many restricted interfaces, it would have to issue multiple authorize requests - probably causing compositor to show many popups/notifications. I think it would be better to send list of interfaces and receive back a list of interfaces for which access was granted instead. Mariusz Ceier On 24 November 2015 at 16:16, Giulio Camuffo <giuliocamu...@gmail.com> wrote: > This new extension is used by clients wanting to execute priviledged > actions such as taking a screenshot. > The usual way of granting special priviledged to apps is to fork and > exec them in the compositor, and then checking if the client is the > known one when it binds the restricted global interface. This works > but is quite limited, as it doesn't allow the compositor to ask the > user if the app is trusted, because it can't wait for the answer in > the bind function as that would block the compositor. > This new protocol instead allows the answer to come after some time > without blocking the compositor or the client. > --- > > For reference, i've implemented this in orbital[0] and it's used by > the screenshooter tool[1]. The name is different but it works exaclty > the same as this one. > One thing missing is how the revoke authorization, if even want/need it? > > 0: > https://github.com/giucam/orbital/blob/master/src/compositor/authorizer.cpp > 1: > https://github.com/giucam/orbital/blob/master/src/screenshooter/main.cpp#L301 > > > Makefile.am| 1 + > unstable/authorizer/authorizer-unstable-v1.xml | 90 > ++ > 2 files changed, 91 insertions(+) > create mode 100644 unstable/authorizer/authorizer-unstable-v1.xml > > diff --git a/Makefile.am b/Makefile.am > index a32e977..bfe9a6a 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -5,6 +5,7 @@ unstable_protocols = > \ > unstable/text-input/text-input-unstable-v1.xml > \ > unstable/input-method/input-method-unstable-v1.xml > \ > unstable/xdg-shell/xdg-shell-unstable-v5.xml > \ > + unstable/authorizer/authorizer-unstable-v1.xml > $(NULL) > > nobase_dist_pkgdata_DATA = > \ > diff --git a/unstable/authorizer/authorizer-unstable-v1.xml > b/unstable/authorizer/authorizer-unstable-v1.xml > new file mode 100644 > index 000..f10dd0e > --- /dev/null > +++ b/unstable/authorizer/authorizer-unstable-v1.xml > @@ -0,0 +1,90 @@ > + > + > + > + > +Copyright © 2015 Giulio Camuffo. > + > +Permission to use, copy, modify, distribute, and sell this > +software and its documentation for any purpose is hereby granted > +without fee, provided that the above copyright notice appear in > +all copies and that both that copyright notice and this permission > +notice appear in supporting documentation, and that the name of > +the copyright holders not be used in advertising or publicity > +pertaining to distribution of the software without specific, > +written prior permission. The copyright holders make no > +representations about the suitability of this software for any > +purpose. It is provided "as is" without express or implied > +warranty. > + > +THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS > +SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND > +FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY > +SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES > +WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN > +AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, > +ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF > +THIS SOFTWARE. > + > + > + > + > + This global interface allows clients to ask the compositor the > + authorization to bind certain restricted global interfaces. > + Any client that aims to bind restricted interfaces should first > + request the authorization by using this interface. Failing to do > + so will result in the compositor sending a protocol error to the > + client when it binds the restricted interface. > + > + The list of restricted interfaces is compositor dependant, but must > + not include the core interfaces defined in wayland.xml. However, if > + an authorization request is done for a non-restricted interface the > + compositor must reply with a grant. > + > + > +
Re: [PATCH wayland-protocols 1/2] Introduce wp_relative_pointer interface
nd the > interface > + version number is reset. > + > I think that it would be better to link to README in wayland-protocols repository that describes this procedure in detail, rather than duplicating this in each protocol file. Also README doesn't define experimental protocols - instead it uses unstable term. Therefore, something like this might be better: "Warning! The protocol described in this file is unstable and backward incompatible changes may be made. See http://cgit.freedesktop.org/wayland/wayland-protocols/plain/README for details." > + > + > + > + Used by the client to notify the server that it will no longer use > this > + relative pointer manager object. > + > + > + > + > + > + Create a relative pointer interface given a wl_pointer object. See > the > + wp_relative_pointer interface for more details. > + > + > + > + > + > + > + > + > + > + A wp_relative_pointer object is an extension to the wl_pointer > interface > + used for emitting relative pointer events. It shares the same focus > as > + wl_pointer objects of the same seat and will only emit events when > it has > + focus. > + > + > + > + > + > + > + > + > + Relative x/y pointer motion from the pointer of the seat > associated with > + this object. > + > + A relative motion is in the same dimension as regular wl_pointer > motion > + events, except they do not represent an absolute position. For > example, > + moving a pointer from (x, y) to (x', y') would have the equivalent > + relative motion (x' - x, y' - y). If a pointer motion caused the > + absolute pointer position to be clipped by for example the edge of > the > + monitor, the relative motion is unaffected by the clipping and will > + represent the unclipped motion. > + > + This event also contains non-accelerated motion deltas. The > + non-accelerated delta is, when applicable, the regular pointer > motion > + delta as it was before having applied motion acceleration and other > + transformations such as normalization. > + > + Note that the non-accelerated delta does not represent 'raw' > events as > + they were read from some device. Pointer motion acceleration is > device- > + and configuration-specific and non-accelerated deltas and > accelerated > + deltas may have the same value on some devices. > + > + Relative motions are not coupled to wl_pointer.motion events, and > can be > + sent in combination with such events, but also independently. > There may > + also be scenarious where wl_pointer.motion is sent, but there is no > + relative motion. The order of an absolute and relative motion event > + originating from the same physical motion is not guaranteed. > + > + If the client needs button events or focus state, it can receive > them > + from a wl_pointer object of the same seat that the > wp_relative_pointer > + object is associated with. > + > + > + + summary="high 32 bits of a 64 bit timestamp with microsecond > granularity"/> > + + summary="low 32 bits of a 64 bit timestamp with microsecond > granularity"/> > + + summary="the x component of the motion vector"/> > + + summary="the y component of the motion vector"/> > + + summary="the x component of the unaccelerated motion vector"/> > + + summary="the y component of the unaccelerated motion vector"/> > + > + > + > + > -- > 2.4.3 > > Mariusz Ceier ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 00/10] weston wayland-protocols migration
Hi, Weston doesn't compile with this patchset - some source files in clients directory still use old or removed headers. I had to replace xdg-shell-server-protocol.h in desktop-shell/shell.c with xdg-shell-unstable-v5-server-protocol.h and disable clients compilation in order to be able to compile weston. If that's not an issue: Reviewed-by: Mariusz Ceier <mceier+wayl...@gmail.com> Mariusz Ceier On 4 November 2015 at 09:49, Jonas Ådahl <jad...@gmail.com> wrote: > Hi, > > This series changes weston to depend on wayland-protocols for the > majority of the protocols previously in the protocols/ directory. The > protocols moved are also renamed to comply with the unstable naming > conventions of wayland-protocols, with the exception of xdg_shell which > will use the current name until the next version. > > I'd appreciate if maintainers of given protocols could at least Ack the > patch changing their protocol implementation, as a semi formal stamp of > approval of the name change. > > Other than that, the workspaces protocol is removed, mostly because it > wasn't a significant enough proof of concept needed for any particular > feature. text-cursor-position.xml however I have left intact, because > without it the zoom accessibility feature proof of concept becomes > a bit too useless. I'd prefer to prefix it with something like toy_ or > weston_, but would like to hear input on this one. Given that it is > completely undocumented it is quite far from a real attempt, but it > seems like something that will be needed sooner or later for > accessibility reasons, so not sure what to do with it right now. > > Things that seemed more weston specific was weston_ prefixed. The > screenshooter protocol and the desktop shell protocol fell into this > category. > > Another protocol left to migrate is scalar.xml. I didn't do this yet > because Pekka had plans on renaming it, and I'll simply wait until he > has a name for it before migrating anything. > > Other than that, the wayland-protocols have shaped up some, with the > pointer gestures protocol as well as the protocols of this series > migrated there. I wouldn't mind if people went and had a look to see if > there are any opinions of how things should work before we make an > initial release. The procedures and some details are explained in the > README file in the toplevel directory. > > One current inconvenience that would be nice with some opinions is what > to do with unstable protocol files after a protocol has been declared > stable. Currently specified to work by having one directory containing > the stable XML file together with a README in the stable/ toplevel > directory, and yet another directory containing the unstable protocol > files together with a README in the unstable/ directory. This seems a bit > backward to me. Deprecating a protocol by moving it into deprecated/ > would still require us to have the protocol in the stable/ and probably > even in the unstable/ directory. This all is a bit awkward to me. One > idea is restructure the tree a bit and put protocols in the directory of > the current state (stable/unstable/obsolete) and then keep the old XML > files in subdirectories in there, having the Makefile deal with > installing XML files appropriately in the corresponding > stable/unstable/obsolete directories. Or we could have all protocols in > the toplevel directory and always have the corresponding XML files in > stable/unstable/obsolete subdirectories and just installing that. Any > opinions about this? > > > Jonas > > > Jonas Ådahl (10): > Use fullscreen-shell.xml from wayland-protocols > Use linux-dmabuf protocol from wayland-protocols > Use presentation timing protocol from wayland-protocols > Use text input protocol from wayland-protocols > Use input method protocol from wayland-protocols > Makefile.am: Make the external xml scanning rule version generic > Use xdg_shell protocol from wayland-protocols > desktop-shell: Rename protocol weston_desktop_shell > Rename screenshooter protocol to weston_screenshooter > Remove workspaces protocol > > Makefile.am | 194 ++-- > clients/desktop-shell.c | 79 ++--- > clients/editor.c| 119 +++ > clients/fullscreen.c| 60 ++-- > clients/keyboard.c | 186 +-- > clients/presentation-shm.c | 65 ++-- > clients/screenshot.c| 21 +- > clients/simple-damage.c | 18 +- > clients/simple-dmabuf.c | 74 ++--- > clients/simple-shm.c| 18 +- > clients/weston-info.c | 19 +- > clients/weston-simple-im.c | 122 +++ > cl
Re: Unstable protocol name breakage
Hi, On 20 October 2015 at 04:22, Jonas Ådahl <jad...@gmail.com> wrote: > Hi again, > > I was about to start migrating generic protocols away from weston into > wayland-protocols. The idea was to start with input-method.xml, text.xml, > linux-dmabuf.xml, presentation_timing.xml, scaler.xml and xdg-shell.xml. > The > question, however, is what to do with the names, because some names already > have the form "wl_[name]", and renaming such an interface to "zwl_[name]1" > during the unstable period, and then back to "wl_[name]" will cause > potential > breakage because some implementations in the wild might expect the > "wl_[name]" > to be the original (ancient) version. > > As mentioned before, I have already moved the fullscreen shell protocol, > and > with the naming schema changes in place, it ended up with the protocol name > "fullscreen-shell-unstable-v1", the interfaces zwl_fullscreen_shell1, and > zwl_fullscreen_shell_mode_feedback1. > > linux-dmabuf.xml is also easy. Since it is already 'z' prefix, to comply > with > the intended naming schema, I'd just need to rename the interfaces to > zlinux_dmabuf1 and zlinux_buffer_params1, and the protocol to > linux-dmabuf-unstable-v1. > > presentation_timing.xml: I suppose this one can be renamed without any > significant implications, since it currently is completely prefix free. I > imagine it'd be zwl_presentation1 and zwl_presentation_feedback1 in a > presentation-timing-unstable-v1(.xml) protocol. > > The problem is the rest of the protocols, since they all already have the > intended stable names. This means we cannot apply a naming schema that > intends > to finally remove the prefix and postfix when declaring stable, since that > would collide with the initial name. How to deal with these names needs to > be > decided, and probably so protocol by protocol. > > scalar.xml: As far as I know, Pekka has plans to change scalar.xml, and > plan to > do so with a name change. So as far as I understand, we need to rename this > one. > > input-method.xml: This one I think might actually be fine to just apply the > naming schema, as the protocol itself has Wayland core principle violations > that need to be solved, i.e. any implementor of this is already broken (by > principle). > > Since it's broken by principle, can't input-method.xml be marked as deprecated (e.g. by implementing/using top-level deprecated attribute ) ? Then leave it in weston as weston-specific protocol, that generates deprecation warnings during compilation and maybe when used by clients connecting to weston; and in wayland-protocols add new protocol that's not broken by principle (but may be based on input-method). Deprecation can also be used for other problematic protocols, but I'm not sure if that's good idea if such protocol is not broken. text.xml: This one I'm not so sure about. Has it ever been implemented > outside > of weston except only as a proof of concept? Would it be fine to use the > same > name? > > xdg-shell.xml: Should we bite the bullet and rename this one, or just > continue > letting its stability state be non-discoverable? It's clearly already > used, and > renaming it will be painful, so not sure about this one. > > Maybe we should at first stabilise protocols that are used, not broken and renaming them will be painful ? > Then comes the IVI protocols. I have no opinions about these, and I don't > know > what any plan with them might be. Should they be moved, or are they purely > a > weston thing? > > For the rest of the protocols (desktop-shell.xml, screenshooter.xml, > text-cursor-position.xml, weston-test.xml, workspaces.xml) I plan to leave > them > be, as they either are purely weston internal, simple toy protocols or > have no > consesus that they are to be ever be official protocols. > > So what should we do about these naming issues? It should have been clear > that > all of these are experimental protocols, but due to the fact that some may > have > started to use these outside of weston anyway even though they being > experimental, is it Ok for us to start causing them to break? If not, what > may > some alternative names be? > > > Jonas > Mariusz Ceier ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Unstable protocol name breakage
On 20 October 2015 at 05:59, Jonas Ådahl <jad...@gmail.com> wrote: > On Tue, Oct 20, 2015 at 05:26:45AM +0200, Mariusz Ceier wrote: > > Hi, > > > > On 20 October 2015 at 04:22, Jonas Ådahl <jad...@gmail.com> wrote: > > > > > Hi again, > > > > > > I was about to start migrating generic protocols away from weston into > > > wayland-protocols. The idea was to start with input-method.xml, > text.xml, > > > linux-dmabuf.xml, presentation_timing.xml, scaler.xml and > xdg-shell.xml. > > > The > > > question, however, is what to do with the names, because some names > already > > > have the form "wl_[name]", and renaming such an interface to > "zwl_[name]1" > > > during the unstable period, and then back to "wl_[name]" will cause > > > potential > > > breakage because some implementations in the wild might expect the > > > "wl_[name]" > > > to be the original (ancient) version. > > > > > > As mentioned before, I have already moved the fullscreen shell > protocol, > > > and > > > with the naming schema changes in place, it ended up with the protocol > name > > > "fullscreen-shell-unstable-v1", the interfaces zwl_fullscreen_shell1, > and > > > zwl_fullscreen_shell_mode_feedback1. > > > > > > linux-dmabuf.xml is also easy. Since it is already 'z' prefix, to > comply > > > with > > > the intended naming schema, I'd just need to rename the interfaces to > > > zlinux_dmabuf1 and zlinux_buffer_params1, and the protocol to > > > linux-dmabuf-unstable-v1. > > > > > > presentation_timing.xml: I suppose this one can be renamed without any > > > significant implications, since it currently is completely prefix > free. I > > > imagine it'd be zwl_presentation1 and zwl_presentation_feedback1 in a > > > presentation-timing-unstable-v1(.xml) protocol. > > > > > > The problem is the rest of the protocols, since they all already have > the > > > intended stable names. This means we cannot apply a naming schema that > > > intends > > > to finally remove the prefix and postfix when declaring stable, since > that > > > would collide with the initial name. How to deal with these names > needs to > > > be > > > decided, and probably so protocol by protocol. > > > > > > scalar.xml: As far as I know, Pekka has plans to change scalar.xml, and > > > plan to > > > do so with a name change. So as far as I understand, we need to rename > this > > > one. > > > > > > input-method.xml: This one I think might actually be fine to just > apply the > > > naming schema, as the protocol itself has Wayland core principle > violations > > > that need to be solved, i.e. any implementor of this is already broken > (by > > > principle). > > > > > > > > Since it's broken by principle, can't input-method.xml be marked as > > deprecated (e.g. by implementing/using top-level deprecated attribute ) ? > > Then leave it in weston as weston-specific protocol, that generates > > deprecation warnings during compilation and maybe when used by clients > > connecting to weston; and in wayland-protocols add new protocol that's > not > > broken by principle (but may be based on input-method). > > Not sure what you mean with top-level deprecated attribute (attributes > on some C code? or disable the text-backend by default whith a warning > if its enabled?). I meant adding deprecated attribute to top-level element in .xml file, that would instruct wayland scanner to generate deprecated attributes for every interface that's defined in this xml file. If we disable it by default we'd just break third > party clients (and can just as well move and rename), and I don't think > we should have compiler warning by default. > > I didn't mean disabling it - just generate deprecation warning at runtime (in weston) and compile time when client uses deprecated protocol. I don't know if it should be deprecated though; the broken-ness is that > it breaks the single-origin of object factories because it creates a > wl_keyboard, but that can be fixed. > > Will the fix most likely break existing clients ? If yes - do we care about that (since protocol is not yet stable, we don't have to) ? If yes - IMO current input-method should be deprecated, left in weston (as weston-specific) and new protocol created in wayland-protocols. In all other situations - input-method.xml should be fixed, 'z' prefix added and such modified protocol mov
Re: State of Wayland protocol development
; > > >> > [0] https://github.com/jadahl/wayland-protocols (only on github for > > >> > demonstration) > > >> > [1] > https://github.com/jadahl/weston/commit/caf37bb527740b5792260deaabc1ce33678351e5 > > >> > > >> The idea of the Weston patch looks good to me too. > > >> > > >> However, could we use git submodule to automate the fetching or a > > >> recent-enough revision of wayland-protocols? It might prevent some > > >> weston FTBFS questions, when we depend on unreleased versions. Or was > > >> the idea to release wayland-protocols often enough so we could always > > >> just rely on the pkg-config version check in configure.ac? That would > > >> be fine too. > > > > > > I don't think we should restrict ourself to using it as a git > submodule. > > > It would for example not be possible if you use Mercurial or some other > > > version control system (SDL comes to mind as an example of that). > > > Releases could be made on-demand, meaning it wouldn't be a problem for > > > weston to depend on a particular version. > > > > > > I'd also expect weston master to depend on wayland master as well as > > > wayland-protocols master, far that matter. > > > > Agreed, but indeed that doesn't preclude Weston from using it as a git > > submodule - just as long as we're considerate of other users who will > > need releases. > > True. Not that I see the actual point though (I only see the installing > being less tested). > > > > > Thanks a lot for doing this Jonas; it sounds good to me. How about we: > > - wait until Monday or Tuesday to see if anyone can pick concrete > > holes in this proposal > > AFAIU Wayland development is the least active on weekends so can just > wait for some more days so work-days contributors have the time to > react. > > - if none, I can create the repository then, with the usual Wayland ACL > > - patch Weston master to include protocols as a submodule > > - patch Weston to move its development protocols (xdg-shell, > > linux-dmabuf, presentation_timing, scaler) to protocols > > - go ahead and commit the protocols you and Carlos have been working > > on (gestures, pointer-lock/rel-pointer, new DnD) > > - document all of the above > > Sounds like a plan to me, except the DND changes poke at wayland.xml and > moving wayland.xml into wayland-protocols/ is not something I've thought > very much about. Should we? > > > > > I'll ack the Weston patches as long as they pass distcheck, and you > > can then just commit them directly. > > > > Cheers, > > Daniel > > > Jonas > Mariusz Ceier ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland 1/2] tests: C++ compilation test
Hi,
On 19 December 2014 at 15:20, Marek Chalupa mchqwe...@gmail.com wrote:
Hi,
git tells me that this patch adds white space error (new line before EOF)
On 15 December 2014 at 11:33, Mariusz Ceier mceier+wayl...@gmail.com
wrote:
This test includes one of wayland headers, which produced
error with C++ compiler. C compiler can't be used for this test,
because it issues only a warning[1] and only when wayland headers
are not installed in system headers path (/usr/include).
[1] wayland-server-protocol.h:201:2: warning: implicit declaration of
function ‘wl_resource_post_event’
Signed-off-by: Mariusz Ceier mceier+wayl...@gmail.com
---
Makefile.am| 4 +++-
configure.ac | 1 +
tests/cpp-compile-test.cpp | 5 +
3 files changed, 9 insertions(+), 1 deletion(-)
create mode 100644 tests/cpp-compile-test.cpp
diff --git a/Makefile.am b/Makefile.am
index 1551762..ea9ffc1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -128,7 +128,8 @@ TESTS = \
queue-test \
signal-test \
resources-test \
- message-test
+ message-test\
+ cpp-compile-test
check_PROGRAMS = \
$(TESTS)\
@@ -180,6 +181,7 @@ resources_test_SOURCES = tests/resources-test.c
resources_test_LDADD = libtest-runner.la
message_test_SOURCES = tests/message-test.c
message_test_LDADD = libtest-runner.la
+cpp_compile_test_SOURCES = tests/cpp-compile-test.cpp
fixed_benchmark_SOURCES = tests/fixed-benchmark.c
fixed_benchmark_LDADD = libtest-runner.la
diff --git a/configure.ac b/configure.ac
index 12dd94c..a5f7e61 100644
--- a/configure.ac
+++ b/configure.ac
@@ -25,6 +25,7 @@ AM_INIT_AUTOMAKE([1.11 foreign no-dist-gzip dist-xz
subdir-objects])
AM_SILENT_RULES([yes])
# Check for programs
+AC_PROG_CXX
I'm think Wayland should be capable to compile without errors even without
C++ compiler.
Shouldn't we have some checks here (later in the code) to disable the
cpp-test if the C++ compiler is not present in the environment?
I think that may be tricky, and I'm not sure how to do this properly.
This thread may be relevant:
http://lists.gnu.org/archive/html/bug-autoconf/2010-05/msg1.html
AC_PROG_CC
# Initialize libtool
diff --git a/tests/cpp-compile-test.cpp b/tests/cpp-compile-test.cpp
new file mode 100644
index 000..1e84e63
--- /dev/null
+++ b/tests/cpp-compile-test.cpp
@@ -0,0 +1,5 @@
+/* This source should compile fine with C++ compiler */
+#include wayland-server-protocol.h
+
+int main() { return 0; }
This should not be in-lined, but I think in this case it doesn't matter at
all, so OK.
+
--
2.1.3
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Thanks,
Marek
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [RFC wayland] cpp-test: check for C++ compiler
This won't work when there's C++ compiler but no 'which', but I think that's ok, since it only affects 1 test. Reviewed-by: Mariusz Ceier mceier+wayl...@gmail.com On 19 December 2014 at 17:40, Marek Chalupa mchqwe...@gmail.com wrote: Do not try to build it if no C++ compiler is present Signed-off-by: Marek Chalupa mchqwe...@gmail.com --- Makefile.am | 10 -- configure.ac | 13 - 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/Makefile.am b/Makefile.am index ea9ffc1..43b741a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -128,8 +128,11 @@ TESTS =\ queue-test \ signal-test \ resources-test \ - message-test\ - cpp-compile-test + message-test + +if ENABLE_CPP_TEST +TESTS += cpp-compile-test +endif check_PROGRAMS = \ $(TESTS)\ @@ -181,7 +184,10 @@ resources_test_SOURCES = tests/resources-test.c resources_test_LDADD = libtest-runner.la message_test_SOURCES = tests/message-test.c message_test_LDADD = libtest-runner.la + +if ENABLE_CPP_TEST cpp_compile_test_SOURCES = tests/cpp-compile-test.cpp +endif fixed_benchmark_SOURCES = tests/fixed-benchmark.c fixed_benchmark_LDADD = libtest-runner.la diff --git a/configure.ac b/configure.ac index a5f7e61..0822d39 100644 --- a/configure.ac +++ b/configure.ac @@ -25,8 +25,19 @@ AM_INIT_AUTOMAKE([1.11 foreign no-dist-gzip dist-xz subdir-objects]) AM_SILENT_RULES([yes]) # Check for programs -AC_PROG_CXX AC_PROG_CC +AC_PROG_CXX + +# check if we have C++ compiler. This is hacky workaround, +# for a reason why it is this way see +# http://lists.gnu.org/archive/html/bug-autoconf/2010-05/msg1.html +have_cpp_compiler=yes + +if ! which $CXX /dev/null; then + have_cpp_compiler=no +fi + +AM_CONDITIONAL(ENABLE_CPP_TEST, test x$have_cpp_compiler = xyes) # Initialize libtool LT_PREREQ([2.2]) -- 2.1.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland 2/2] scanner: Fix header generatation for server protocols
Server protocols headers should include wayland-server.h, instead of wayland-util.h. Otherwise they're not useable with C++ compiler unless wayland-server.h was included earlier. Signed-off-by: Mariusz Ceier mceier+wayl...@gmail.com --- src/scanner.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scanner.c b/src/scanner.c index ca03c57..1f1e59a 100644 --- a/src/scanner.c +++ b/src/scanner.c @@ -1002,7 +1002,7 @@ emit_header(struct protocol *protocol, enum side side) struct wl_resource;\n\n, protocol-uppercase_name, s, protocol-uppercase_name, s, - (side == SERVER) ? wayland-util.h : wayland-client.h); + (side == SERVER) ? wayland-server.h : wayland-client.h); wl_list_for_each(i, protocol-interface_list, link) printf(struct %s;\n, i-name); -- 2.1.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland 1/2] tests: C++ compilation test
This test includes one of wayland headers, which produced
error with C++ compiler. C compiler can't be used for this test,
because it issues only a warning[1] and only when wayland headers
are not installed in system headers path (/usr/include).
[1] wayland-server-protocol.h:201:2: warning: implicit declaration of function
‘wl_resource_post_event’
Signed-off-by: Mariusz Ceier mceier+wayl...@gmail.com
---
Makefile.am| 4 +++-
configure.ac | 1 +
tests/cpp-compile-test.cpp | 5 +
3 files changed, 9 insertions(+), 1 deletion(-)
create mode 100644 tests/cpp-compile-test.cpp
diff --git a/Makefile.am b/Makefile.am
index 1551762..ea9ffc1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -128,7 +128,8 @@ TESTS = \
queue-test \
signal-test \
resources-test \
- message-test
+ message-test\
+ cpp-compile-test
check_PROGRAMS = \
$(TESTS)\
@@ -180,6 +181,7 @@ resources_test_SOURCES = tests/resources-test.c
resources_test_LDADD = libtest-runner.la
message_test_SOURCES = tests/message-test.c
message_test_LDADD = libtest-runner.la
+cpp_compile_test_SOURCES = tests/cpp-compile-test.cpp
fixed_benchmark_SOURCES = tests/fixed-benchmark.c
fixed_benchmark_LDADD = libtest-runner.la
diff --git a/configure.ac b/configure.ac
index 12dd94c..a5f7e61 100644
--- a/configure.ac
+++ b/configure.ac
@@ -25,6 +25,7 @@ AM_INIT_AUTOMAKE([1.11 foreign no-dist-gzip dist-xz
subdir-objects])
AM_SILENT_RULES([yes])
# Check for programs
+AC_PROG_CXX
AC_PROG_CC
# Initialize libtool
diff --git a/tests/cpp-compile-test.cpp b/tests/cpp-compile-test.cpp
new file mode 100644
index 000..1e84e63
--- /dev/null
+++ b/tests/cpp-compile-test.cpp
@@ -0,0 +1,5 @@
+/* This source should compile fine with C++ compiler */
+#include wayland-server-protocol.h
+
+int main() { return 0; }
+
--
2.1.3
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland v2 2/2] scanner: Fix header generation for server protocols
Server protocols headers should include wayland-server.h, instead of wayland-util.h. Otherwise they're not useable with C++ compiler unless wayland-server.h was included earlier. Signed-off-by: Mariusz Ceier mceier+wayl...@gmail.com --- Notes: v2: s/generatation/generation/ src/scanner.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scanner.c b/src/scanner.c index ca03c57..1f1e59a 100644 --- a/src/scanner.c +++ b/src/scanner.c @@ -1002,7 +1002,7 @@ emit_header(struct protocol *protocol, enum side side) struct wl_resource;\n\n, protocol-uppercase_name, s, protocol-uppercase_name, s, - (side == SERVER) ? wayland-util.h : wayland-client.h); + (side == SERVER) ? wayland-server.h : wayland-client.h); wl_list_for_each(i, protocol-interface_list, link) printf(struct %s;\n, i-name); -- 2.1.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] Destroy resources when destroying an output
Hi,
I sent similar patch in the past, and this reply from Pekka may be
still relevant:
http://lists.freedesktop.org/archives/wayland-devel/2013-December/012691.html
Mariusz Ceier
On 8 May 2014 19:38, Neil Roberts n...@linux.intel.com wrote:
When an output is destroyed it now also destroys any resources that
were pointing to it. Otherwise if the resource is destroyed after the
output then the resource would try to remove itself from the resource
list but the head of the resource list would no longer be valid and it
would write to invalid memory.
This was found using Valgrind. It looks like there is a similar
problem for weston_pointer_destroy and in that case there is even a
comment suggesting we should do something about it.
https://bugs.freedesktop.org/show_bug.cgi?id=78415
---
src/compositor.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/compositor.c b/src/compositor.c
index cd1ca9a..4162315 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -3110,6 +3110,8 @@ weston_compositor_remove_output(struct
weston_compositor *compositor,
WL_EXPORT void
weston_output_destroy(struct weston_output *output)
{
+ struct wl_resource *resource, *tmp;
+
output-destroying = 1;
weston_compositor_remove_output(output-compositor, output);
@@ -3124,6 +3126,9 @@ weston_output_destroy(struct weston_output *output)
output-compositor-output_id_pool = ~(1 output-id);
wl_global_destroy(output-global);
+
+ wl_resource_for_each_safe(resource, tmp, output-resource_list)
+ wl_resource_destroy(resource);
}
static void
--
1.9.0
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] Don't include xserver-server-protocol.h
It was removed in 757d8aff2757387bcd594e2ee2a27055b366eac3,
along with xserver protocol.
Signed-off-by: Mariusz Ceier mceier+wayl...@gmail.com
---
xwayland/dnd.c| 1 -
xwayland/launcher.c | 1 -
xwayland/window-manager.c | 1 -
3 files changed, 3 deletions(-)
diff --git a/xwayland/dnd.c b/xwayland/dnd.c
index daeb08d..f0f2457 100644
--- a/xwayland/dnd.c
+++ b/xwayland/dnd.c
@@ -37,7 +37,6 @@
#include cairo-util.h
#include compositor.h
-#include xserver-server-protocol.h
#include hash.h
static void
diff --git a/xwayland/launcher.c b/xwayland/launcher.c
index f37cbe4..f0a87f5 100644
--- a/xwayland/launcher.c
+++ b/xwayland/launcher.c
@@ -33,7 +33,6 @@
#include signal.h
#include xwayland.h
-#include xserver-server-protocol.h
static int
diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index c8810a9..7a042e5 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -38,7 +38,6 @@
#include cairo-util.h
#include compositor.h
-#include xserver-server-protocol.h
#include hash.h
struct wm_size_hints {
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] [weston] Don't crash when eglCreateContext fails
eglCreateContext fails with every EGLConfig that nvidia blob 334.16 provides causing NULL pointer dereference in gl_renderer_destroy when destroying fragment and fan bindings. This should fix #74699. Signed-off-by: Mariusz Ceier mceier+wayl...@gmail.com --- src/gl-renderer.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/gl-renderer.c b/src/gl-renderer.c index d03bce6..76325f4 100644 --- a/src/gl-renderer.c +++ b/src/gl-renderer.c @@ -1766,8 +1766,10 @@ gl_renderer_destroy(struct weston_compositor *ec) wl_array_release(gr-vertices); wl_array_release(gr-vtxcnt); - weston_binding_destroy(gr-fragment_binding); - weston_binding_destroy(gr-fan_binding); + if (gr-fragment_binding) + weston_binding_destroy(gr-fragment_binding); + if (gr-fan_binding) + weston_binding_destroy(gr-fan_binding); free(gr); } -- 1.8.5.4 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] tests/resources-test: Fix wl_resource_post_event call
I think both patches (and original code too obviously) are wrong - WL_DISPLAY_ERROR (opcode 0) event has signature ous, so Quentin patch calls event with too many arguments and ignores types, and Marek patch calls event with too few arguments and incorrect type for the second argument. Imo, correct invocation should be either: wl_resource_post_event(res, WL_DISPLAY_ERROR, res, WL_DISPLAY_ERROR_INVALID_OBJECT,This error will be ignored); or wl_resource_post_event(res, WL_DISPLAY_ERROR, NULL, WL_DISPLAY_ERROR_INVALID_OBJECT,This error will be ignored); am I right ? On 6 January 2014 14:59, Marek Ch mchqwe...@gmail.com wrote: The call is wrong, I posted it here: http://lists.freedesktop.org/archives/wayland-devel/2013-November/012141.html But without any reaction. Good that somobody else renewed it :) On 05/01/2014, Quentin Glidic sardemff7+wayl...@sardemff7.net wrote: From: Quentin Glidic sardemff7+...@sardemff7.net Signed-off-by: Quentin Glidic sardemff7+...@sardemff7.net --- I do not know if this call is wrong of this the test just reveals a bug but without this, the wl_resource_post_event_array is called with random values tests/resources-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/resources-test.c b/tests/resources-test.c index d7a428a..2428964 100644 --- a/tests/resources-test.c +++ b/tests/resources-test.c @@ -108,7 +108,7 @@ TEST(destroy_res_tst) wl_resource_add_destroy_listener(res, destroy_listener); /* without implementation this should be ignored .. */ - wl_resource_post_event(res, 0); + wl_resource_post_event(res, 0, 0, 0, 0, 0); id = wl_resource_get_id(res); link = wl_resource_get_link(res); -- 1.8.5.2 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] Destroy resources when destroying input and output
Some structures containing resources list are freed before resources
on that list are destroyed, and that triggers invalid read/write
in compositor.c:3103 indirectly called from text-backend.c:938
when running weston under valgrind.
This patch destroys resources on these lists before such
structures are freed.
That fixes at least x11 and freerds backend.
Signed-off-by: Mariusz Ceier mceier+wayl...@gmail.com
---
src/compositor.c | 4
src/input.c | 15 ---
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/src/compositor.c b/src/compositor.c
index ff0f3ab..a4077e8 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -3183,6 +3183,7 @@ weston_compositor_verify_pointers(struct
weston_compositor *ec)
WL_EXPORT void
weston_output_destroy(struct weston_output *output)
{
+ struct wl_resource *resource, *next_resource;
output-destroying = 1;
weston_compositor_remove_output(output-compositor, output);
@@ -3192,6 +3193,9 @@ weston_output_destroy(struct weston_output *output)
wl_signal_emit(output-destroy_signal, output);
+ wl_resource_for_each_safe(resource, next_resource,
output-resource_list)
+ wl_resource_destroy(resource);
+
free(output-name);
pixman_region32_fini(output-region);
pixman_region32_fini(output-previous_damage);
diff --git a/src/input.c b/src/input.c
index 07e9d6c..062a2cb 100644
--- a/src/input.c
+++ b/src/input.c
@@ -471,10 +471,13 @@ weston_pointer_create(struct weston_seat *seat)
WL_EXPORT void
weston_pointer_destroy(struct weston_pointer *pointer)
{
+ struct wl_resource *resource, *next_resource;
+
if (pointer-sprite)
pointer_unmap_sprite(pointer);
- /* XXX: What about pointer-resource_list? */
+ wl_resource_for_each_safe(resource, next_resource,
pointer-resource_list)
+ wl_resource_destroy(resource);
wl_list_remove(pointer-focus_resource_listener.link);
wl_list_remove(pointer-focus_view_listener.link);
@@ -520,7 +523,7 @@ weston_xkb_info_destroy(struct weston_xkb_info *xkb_info);
WL_EXPORT void
weston_keyboard_destroy(struct weston_keyboard *keyboard)
{
- /* XXX: What about keyboard-resource_list? */
+ struct wl_resource *resource, *next_resource;
#ifdef ENABLE_XKBCOMMON
if (keyboard-seat-compositor-use_xkbcommon) {
@@ -533,6 +536,9 @@ weston_keyboard_destroy(struct weston_keyboard *keyboard)
}
#endif
+ wl_resource_for_each_safe(resource, next_resource,
keyboard-resource_list)
+ wl_resource_destroy(resource);
+
wl_array_release(keyboard-keys);
wl_list_remove(keyboard-focus_resource_listener.link);
free(keyboard);
@@ -570,7 +576,10 @@ weston_touch_create(void)
WL_EXPORT void
weston_touch_destroy(struct weston_touch *touch)
{
- /* XXX: What about touch-resource_list? */
+ struct wl_resource *resource, *next_resource;
+
+ wl_resource_for_each_safe(resource, next_resource,
touch-resource_list)
+ wl_resource_destroy(resource);
wl_list_remove(touch-focus_view_listener.link);
wl_list_remove(touch-focus_resource_listener.link);
--
1.8.5.2
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 1/1] Add a FreeRds backend, take 3
2) When using windows remote desktop client, weston exits after accepting connection due to SIGPIPE signal. Here's backtrace: ==31717== ==31717== Process terminating with default action of signal 13 (SIGPIPE) ==31717==at 0x6080AD0: __write_nocancel (syscall-template.S:81) ==31717==by 0x6A3C156: freerds_send_stream (compositor-freerds.c:130) ==31717==by 0x6A3C4A4: freerds_refresh_region (compositor-freerds.c:186) ==31717==by 0x6A3DA19: freerds_treat_message (compositor-freerds.c:821) ==31717==by 0x6A3DF0A: freerds_client_activity (compositor-freerds.c:920) ==31717==by 0x4E3D7F7: wl_event_source_fd_dispatch (event-loop.c:86) ==31717==by 0x4E3E1A8: wl_event_loop_dispatch (event-loop.c:421) ==31717==by 0x4E3BFEC: wl_display_run (wayland-server.c:961) ==31717==by 0x411C3F: main (compositor.c:4242) I can provide more detailed logs, if anyone is interested. Yes please send them. I uploaded them here: https://github.com/mceier/weston-logs These are complete logs from valgrind and gdb, for 4 different tests. Tests have (very) short description in README. Regards, thanks for testing and the feedback. -- David FORT website: http://www.hardening-consulting.com/ ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] Add missing return
Signed-off-by: Mariusz Ceier mceier+wayl...@gmail.com --- src/data-device.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/data-device.c b/src/data-device.c index ec3df33..8de5063 100644 --- a/src/data-device.c +++ b/src/data-device.c @@ -393,6 +393,7 @@ weston_seat_start_drag(struct weston_seat *seat, weston_pointer_set_focus(seat-pointer, NULL, wl_fixed_from_int(0), wl_fixed_from_int(0)); weston_pointer_start_grab(seat-pointer, drag-grab); + return 0; } static void -- 1.8.3.2 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] window: fix NULL pointer dereference
I used attached program to trigger it - it's modified fullscreen.c client
with 2 subsurfaces.
When I run it, press 'f' to make the main surface fullscreen and move
pointer over blue border, I get the crash.
On 16 July 2013 18:59, Rob Bradford robert.bradf...@intel.com wrote:
On 16 July 2013 01:25, Mariusz Ceier mceier+wayl...@gmail.com wrote:
NULL pointer dereference happens when input-focus_widget == NULL
and input-grab == NULL.
I worry this is working around the problem rather than addressing the
root cause. Why are we getting NULL for the focus_widget?
What steps did you take to generate a segfault here? Perhaps you could
open a bug
Rob
/*
* Copyright © 2008 Kristian Høgsberg
* Copyright © 2012 Intel Corporation
*
* Permission to use, copy, modify, distribute, and sell this software and its
* documentation for any purpose is hereby granted without fee, provided that
* the above copyright notice appear in all copies and that both that copyright
* notice and this permission notice appear in supporting documentation, and
* that the name of the copyright holders not be used in advertising or
* publicity pertaining to distribution of the software without specific,
* written prior permission. The copyright holders make no representations
* about the suitability of this software for any purpose. It is provided as
* is without express or implied warranty.
*
* THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
* INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
* EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
* CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
* DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
* TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
* OF THIS SOFTWARE.
*/
#include stdint.h
#include stdio.h
#include stdlib.h
#include stdarg.h
#include string.h
#include math.h
#include cairo.h
#include linux/input.h
#include wayland-client.h
#include window.h
struct fullscreen {
struct display *display;
struct window *window;
struct widget *widget;
struct widget *red_subsurface;
struct widget *green_subsurface;
int width, height;
int fullscreen;
float pointer_x, pointer_y;
enum wl_shell_surface_fullscreen_method fullscreen_method;
};
static void
fullscreen_handler(struct window *window, void *data)
{
struct fullscreen *fullscreen = data;
fullscreen-fullscreen ^= 1;
window_set_fullscreen(window, fullscreen-fullscreen);
}
static void
resize_handler(struct widget *widget, int width, int height, void *data)
{
struct fullscreen *fullscreen = data;
widget_set_size(widget, fullscreen-width, fullscreen-height);
}
static void
draw_string(cairo_t *cr,
const char *fmt, ...)
{
char buffer[4096];
char *p, *end;
va_list argp;
cairo_text_extents_t text_extents;
cairo_font_extents_t font_extents;
cairo_save(cr);
cairo_select_font_face(cr, sans,
CAIRO_FONT_SLANT_NORMAL,
CAIRO_FONT_WEIGHT_NORMAL);
cairo_set_font_size(cr, 14);
cairo_font_extents (cr, font_extents);
va_start(argp, fmt);
vsnprintf(buffer, sizeof(buffer), fmt, argp);
p = buffer;
while (*p) {
end = strchr(p, '\n');
if (end)
*end = 0;
cairo_show_text(cr, p);
cairo_text_extents (cr, p, text_extents);
cairo_rel_move_to (cr, -text_extents.x_advance, font_extents.height);
if (end)
p = end + 1;
else
break;
}
va_end(argp);
cairo_restore(cr);
}
static void
redraw_handler(struct widget *widget, void *data)
{
struct fullscreen *fullscreen = data;
struct rectangle allocation;
cairo_surface_t *surface;
cairo_t *cr;
int i;
double x, y, border;
const char *method_name[] = { default, scale, driver, fill };
surface = window_get_surface(fullscreen-window);
if (surface == NULL ||
cairo_surface_status(surface) != CAIRO_STATUS_SUCCESS) {
fprintf(stderr, failed to create cairo egl surface\n);
return;
}
widget_get_allocation(fullscreen-widget, allocation);
cr = widget_cairo_create(widget);
cairo_set_source_rgb(cr, 0, 0, 0);
cairo_paint (cr);
cairo_set_source_rgb(cr, 0, 0, 1);
cairo_set_line_width (cr, 10);
cairo_rectangle(cr, 5, 5, allocation.width - 10, allocation.height - 10);
cairo_stroke (cr);
cairo_move_to(cr,
allocation.x + 15,
allocation.y + 25);
cairo_set_source_rgb(cr, 1, 1, 1);
draw_string(cr,
Surface size: %d, %d\n
Scale: %d, transform: %d\n
Pointer: %f,%f\n
Fullscreen: %d, method: %s\n
Keys: (s)cale, (t)ransform, si(z)e, (m)ethod, (f)ullscreen, (q)uit\n,
fullscreen-width, fullscreen-height,
window_get_buffer_scale (fullscreen-window),
window_get_buffer_transform (fullscreen-window),
fullscreen-pointer_x, fullscreen-pointer_y,
fullscreen-fullscreen, method_name[fullscreen-fullscreen_method]);
y = 100;
i = 0;
while (y + 60 fullscreen-height) {
border = (i++ % 2 == 0
[PATCH weston] window: fix NULL pointer dereference
NULL pointer dereference happens when input-focus_widget == NULL
and input-grab == NULL.
Signed-off-by: Mariusz Ceier mceier+wayl...@gmail.com
---
clients/window.c | 17 ++---
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/clients/window.c b/clients/window.c
index cbfe12f..9933cb9 100644
--- a/clients/window.c
+++ b/clients/window.c
@@ -2819,14 +2819,17 @@ pointer_handle_motion(void *data, struct wl_pointer
*pointer,
widget = input-grab;
else
widget = input-focus_widget;
- if (widget widget-motion_handler)
- cursor = widget-motion_handler(input-focus_widget,
- input, time, sx, sy,
- widget-user_data);
- else
- cursor = input-focus_widget-default_cursor;
+ if (widget)
+ {
+ if (widget-motion_handler)
+ cursor = widget-motion_handler(input-focus_widget,
+ input, time, sx, sy,
+ widget-user_data);
+ else
+ cursor = widget-default_cursor;
+ input_set_pointer_image(input, cursor);
+ }
- input_set_pointer_image(input, cursor);
}
static void
--
1.8.3.2
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland] connection: Handle empty signature and signature with just a version.
Functions like wl_argument_from_va_list expect from get_next_argument, to initialize details-type but when the signature is empty or contains only version (like in desktop-shell-protocol.c in weston) it is left uninitialized. This patch fixes it, by initializing details-type with '\0' value, signaling end of arguments. Signed-off-by: Mariusz Ceier mceier+wayl...@gmail.com --- src/connection.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/connection.c b/src/connection.c index 2ca9bce..9bb850c 100644 --- a/src/connection.c +++ b/src/connection.c @@ -419,6 +419,7 @@ get_next_argument(const char *signature, struct argument_details *details) details-nullable = 1; } } + details-type = '\0'; return signature; } -- 1.8.3.2 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
