Re: [PATCH 2/2] compositor-wayland: Add touch support
At 2014-05-08 09:55:23, Jason Ekstrand ja...@jlekstrand.net wrote: Boyan, By and large, this looks really good! I have just a few comments below. As I said in another e-mail, I don't have any touch hardware I can test this on, so I wasn't able to actually test it. Unfortunately, I don't have touch hardware either. However I found a interesting project [1] on github, though I'm still figuring out how to use it. On Tue, May 6, 2014 at 9:46 PM, Boyan Ding stu_...@126.com wrote: Adding touch support to weston's nested wayland backend to make testing easier. https://bugs.freedesktop.org/show_bug.cgi?id=77769 --- src/compositor-wayland.c | 95 1 file changed, 95 insertions(+) diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c index a08b71a..45040d4 100644 --- a/src/compositor-wayland.c +++ b/src/compositor-wayland.c @@ -1582,6 +1582,91 @@ static const struct wl_keyboard_listener keyboard_listener = { }; static void +input_handle_touch_down(void *data, struct wl_touch *touch, uint32_t serial, + uint32_t time, struct wl_surface *surface, int32_t id, + wl_fixed_t x, wl_fixed_t y) +{ + struct wayland_input *input = data; + int32_t fx, fy; + + if (input-output-frame) { + frame_touch_down(input-output-frame, input, id, +wl_fixed_to_int(x), wl_fixed_to_int(y)); + frame_interior(input-output-frame, fx, fy, NULL, NULL); + x -= wl_fixed_from_int(fx); + y -= wl_fixed_from_int(fy); You probably want to handle FRAME_STATUS_MOVE here. See also input_handle_button. I saw the code handling FRAME_STATUS_MOVE in input_handle_button returned after handling that. Do we need to return here? Also, I saw the pointer code notify the event only if the location is THEME_LOCATION_CLIENT_AREA. Do we need to do the same thing? (I noticed the change of return type of frame_touch_down from void to theme_location in [2] but that patch hasn't been applied. + + if (frame_status(input-output-frame) FRAME_STATUS_REPAINT) + weston_output_schedule_repaint(input-output-base); + } + + weston_output_transform_coordinate(input-output-base, x, y, x, y); + + notify_touch(input-base, time, id, x, y, WL_TOUCH_DOWN); +} + +static void +input_handle_touch_up(void *data, struct wl_touch *touch, uint32_t serial, + uint32_t time, int32_t id) +{ + struct wayland_input *input = data; + + if (input-output-frame) { + frame_touch_up(input-output-frame, input, id); + You need to handle FRAME_STATUS_CLOSE here. See also input_handle_button. Perhaps, we want to add a wayland_output_handle_frame_status function to handle all of these so they can all be put in one place. If we want to do that, then it should probably be in it's own patch. + if (frame_status(input-output-frame) FRAME_STATUS_REPAINT) + weston_output_schedule_repaint(input-output-base); + } + + notify_touch(input-base, time, id, 0, 0, WL_TOUCH_UP); +} + +static void +input_handle_touch_motion(void *data, struct wl_touch *touch, uint32_t time, + int32_t id, wl_fixed_t x, wl_fixed_t y) +{ + struct wayland_input *input = data; + int32_t fx, fy; + + if (input-output-frame) { + frame_touch_motion(input-output-frame, input, id, +wl_fixed_to_int(x), wl_fixed_to_int(y)); + + frame_interior(input-output-frame, fx, fy, NULL, NULL); + x -= wl_fixed_from_int(fx); + y -= wl_fixed_from_int(fy); + + if (frame_status(input-output-frame) FRAME_STATUS_REPAINT) + weston_output_schedule_repaint(input-output-base); + } + + weston_output_transform_coordinate(input-output-base, x, y, x, y); + + notify_touch(input-base, time, id, x, y, WL_TOUCH_MOTION); +} + +static void +input_handle_touch_frame(void *data, struct wl_touch *touch) +{ + struct wayland_input *input = data; + + notify_touch_frame(input-base); +} + +static void +input_handle_touch_cancel(void *data, struct wl_touch *touch) +{ We should add a frame_touch_cancel function to frame.c and call it here. Also, we should probably add support for WL_TOUCH_CANCEL in notify_touch in input.c and call it here. If that's a little too involved for now, that's ok. Just call notify_touch(input-base, 0, -1, 0, 0, WL_TOUCH_CANCEL) here and we can implement it in notify_touch later. It's just a big switch statement, so this won't hurt anything. +} + +static const struct wl_touch_listener touch_listener = { + input_handle_touch_down, + input_handle_touch_up, + input_handle_touch_motion, + input_handle_touch_frame, + input_handle_touch_cancel +}; +
Re: [PATCH] Destroy resources when destroying input and output
On Thu, 8 May 2014 19:32:12 -0500 Jason Ekstrand ja...@jlekstrand.net wrote: On Tue, Dec 31, 2013 at 3:37 AM, Pekka Paalanen ppaala...@gmail.com wrote: You cannot just go and destroy wl_resources, because there are clients using them - that is why the wl_resources exist in the first place. Clients use all protocol objects they have, and if the protocol does not say an object is destroyed, then it must be legal to use it, regardless of what happens in the server. If you can guarantee, that clients know the protocol object has gone away without any races, then the clients have already destroyed the protocol objects themselves, which means that you have no wl_resources to destroy in the list. If you cannot guarantee that, then the protocol objects (wl_resources) must continue to exist in such a way, that clients do not get a fatal protocol error when using them. The objects just do nothing. This state is called inert. If this principle is broken, it usually leads to races between the server and the client, which sometimes leads to a fatal protocol error and causes disconnection of the client. This happens, when the server destroys a wl_resource, and then receives a request from the client for that destroyed resource. The client may have sent the request even before the wl_resource was destroyed in the server, at which point it was a legal request from the client's perspective (this is the essence of the race). However, since wl_output interface contains no requests, your patch might be right. A client cannot send a wl_output request after the wl_resource has been destroyed in the server, because the interface has no requests. The wl_output could still be used as an argument to other requests in other interfaces, though. Whether that is fatal, or just silently gets turned into NULL argument in libwayland-server, I do not remember off-hand. Would be best to verify what happens in that case, since it must not be fatal to the client. Yes, I just looked it up. If a client sends a request with the output as an argument after the output gets destroyed, the client will get an INVALID_OBJECT protocol error and disconnect. This is a problem. If this patch leads to a race with possibly a protocol error as the result (think about output hot-unplug), then I think you would need to turn the wl_resource into an inert object instead of destroying it directly. The way to do this would be to set dummy handler and call wl_resource_set_destructor(resource, NULL); This leaves the resource inert and allows us to destroy the underlying object. The resources will get cleaned up when the client quits. Looking at it in general, there is one more fun complication. If the inert object has requests in its interface, that create new objects, the server cannot just ignore those requests. I think the server will need to actually create new objects if requested, but make those inert too. Otherwise there would again be a mismatch between the server and the client on which objects exist. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH] event: Cheking for NULL before dereferencing the pointer.
Checking for NULL before dereferencing the wl_event_source pointer so as to avoid crash. Signed-off-by: Srivardhan Hebbar sri.heb...@samsung.com --- src/event-loop.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/event-loop.c b/src/event-loop.c index 9790cde..b62d16e 100644 --- a/src/event-loop.c +++ b/src/event-loop.c @@ -312,7 +312,12 @@ wl_event_source_check(struct wl_event_source *source) WL_EXPORT int wl_event_source_remove(struct wl_event_source *source) { - struct wl_event_loop *loop = source-loop; + struct wl_event_loop *loop; + + if (source == NULL) + return 0; + + loop = source-loop; /* We need to explicitly remove the fd, since closing the fd * isn't enough in case we've dup'ed the fd. */ -- 1.7.9.5 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Bug 78372 - create multiple windows with offset
On Thu, 08 May 2014 13:13:34 -0700 Bill Spitzak spit...@gmail.com wrote: I posted the same patch for the xserver for 6 months to allow it to read a different xorg.conf file. I finally got sick of being ignored and gave up, also recent xwayland can now work on a non-EGL wayland without having a special xorg.conf file, thus fixing it for me. However the underlying bug still exists and I'm sure there will be text that has to be in one xorg.conf file or the other that makes the other server choke. Maybe then somebody will fix it. Maybe you completely missed it, but the X server part of xwayland was rewritten almost from scratch. It does not use Xorg anymore at all. No DDXes. I'm not sure if it even tries to read any config files anymore. Now, the same source code tree that produces the familar 'Xorg' program and lots of modules, can also produce a program called 'Xwayland'. That single binary is now probably the only thing you need from the xserver tree for xwayland to work. See the new xwayland build instructions. For hardware accelerated GLX clients you might have to wait a bit more, but I see that glxgears works just fine - it gets llvmpipe. I didn't try to enable Glamor at all. With the very little effort I used, I'm in a state, where I can run 'glxgears' inside Weston, and it just works. - pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Bug 78372 - create multiple windows with offset
On Thu, 08 May 2014 10:52:51 -0700 Bill Spitzak spit...@gmail.com wrote: On 05/07/2014 10:54 PM, Pekka Paalanen wrote: This is similar to session save/restore, lacking a better term for it. We do not even pretend to support or enable this yet. It is just yet one more feature that the shell protocol suite for desktop should cover, but so far no-one has done any work on it AFAIK. If there is not one already, you are welcome to open a feature request bug about application layout save/restore. But the only practical method I can see is to allow the client to query and set the output and x/y position of any surface directly. You have just been starting at X11 for too long. Possibly you are reading the words save/restore literally, in that you are imagining some blob of data stored in the compositor that is recognized to restore the layout. However this is NOT what is wanted. Sure, that's the first thing comes to my mind. Let the client save a cookie or an encrypted blob, that it can pass back to the server when it creates the windows again. /handwaving/ When I say that I have not heard anyone working on it, it really means I do not have nor seen any plan *yet*. It is a complicated issue, which cannot be designed in an email, and especially not by me. Clients must be able to generate a usable layout the first time they are run, they must be able to do something intelligent if the set of windows changes from that in the saved layout, and use a layout from one system on another (including cross-platform), and be able to choose layouts from a list and add the current layout as a new item on the list. No. With separate windows you simply cannot do that. The client has no clue whatsoever on e.g. what positions on an output might be ok, what else is on the screen, or which outputs would be ok to conquer. You also might be on a tiling compositor, where your expectations simply totally fail. Your program would have to be a mind-reader to get the initial layout right for any user. Instead, what an application could do, is describe the expectations it has for its initial window layout, if possible. That is again left up for the desktop shell protocol suite to develop. If you want to be in absolute control of the layout (you the app, not even the user), use a single big window with compartments or something, maybe per output. If you think that sucks, I would probably agree. But I still cannot see how you could guess which output to pick for what, but that's probably just me never using special purpose monitors. - pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] event: Cheking for NULL before dereferencing the pointer.
Le 09/05/2014 08:43, Srivardhan Hebbar a écrit : Checking for NULL before dereferencing the wl_event_source pointer so as to avoid crash. Signed-off-by: Srivardhan Hebbar sri.heb...@samsung.com --- src/event-loop.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/event-loop.c b/src/event-loop.c index 9790cde..b62d16e 100644 --- a/src/event-loop.c +++ b/src/event-loop.c @@ -312,7 +312,12 @@ wl_event_source_check(struct wl_event_source *source) WL_EXPORT int wl_event_source_remove(struct wl_event_source *source) { - struct wl_event_loop *loop = source-loop; + struct wl_event_loop *loop; + + if (source == NULL) + return 0; + + loop = source-loop; /* We need to explicitly remove the fd, since closing the fd * isn't enough in case we've dup'ed the fd. */ Hello Srivardhan, do you have a case where this check is hit ? I may be wrong but having no loop associated with a source event is not supposed to happen. So my guess is that a caller of wl_event_source_remove has forgotten to nullify the event source after the call. Regards. -- 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
RE: [Weston] More discussion about weston output relative motion algorithm
Hi, Pq Thanks for your comment and idea. I list several cases for discussion. Case 1: Original: ├── │ B │ └── Action: Move A leftof B ├───┼───┤ │ A │ B │ └───┴───┘ As you said, B must not change, A will have negative coordinates. That is fine. Case 2: Original: ├───┼───┤ │ C │ D │E └───┴───┘ Move D leftof C: C must not change. How about E? E does not need change. ├───┼───┤ │ D │ C │ | E └───┴───┘ D will have negative coordinates. A big hole is left. It is not accepted by user at least I think so. An optimization is E will move left and is side with C ├───┼───┤ │ D │ C │E └───┴───┘ Case 3: ├───┼───┤ │ C │ D │E | F └───┴───┘ Action: Move E leftof D C, D's coordinate will not change. E and C will have the same relation with D, however they are not clone. F will be move forward. Right? ├───┼───┤ │C/E│D | F └───┴───┘ Case 4: ├───┼───┤ │ C │ D │E | F └───┴───┘ ││ │ G | └───┴───┘ Action: Move E leftof D C, D's coordinate will not change. E and C will have the same relation with D. F will be move forward. The relation between F and G will be built up. Maybe you want G to be moved with E. That will be crazy. ├───┼───┤ │C/E│D | F | └───┴───┘ │ ││ G| └───┴───┘ After the cases above, list a complex one. ├───┼───┤ │ │H | I | └───┴───┘ │ ││ G| ├───┼───┤ │C/E│D | F | K/L│M └───┴─┘ │ ││J || | N| └───┴───┘ Move F leftof H, and find some output to take F's position. Then chain reaction will happen. Here you will define an order who take the place. For example, K/L will take the place of F. What is the relation between G and K or L? what is the relation between J and K or L? Things turns to be much more complex. Just have a think what will be. It is an interesting thing. After that, you find a conclusion. 1) position to new place will be easy. 2) find replacer and rebuild the relations will be complex. 3) Chain reaction will make you crazy. Because no one expect how many outputs will be available and how many links after that. Maybe 1, or 2 or 3 or more... It is very easy to be in a loop. And more hole in layout happens. Anyway, it is a little hard to make everything happy. :) Thanks Regards Quanxian Wang ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Bug 78372 - create multiple windows with offset
Hi, On 9 May 2014 08:34, Pekka Paalanen ppaala...@gmail.com wrote: On Thu, 08 May 2014 10:52:51 -0700 Bill Spitzak spit...@gmail.com wrote: Possibly you are reading the words save/restore literally, in that you are imagining some blob of data stored in the compositor that is recognized to restore the layout. However this is NOT what is wanted. Sure, that's the first thing comes to my mind. Let the client save a cookie or an encrypted blob, that it can pass back to the server when it creates the windows again. /handwaving/ When I say that I have not heard anyone working on it, it really means I do not have nor seen any plan *yet*. It is a complicated issue, which cannot be designed in an email, and especially not by me. I wouldn't say it's that much of a handwave: it's actually exactly what we came up with a couple of years ago when we were talking about session management and persistent state. Seemed to be the best option indeed. But, like you say, no-one's really approached it yet. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] desktop-shell: Fix black edges on scaled desktop pattern
On Thu, 08 May 2014 20:00:35 -0700 Bill Spitzak spit...@gmail.com wrote: Filter sampling outside the source image can leak black into the edges of the desktop image. This is most easily seen by scaling the default tiled image with this weston.ini: # no background-image and no background-color background-type=scale-crop --- clients/desktop-shell.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c index 4880888..e121cc7 100644 --- a/clients/desktop-shell.c +++ b/clients/desktop-shell.c @@ -724,6 +724,7 @@ background_draw(struct widget *widget, void *data) case BACKGROUND_SCALE: cairo_matrix_init_scale(matrix, sx, sy); cairo_pattern_set_matrix(pattern, matrix); + cairo_pattern_set_extend(pattern, CAIRO_EXTEND_PAD); break; case BACKGROUND_SCALE_CROP: s = (sx sy) ? sx : sy; @@ -733,6 +734,7 @@ background_draw(struct widget *widget, void *data) cairo_matrix_init_translate(matrix, tx, ty); cairo_matrix_scale(matrix, s, s); cairo_pattern_set_matrix(pattern, matrix); + cairo_pattern_set_extend(pattern, CAIRO_EXTEND_PAD); break; case BACKGROUND_TILE: cairo_pattern_set_extend(pattern, CAIRO_EXTEND_REPEAT); Ok, I see the problem, and I see this fix should be good, but for some reason this patch does not apply at all. I had to do the change manually. Anyway, it works right and fixes the issue. Hm, it does apply with --ignore-whitespace, though. When I save the raw email, there seems to be extra spaces in the beginning of all context lines. Maybe Format=flowed messes it up? Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
RE: [PATCH] event: Cheking for NULL before dereferencing the pointer.
-Original Message- From: wayland-devel [mailto:wayland-devel- boun...@lists.freedesktop.org] On Behalf Of Hardening Sent: Friday, May 09, 2014 1:08 PM To: wayland-devel@lists.freedesktop.org Subject: Re: [PATCH] event: Cheking for NULL before dereferencing the pointer. Le 09/05/2014 08:43, Srivardhan Hebbar a écrit : Checking for NULL before dereferencing the wl_event_source pointer so as to avoid crash. Signed-off-by: Srivardhan Hebbar sri.heb...@samsung.com --- src/event-loop.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/event-loop.c b/src/event-loop.c index 9790cde..b62d16e 100644 --- a/src/event-loop.c +++ b/src/event-loop.c @@ -312,7 +312,12 @@ wl_event_source_check(struct wl_event_source *source) WL_EXPORT int wl_event_source_remove(struct wl_event_source *source) { - struct wl_event_loop *loop = source-loop; + struct wl_event_loop *loop; + + if (source == NULL) + return 0; + + loop = source-loop; /* We need to explicitly remove the fd, since closing the fd * isn't enough in case we've dup'ed the fd. */ Hello Srivardhan, do you have a case where this check is hit ? I may be wrong but having no loop associated with a source event is not supposed to happen. So my guess is that a caller of wl_event_source_remove has forgotten to nullify the event source after the call. Hi, I was going through the code of Weston. In the main function in compositor.c wl_event_loop_add_signal() is called to allocate the memory for signals[](Line no: 4196. struct wl_event_source *signals[4]). The function returns NULL if memory allocation failed. After that there is no NULL check for 'signals'. In the end while clearing up, this function is called. So if memory allocation failed then a NULL is passed to this function. Hence adding code to check for the NULL. Thank-you, Hebbar Regards. -- 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 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] event: Cheking for NULL before dereferencing the pointer.
On Fri, 09 May 2014 14:56:14 +0530 Srivardhan sri.heb...@samsung.com wrote: -Original Message- From: wayland-devel [mailto:wayland-devel- boun...@lists.freedesktop.org] On Behalf Of Hardening Sent: Friday, May 09, 2014 1:08 PM To: wayland-devel@lists.freedesktop.org Subject: Re: [PATCH] event: Cheking for NULL before dereferencing the pointer. Le 09/05/2014 08:43, Srivardhan Hebbar a écrit : Checking for NULL before dereferencing the wl_event_source pointer so as to avoid crash. Signed-off-by: Srivardhan Hebbar sri.heb...@samsung.com --- src/event-loop.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/event-loop.c b/src/event-loop.c index 9790cde..b62d16e 100644 --- a/src/event-loop.c +++ b/src/event-loop.c @@ -312,7 +312,12 @@ wl_event_source_check(struct wl_event_source *source) WL_EXPORT int wl_event_source_remove(struct wl_event_source *source) { - struct wl_event_loop *loop = source-loop; + struct wl_event_loop *loop; + + if (source == NULL) + return 0; + + loop = source-loop; /* We need to explicitly remove the fd, since closing the fd * isn't enough in case we've dup'ed the fd. */ Hello Srivardhan, do you have a case where this check is hit ? I may be wrong but having no loop associated with a source event is not supposed to happen. So my guess is that a caller of wl_event_source_remove has forgotten to nullify the event source after the call. Hi, I was going through the code of Weston. In the main function in compositor.c wl_event_loop_add_signal() is called to allocate the memory for signals[](Line no: 4196. struct wl_event_source *signals[4]). The function returns NULL if memory allocation failed. After that there is no NULL check for 'signals'. In the end while clearing up, this function is called. So if memory allocation failed then a NULL is passed to this function. Hence adding code to check for the NULL. Hi, I think we should be fixing the caller instead of wl_event_source_remove(). I do not believe NULL is a valid argument for it, so the bug is in the caller. If any wl_event_loop_add_signal() in Weston fails, it would be a reason to exit with an error. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
RE: [PATCH] event: Cheking for NULL before dereferencing the pointer.
-Original Message- From: Pekka Paalanen [mailto:ppaala...@gmail.com] Sent: Friday, May 09, 2014 3:09 PM To: Srivardhan Cc: 'Hardening'; wayland-devel@lists.freedesktop.org Subject: Re: [PATCH] event: Cheking for NULL before dereferencing the pointer. On Fri, 09 May 2014 14:56:14 +0530 Srivardhan sri.heb...@samsung.com wrote: -Original Message- From: wayland-devel [mailto:wayland-devel- boun...@lists.freedesktop.org] On Behalf Of Hardening Sent: Friday, May 09, 2014 1:08 PM To: wayland-devel@lists.freedesktop.org Subject: Re: [PATCH] event: Cheking for NULL before dereferencing the pointer. Le 09/05/2014 08:43, Srivardhan Hebbar a écrit : Checking for NULL before dereferencing the wl_event_source pointer so as to avoid crash. Signed-off-by: Srivardhan Hebbar sri.heb...@samsung.com --- src/event-loop.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/event-loop.c b/src/event-loop.c index 9790cde..b62d16e 100644 --- a/src/event-loop.c +++ b/src/event-loop.c @@ -312,7 +312,12 @@ wl_event_source_check(struct wl_event_source *source) WL_EXPORT int wl_event_source_remove(struct wl_event_source *source) { - struct wl_event_loop *loop = source-loop; + struct wl_event_loop *loop; + + if (source == NULL) + return 0; + + loop = source-loop; /* We need to explicitly remove the fd, since closing the fd * isn't enough in case we've dup'ed the fd. */ Hello Srivardhan, do you have a case where this check is hit ? I may be wrong but having no loop associated with a source event is not supposed to happen. So my guess is that a caller of wl_event_source_remove has forgotten to nullify the event source after the call. Hi, I was going through the code of Weston. In the main function in compositor.c wl_event_loop_add_signal() is called to allocate the memory for signals[](Line no: 4196. struct wl_event_source *signals[4]). The function returns NULL if memory allocation failed. After that there is no NULL check for 'signals'. In the end while clearing up, this function is called. So if memory allocation failed then a NULL is passed to this function. Hence adding code to check for the NULL. Hi, I think we should be fixing the caller instead of wl_event_source_remove(). I do not believe NULL is a valid argument for it, so the bug is in the caller. If any wl_event_loop_add_signal() in Weston fails, it would be a reason to exit with an error. Oh... k... Will fix that and send the patch... In general isn't it fine to check for NULL before dereferencing? Thank-you, Hebbar Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] event: Cheking for NULL before dereferencing the pointer.
On Fri, 09 May 2014 15:21:51 +0530 Srivardhan sri.heb...@samsung.com wrote: -Original Message- From: Pekka Paalanen [mailto:ppaala...@gmail.com] Sent: Friday, May 09, 2014 3:09 PM To: Srivardhan Cc: 'Hardening'; wayland-devel@lists.freedesktop.org Subject: Re: [PATCH] event: Cheking for NULL before dereferencing the pointer. On Fri, 09 May 2014 14:56:14 +0530 Srivardhan sri.heb...@samsung.com wrote: -Original Message- From: wayland-devel [mailto:wayland-devel- boun...@lists.freedesktop.org] On Behalf Of Hardening Sent: Friday, May 09, 2014 1:08 PM To: wayland-devel@lists.freedesktop.org Subject: Re: [PATCH] event: Cheking for NULL before dereferencing the pointer. Le 09/05/2014 08:43, Srivardhan Hebbar a écrit : Checking for NULL before dereferencing the wl_event_source pointer so as to avoid crash. Signed-off-by: Srivardhan Hebbar sri.heb...@samsung.com --- src/event-loop.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/event-loop.c b/src/event-loop.c index 9790cde..b62d16e 100644 --- a/src/event-loop.c +++ b/src/event-loop.c @@ -312,7 +312,12 @@ wl_event_source_check(struct wl_event_source *source) WL_EXPORT int wl_event_source_remove(struct wl_event_source *source) { - struct wl_event_loop *loop = source-loop; + struct wl_event_loop *loop; + + if (source == NULL) + return 0; + + loop = source-loop; /* We need to explicitly remove the fd, since closing the fd * isn't enough in case we've dup'ed the fd. */ Hello Srivardhan, do you have a case where this check is hit ? I may be wrong but having no loop associated with a source event is not supposed to happen. So my guess is that a caller of wl_event_source_remove has forgotten to nullify the event source after the call. Hi, I was going through the code of Weston. In the main function in compositor.c wl_event_loop_add_signal() is called to allocate the memory for signals[](Line no: 4196. struct wl_event_source *signals[4]). The function returns NULL if memory allocation failed. After that there is no NULL check for 'signals'. In the end while clearing up, this function is called. So if memory allocation failed then a NULL is passed to this function. Hence adding code to check for the NULL. Hi, I think we should be fixing the caller instead of wl_event_source_remove(). I do not believe NULL is a valid argument for it, so the bug is in the caller. If any wl_event_loop_add_signal() in Weston fails, it would be a reason to exit with an error. Oh... k... Will fix that and send the patch... In general isn't it fine to check for NULL before dereferencing? Checking is one thing, silently hiding bugs is another thing. If NULL is a legal input, then of course it needs to be checked. If NULL can happen, but is a runtime error, the program needs to be vocal about it, e.g. relay the error back to the caller. If API specification says NULL is not a valid input, putting an assert() would be fine, since violating that is a programmer error in the caller. I think wl_event_source_remove() falls into the last category. All functions in wayland-util.h belong to this category, too. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] rpi: build fix for compute_rects debug
From: Pekka Paalanen pekka.paala...@collabora.co.uk See 918f2dd4cfb8b177f67b45653efbbe4325cbe9dc Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk --- src/rpi-renderer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rpi-renderer.c b/src/rpi-renderer.c index 3a7f65c..c222eb6 100644 --- a/src/rpi-renderer.c +++ b/src/rpi-renderer.c @@ -858,8 +858,8 @@ rpir_view_compute_rects(struct rpir_view *view, src_height = int_max(src_height, 0); DBG(rpir_view %p %dx%d: p1 %f, %f; p2 %f, %f\n, view, - view-view-geometry.width, - view-view-geometry.height, + view-view-surface-width, + view-view-surface-height, p1.f[0], p1.f[1], p2.f[0], p2.f[1]); DBG(src rect %d;%d, %d;%d, %d;%dx%d;%d\n, src_x 16, src_x 0x, -- 1.8.5.5 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] event: Cheking for NULL before dereferencing the pointer.
Le 09/05/2014 12:20, Pekka Paalanen a écrit : On Fri, 09 May 2014 15:21:51 +0530 Srivardhan sri.heb...@samsung.com wrote: -Original Message- From: Pekka Paalanen [mailto:ppaala...@gmail.com] Sent: Friday, May 09, 2014 3:09 PM To: Srivardhan Cc: 'Hardening'; wayland-devel@lists.freedesktop.org Subject: Re: [PATCH] event: Cheking for NULL before dereferencing the pointer. On Fri, 09 May 2014 14:56:14 +0530 Srivardhan sri.heb...@samsung.com wrote: [...] Checking is one thing, silently hiding bugs is another thing. If NULL is a legal input, then of course it needs to be checked. If NULL can happen, but is a runtime error, the program needs to be vocal about it, e.g. relay the error back to the caller. If API specification says NULL is not a valid input, putting an assert() would be fine, since violating that is a programmer error in the caller. I think wl_event_source_remove() falls into the last category. All functions in wayland-util.h belong to this category, too. IMHO wl_event_source_remove() should take a wl_event_source ** as parameter and set to NULL the event_source pointer (preventing anyone to use it). Using eclipse call hierarchy, I've seen many places where this extra precaution is not taken. I don't know if wl_event_source_remove() can be considered as part of the libwayland API and so fixed in stone ? Regards. -- 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 weston] vaapi-recorder: Don't loop trying to write on out of space condition
From: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com The error handling for the function that writes the encoded frame on the disk was bogus, always assuming the buffer supplied to the encoder was too small. That would cause a bigger buffer to be allocated and another attempt to encode the frame was done. In the case of a failure to write to disk (due to ENOSPC, for instance) that would cause an endless loop. --- Possibly related to https://bugs.freedesktop.org/show_bug.cgi?id=69330 --- src/compositor-drm.c | 27 +++ src/vaapi-recorder.c | 42 +- src/vaapi-recorder.h | 2 +- 3 files changed, 53 insertions(+), 18 deletions(-) diff --git a/src/compositor-drm.c b/src/compositor-drm.c index 5f59789..7d514e4 100644 --- a/src/compositor-drm.c +++ b/src/compositor-drm.c @@ -2558,6 +2558,18 @@ planes_binding(struct weston_seat *seat, uint32_t time, uint32_t key, void *data #ifdef BUILD_VAAPI_RECORDER static void +recorder_destroy(struct drm_output *output) +{ + vaapi_recorder_destroy(output-recorder); + output-recorder = NULL; + + output-base.disable_planes--; + + wl_list_remove(output-recorder_frame_listener.link); + weston_log([libva recorder] done\n); +} + +static void recorder_frame_notify(struct wl_listener *listener, void *data) { struct drm_output *output; @@ -2579,7 +2591,12 @@ recorder_frame_notify(struct wl_listener *listener, void *data) return; } - vaapi_recorder_frame(output-recorder, fd, output-current-stride); + ret = vaapi_recorder_frame(output-recorder, fd, + output-current-stride); + if (ret 0) { + weston_log([libva recorder] aborted: %m\n); + recorder_destroy(output); + } } static void * @@ -2637,13 +2654,7 @@ recorder_binding(struct weston_seat *seat, uint32_t time, uint32_t key, weston_log([libva recorder] initialized\n); } else { - vaapi_recorder_destroy(output-recorder); - output-recorder = NULL; - - output-base.disable_planes--; - - wl_list_remove(output-recorder_frame_listener.link); - weston_log([libva recorder] done\n); + recorder_destroy(output); } } #else diff --git a/src/vaapi-recorder.c b/src/vaapi-recorder.c index 0095a42..921494d 100644 --- a/src/vaapi-recorder.c +++ b/src/vaapi-recorder.c @@ -50,6 +50,7 @@ #include string.h #include unistd.h #include assert.h +#include errno.h #include sys/types.h #include sys/stat.h @@ -93,6 +94,7 @@ struct vaapi_recorder { int width, height; int frame_count; + int error; int destroying; pthread_t worker_thread; pthread_mutex_t mutex; @@ -761,7 +763,13 @@ encoder_create_output_buffer(struct vaapi_recorder *r) return VA_INVALID_ID; } -static int +enum output_write_status { + OUTPUT_WRITE_SUCCESS, + OUTPUT_WRITE_OVERFLOW, + OUTPUT_WRITE_FATAL +}; + +static enum output_write_status encoder_write_output(struct vaapi_recorder *r, VABufferID output_buf) { VACodedBufferSegment *segment; @@ -770,19 +778,22 @@ encoder_write_output(struct vaapi_recorder *r, VABufferID output_buf) status = vaMapBuffer(r-va_dpy, output_buf, (void **) segment); if (status != VA_STATUS_SUCCESS) - return -1; + return OUTPUT_WRITE_FATAL; if (segment-status VA_CODED_BUF_STATUS_SLICE_OVERFLOW_MASK) { r-encoder.output_size *= 2; vaUnmapBuffer(r-va_dpy, output_buf); - return -1; + return OUTPUT_WRITE_OVERFLOW; } count = write(r-output_fd, segment-buf, segment-size); vaUnmapBuffer(r-va_dpy, output_buf); - return count; + if (count 0) + return OUTPUT_WRITE_FATAL; + + return OUTPUT_WRITE_SUCCESS; } static void @@ -792,9 +803,8 @@ encoder_encode(struct vaapi_recorder *r, VASurfaceID input) VABufferID buffers[8]; int count = 0; - - int slice_type; - int ret, i; + int i, slice_type; + enum output_write_status ret; if ((r-frame_count % r-encoder.intra_period) == 0) slice_type = SLICE_TYPE_I; @@ -829,7 +839,10 @@ encoder_encode(struct vaapi_recorder *r, VASurfaceID input) output_buf = VA_INVALID_ID; vaDestroyBuffer(r-va_dpy, buffers[--count]); - } while (ret 0); + } while (ret == OUTPUT_WRITE_OVERFLOW); + + if (ret == OUTPUT_WRITE_FATAL) + r-error = errno; for (i = 0; i count; i++) vaDestroyBuffer(r-va_dpy, buffers[i]); @@ -1138,11 +1151,19 @@ worker_thread_function(void *data) return NULL; } -void +int vaapi_recorder_frame(struct vaapi_recorder *r, int prime_fd, int
Re: [PATCH] event: Cheking for NULL before dereferencing the pointer.
On Fri, 09 May 2014 14:50:19 +0200 Hardening rdp.eff...@gmail.com wrote: Le 09/05/2014 12:20, Pekka Paalanen a écrit : On Fri, 09 May 2014 15:21:51 +0530 Srivardhan sri.heb...@samsung.com wrote: -Original Message- From: Pekka Paalanen [mailto:ppaala...@gmail.com] Sent: Friday, May 09, 2014 3:09 PM To: Srivardhan Cc: 'Hardening'; wayland-devel@lists.freedesktop.org Subject: Re: [PATCH] event: Cheking for NULL before dereferencing the pointer. On Fri, 09 May 2014 14:56:14 +0530 Srivardhan sri.heb...@samsung.com wrote: [...] Checking is one thing, silently hiding bugs is another thing. If NULL is a legal input, then of course it needs to be checked. If NULL can happen, but is a runtime error, the program needs to be vocal about it, e.g. relay the error back to the caller. If API specification says NULL is not a valid input, putting an assert() would be fine, since violating that is a programmer error in the caller. I think wl_event_source_remove() falls into the last category. All functions in wayland-util.h belong to this category, too. IMHO wl_event_source_remove() should take a wl_event_source ** as parameter and set to NULL the event_source pointer (preventing anyone to use it). Using eclipse call hierarchy, I've seen many places where this extra precaution is not taken. I don't know if wl_event_source_remove() can be considered as part of the libwayland API and so fixed in stone ? If it is exported in a release, it is set in stone. And so it is. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] Destroy resources when destroying input and output
Perhaps we should consider applying the patch anyway even though it's not ideal. Currently if a client uses a dead output in a request such as xdg_surface.set_output Weston will end up with a weston_output pointer that points to freed memory. This could cause the compositor to crash. That is worse than the alternative provided by this patch which is to make the client abort. The clients know about the output being destroyed via the wl_registry.global_remove event so in practice they would only hit the problem in the unlikely event that they used the output in a request in the short time between the output being unplugged and noticing the removal event. In the longer term I was thinking maybe it would be good to handle the inert resource idea within libwayland-server. We could add a function like wl_resource_zombify() which would mark the resource as a zombie and call the destroy handlers. From the compositor's perspective it can then act as if the resource has been destroyed. We could detect zombie resources being used within the request marshalling code and ignore the request. If the request creates new resource we can internally create new zombie resources too and Weston would never need to know about it. I am planning to experiment with this approach now. Regards, - Neil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] Destroy resources when destroying input and output
On Fri, 09 May 2014 14:33:58 +0100 Neil Roberts n...@linux.intel.com wrote: Perhaps we should consider applying the patch anyway even though it's not ideal. Currently if a client uses a dead output in a request such as xdg_surface.set_output Weston will end up with a weston_output pointer that points to freed memory. This could cause the compositor to crash. True, it's very bad now. That is worse than the alternative provided by this patch which is to make the client abort. The clients know about the output being destroyed via the wl_registry.global_remove event so in practice they would only hit the problem in the unlikely event that they used the output in a request in the short time between the output being unplugged and noticing the removal event. That is also true, but if it is fixed improperly now, there is a very good chance we just forget about the problem, and never fix it for real. Especially when it becomes very hard to trigger. At least make sure we have an open bug report about it, please. In the longer term I was thinking maybe it would be good to handle the inert resource idea within libwayland-server. We could add a function like wl_resource_zombify() which would mark the resource as a zombie and call the destroy handlers. From the compositor's perspective it can then act as if the resource has been destroyed. We could detect zombie resources being used within the request marshalling code and ignore the request. If the request creates new resource we can internally create new zombie resources too and Weston would never need to know about it. I am planning to experiment with this approach now. Hmm... will be interesting to see, if that works out. It does feel like quite a lot of magic in libwayland-server, while also making life a lot easier. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 1/2] Set to NULL event source after a call to wl_event_source_remove
This patch sets to NULL event sources after a call to wl_event_source_remove() has been made. --- src/clipboard.c | 3 ++- src/compositor-drm.c | 3 +++ src/compositor-rpi.c | 1 + src/compositor-x11.c | 2 ++ src/compositor.c | 6 +- src/evdev-touchpad.c | 1 + src/evdev.c | 4 +++- src/libinput-seat.c | 1 + src/logind-util.c| 2 ++ xwayland/launcher.c | 6 +- xwayland/selection.c | 9 +++-- 11 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/clipboard.c b/src/clipboard.c index 5a3a02d..0e25dc1 100644 --- a/src/clipboard.c +++ b/src/clipboard.c @@ -61,6 +61,7 @@ clipboard_source_unref(struct clipboard_source *source) if (source-event_source) { wl_event_source_remove(source-event_source); + source-event_source = NULL; close(source-fd); } wl_signal_emit(source-base.destroy_signal, @@ -90,8 +91,8 @@ clipboard_source_data(int fd, uint32_t mask, void *data) len = read(fd, p, size); if (len == 0) { wl_event_source_remove(source-event_source); - close(fd); source-event_source = NULL; + close(fd); } else if (len 0) { clipboard_source_unref(source); clipboard-source = NULL; diff --git a/src/compositor-drm.c b/src/compositor-drm.c index 5f59789..0110f41 100644 --- a/src/compositor-drm.c +++ b/src/compositor-drm.c @@ -2379,7 +2379,9 @@ drm_destroy(struct weston_compositor *ec) udev_input_destroy(d-input); wl_event_source_remove(d-udev_drm_source); + d-udev_drm_source = NULL; wl_event_source_remove(d-drm_source); + d-drm_source = NULL; destroy_sprites(d); @@ -2849,6 +2851,7 @@ drm_compositor_create(struct wl_display *display, err_udev_monitor: wl_event_source_remove(ec-udev_drm_source); + ec-udev_drm_source = NULL; udev_monitor_unref(ec-udev_monitor); err_drm_source: wl_event_source_remove(ec-drm_source); diff --git a/src/compositor-rpi.c b/src/compositor-rpi.c index 287451d..cbfb770 100644 --- a/src/compositor-rpi.c +++ b/src/compositor-rpi.c @@ -213,6 +213,7 @@ static void rpi_flippipe_release(struct rpi_flippipe *flippipe) { wl_event_source_remove(flippipe-source); + flippipe-source = NULL; close(flippipe-readfd); close(flippipe-writefd); } diff --git a/src/compositor-x11.c b/src/compositor-x11.c index 56b3228..9f171e7 100644 --- a/src/compositor-x11.c +++ b/src/compositor-x11.c @@ -485,6 +485,7 @@ x11_output_destroy(struct weston_output *output_base) (struct x11_compositor *)output-base.compositor; wl_event_source_remove(output-finish_frame_timer); + output-finish_frame_timer = NULL; if (compositor-use_pixman) { pixman_renderer_output_destroy(output_base); @@ -1424,6 +1425,7 @@ x11_destroy(struct weston_compositor *ec) struct x11_compositor *compositor = (struct x11_compositor *)ec; wl_event_source_remove(compositor-xcb_source); + compositor-xcb_source = NULL; x11_input_destroy(compositor); weston_compositor_shutdown(ec); /* destroys outputs, too */ diff --git a/src/compositor.c b/src/compositor.c index cd1ca9a..6ad3387 100644 --- a/src/compositor.c +++ b/src/compositor.c @@ -3736,8 +3736,12 @@ weston_compositor_shutdown(struct weston_compositor *ec) struct weston_output *output, *next; wl_event_source_remove(ec-idle_source); - if (ec-input_loop_source) + ec-idle_source = NULL; + + if (ec-input_loop_source) { wl_event_source_remove(ec-input_loop_source); + ec-input_loop_source = NULL; + } /* Destroy all outputs associated with this compositor */ wl_list_for_each_safe(output, next, ec-output_list, link) diff --git a/src/evdev-touchpad.c b/src/evdev-touchpad.c index 69f913a..360f87f 100644 --- a/src/evdev-touchpad.c +++ b/src/evdev-touchpad.c @@ -663,6 +663,7 @@ touchpad_destroy(struct evdev_dispatch *dispatch) touchpad-filter-interface-destroy(touchpad-filter); wl_event_source_remove(touchpad-fsm.timer_source); + touchpad-fsm.timer_source = NULL; free(dispatch); } diff --git a/src/evdev.c b/src/evdev.c index 888dfbd..a1bce2c 100644 --- a/src/evdev.c +++ b/src/evdev.c @@ -697,8 +697,10 @@ evdev_device_destroy(struct evdev_device *device) if (dispatch) dispatch-interface-destroy(dispatch); - if (device-source) + if (device-source) { wl_event_source_remove(device-source); + device-source = NULL; + } if (device-output) wl_list_remove(device-output_destroy_listener.link); wl_list_remove(device-link); diff --git a/src/libinput-seat.c b/src/libinput-seat.c index a38d470..34f1aab 100644 --- a/src/libinput-seat.c +++ b/src/libinput-seat.c @@
[PATCH 2/2] Handle OOM with signal events
This patch handles the case where a signal event source can not be created. --- src/compositor.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/compositor.c b/src/compositor.c index 6ad3387..047df8a 100644 --- a/src/compositor.c +++ b/src/compositor.c @@ -4197,6 +4197,7 @@ int main(int argc, char *argv[]) display = wl_display_create(); loop = wl_display_get_event_loop(display); + memset(signals, 0, sizeof(signals)); signals[0] = wl_event_loop_add_signal(loop, SIGTERM, on_term_signal, display); signals[1] = wl_event_loop_add_signal(loop, SIGINT, on_term_signal, @@ -4208,6 +4209,9 @@ int main(int argc, char *argv[]) signals[3] = wl_event_loop_add_signal(loop, SIGCHLD, sigchld_handler, NULL); + if (!signals[0] || !signals[1] || !signals[2] || !signals[3]) + goto out_signals; + config = weston_config_parse(weston.ini); if (config != NULL) { weston_log(Using config file '%s'\n, @@ -4321,8 +4325,11 @@ int main(int argc, char *argv[]) wl_signal_emit(ec-destroy_signal, ec); - for (i = ARRAY_LENGTH(signals); i;) - wl_event_source_remove(signals[--i]); + out_signals: + for (i = ARRAY_LENGTH(signals); i; i--) { + if (signals[i-1]) + wl_event_source_remove(signals[i-1]); + } weston_compositor_xkb_destroy(ec); -- 1.8.1.2 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] Destroy resources when destroying input and output
On Fri, May 9, 2014 at 1:37 AM, Pekka Paalanen ppaala...@gmail.com wrote: Looking at it in general, there is one more fun complication. If the inert object has requests in its interface, that create new objects, the server cannot just ignore those requests. I think the server will need to actually create new objects if requested, but make those inert too. Otherwise there would again be a mismatch between the server and the client on which objects exist. I was hoping that this wasn't actually going to be a problem, but on wl_seat it is. The server can destroy a seat while a client is calling wl_seat.get_pointer for instance. Then, yes, we do have to create an intert object. On Fri, May 9, 2014 at 9:02 AM, Pekka Paalanen ppaala...@gmail.com wrote: On Fri, 09 May 2014 14:33:58 +0100 Neil Roberts n...@linux.intel.com wrote: Perhaps we should consider applying the patch anyway even though it's not ideal. Currently if a client uses a dead output in a request such as xdg_surface.set_output Weston will end up with a weston_output pointer that points to freed memory. This could cause the compositor to crash. True, it's very bad now. That is worse than the alternative provided by this patch which is to make the client abort. The clients know about the output being destroyed via the wl_registry.global_remove event so in practice they would only hit the problem in the unlikely event that they used the output in a request in the short time between the output being unplugged and noticing the removal event. That is also true, but if it is fixed improperly now, there is a very good chance we just forget about the problem, and never fix it for real. Especially when it becomes very hard to trigger. At least make sure we have an open bug report about it, please. I agree. This is bad and we need to make sure it gets fixed properly. In the longer term I was thinking maybe it would be good to handle the inert resource idea within libwayland-server. We could add a function like wl_resource_zombify() which would mark the resource as a zombie and call the destroy handlers. From the compositor's perspective it can then act as if the resource has been destroyed. We could detect zombie resources being used within the request marshalling code and ignore the request. If the request creates new resource we can internally create new zombie resources too and Weston would never need to know about it. I am planning to experiment with this approach now. Hmm... will be interesting to see, if that works out. It does feel like quite a lot of magic in libwayland-server, while also making life a lot easier. Most of the magic there is in allowing resources with no handler in libwayland-server. The patch would be about 4 lines. Right now, client-side wl_proxy objects are allowed to have a NULL implementation and there's no problem; server-side, this is not currently allowed. If we allowed this ten Neil's wl_resource_zombify would only have to set the destructor, and implementation to NULL. For that matter, we could just do that explicitly in weston and not add API for it. The big chunk of magic is in handling new_id requests. We would need to create zombie wl_resource's for each new_id argument on a call to a zombified wl_resource. Now that Pekka mentioned it, I'm not sure that we're handling those correctly client-side if there's no implementation set. --Jason Ekstrand ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH 2/2] compositor-wayland: Add touch support
On Fri, May 9, 2014 at 1:09 AM, Boyan Ding stu_...@126.com wrote: At 2014-05-08 09:55:23, Jason Ekstrand ja...@jlekstrand.net wrote: Boyan, By and large, this looks really good! I have just a few comments below. As I said in another e-mail, I don't have any touch hardware I can test this on, so I wasn't able to actually test it. Unfortunately, I don't have touch hardware either. However I found a interesting project [1] on github, though I'm still figuring out how to use it. I'll be curious to see how well that works. I don't have a touch screen either and it would be nice to be able to test things anyway. On Tue, May 6, 2014 at 9:46 PM, Boyan Ding stu_...@126.com wrote: Adding touch support to weston's nested wayland backend to make testing easier. https://bugs.freedesktop.org/show_bug.cgi?id=77769 --- src/compositor-wayland.c | 95 1 file changed, 95 insertions(+) diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c index a08b71a..45040d4 100644 --- a/src/compositor-wayland.c +++ b/src/compositor-wayland.c @@ -1582,6 +1582,91 @@ static const struct wl_keyboard_listener keyboard_listener = { }; static void +input_handle_touch_down(void *data, struct wl_touch *touch, uint32_t serial, + uint32_t time, struct wl_surface *surface, int32_t id, + wl_fixed_t x, wl_fixed_t y) +{ + struct wayland_input *input = data; + int32_t fx, fy; + + if (input-output-frame) { + frame_touch_down(input-output-frame, input, id, +wl_fixed_to_int(x), wl_fixed_to_int(y)); + frame_interior(input-output-frame, fx, fy, NULL, NULL); + x -= wl_fixed_from_int(fx); + y -= wl_fixed_from_int(fy); You probably want to handle FRAME_STATUS_MOVE here. See also input_handle_button. I saw the code handling FRAME_STATUS_MOVE in input_handle_button returned after handling that. Do we need to return here? Also, I saw the pointer code notify the event only if the location is THEME_LOCATION_CLIENT_AREA. Do we need to do the same thing? (I noticed the change of return type of frame_touch_down from void to theme_location in [2] but that patch hasn't been applied. Actually, the return probably shouldn't be there in the pointer code. None of the frame states preempt any of the others. Regarding notfying, that's a more delicate question. We probably want to do one of two things: a) only call weston_touc_notify if the first touch point went down inside the client area, otherwise only pass touch points to the frame or b) track which touch points went down inside the client area and only notify for those. I think a) would be the simplest to implement and would probably provide the best user experience. + + if (frame_status(input-output-frame) FRAME_STATUS_REPAINT) + weston_output_schedule_repaint(input-output-base); + } + + weston_output_transform_coordinate(input-output-base, x, y, x, y); + + notify_touch(input-base, time, id, x, y, WL_TOUCH_DOWN); +} + +static void +input_handle_touch_up(void *data, struct wl_touch *touch, uint32_t serial, + uint32_t time, int32_t id) +{ + struct wayland_input *input = data; + + if (input-output-frame) { + frame_touch_up(input-output-frame, input, id); + You need to handle FRAME_STATUS_CLOSE here. See also input_handle_button. Perhaps, we want to add a wayland_output_handle_frame_status function to handle all of these so they can all be put in one place. If we want to do that, then it should probably be in it's own patch. + if (frame_status(input-output-frame) FRAME_STATUS_REPAINT) + weston_output_schedule_repaint(input-output-base); + } + + notify_touch(input-base, time, id, 0, 0, WL_TOUCH_UP); +} + +static void +input_handle_touch_motion(void *data, struct wl_touch *touch, uint32_t time, + int32_t id, wl_fixed_t x, wl_fixed_t y) +{ + struct wayland_input *input = data; + int32_t fx, fy; + + if (input-output-frame) { + frame_touch_motion(input-output-frame, input, id, +wl_fixed_to_int(x), wl_fixed_to_int(y)); + + frame_interior(input-output-frame, fx, fy, NULL, NULL); + x -= wl_fixed_from_int(fx); + y -= wl_fixed_from_int(fy); + + if (frame_status(input-output-frame) FRAME_STATUS_REPAINT) + weston_output_schedule_repaint(input-output-base); + } + + weston_output_transform_coordinate(input-output-base, x, y, x, y); + + notify_touch(input-base, time, id, x, y, WL_TOUCH_MOTION); +} + +static void +input_handle_touch_frame(void *data, struct wl_touch *touch) +{ + struct wayland_input *input =
Re: [PATCH] Destroy resources when destroying input and output
Jason Ekstrand ja...@jlekstrand.net writes: Most of the magic there is in allowing resources with no handler in libwayland-server. The patch would be about 4 lines. Right now, client-side wl_proxy objects are allowed to have a NULL implementation and there's no problem; server-side, this is not currently allowed. If we allowed this ten Neil's wl_resource_zombify would only have to set the destructor, and implementation to NULL. For that matter, we could just do that explicitly in weston and not add API for it. Don't forget that we also want to ignore requests that have zombie resources as arguments, not just if the resource is directly used as a target of a request. It looks like the client-side already has code to handle zombie objects by putting a marker called WL_ZOMBIE_OBJECT in the ID map. Perhaps we should use the same mechanism on the server side too. If a zombie object is received in an event on the client side it looks like wl_closure_lookup_objects just sets the proxy object to NULL. Is that safe? Wouldn't the event handlers crash if they get a NULL resource in an event? If we can somehow make wl_closure_lookup_objects report when it finds a zombie object we can make the upper layers just consume the event. We could do this on both the client and side and the server side. - Neil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland 3/6 v2] protocol: Fix order of wl_pointer, wl_keyboard and wl_touch messages
The release message of wl_pointer, wl_keyboard and wl_touch introduced in version 3 was placed first in the respective interface XML element, causing wayland-scanner to misbehave and set the version number of the release message to all subsequent messages with no explicitly specified since version. Signed-off-by: Jonas Ådahl jad...@gmail.com --- Fixed typo in commit subject and changed the wording of the commit message a bit. The content of the patch is identical to v1. Jonas protocol/wayland.xml | 31 +++ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/protocol/wayland.xml b/protocol/wayland.xml index 330f8ab..22eb6e7 100644 --- a/protocol/wayland.xml +++ b/protocol/wayland.xml @@ -1384,10 +1384,6 @@ arg name=hotspot_y type=int summary=y coordinate in surface-relative coordinates/ /request -request name=release type=destructor since=3 - description summary=release the pointer object/ -/request - event name=enter description summary=enter event Notification that this seat's pointer is focused on a certain @@ -1485,6 +1481,13 @@ arg name=axis type=uint/ arg name=value type=fixed/ /event + +!-- Version 3 additions -- + +request name=release type=destructor since=3 + description summary=release the pointer object/ +/request + /interface interface name=wl_keyboard version=3 @@ -1493,10 +1496,6 @@ associated with a seat. /description -request name=release type=destructor since=3 - description summary=release the keyboard object/ -/request - enum name=keymap_format description summary=keyboard mapping format This specifies the format of the keymap provided to the @@ -1573,6 +1572,12 @@ arg name=mods_locked type=uint/ arg name=group type=uint/ /event + +!-- Version 3 additions -- + +request name=release type=destructor since=3 + description summary=release the keyboard object/ +/request /interface interface name=wl_touch version=3 @@ -1587,10 +1592,6 @@ contact point can be identified by the ID of the sequence. /description -request name=release type=destructor since=3 - description summary=release the touch object/ -/request - event name=down description summary=touch down event and beginning of a touch sequence A new touch point has appeared on the surface. This touch point is @@ -1643,6 +1644,12 @@ this surface may re-use the touch point ID. /description /event + +!-- Version 3 additions -- + +request name=release type=destructor since=3 + description summary=release the touch object/ +/request /interface interface name=wl_output version=2 -- 1.9.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Bug 78372 - create multiple windows with offset
On 05/09/2014 12:34 AM, Pekka Paalanen wrote: Possibly you are reading the words save/restore literally, in that you are imagining some blob of data stored in the compositor that is recognized to restore the layout. However this is NOT what is wanted. Sure, that's the first thing comes to my mind. Let the client save a cookie or an encrypted blob, that it can pass back to the server when it creates the windows again. /handwaving/ That is what I was worried about. This will not work, as we need layouts that work the first time the client is run, that can be used by different compositors, and can port to non-Wayland systems. The client has no clue whatsoever on e.g. what positions on an output might be ok, what else is on the screen, or which outputs would be ok to conquer. You also might be on a tiling compositor, where your expectations simply totally fail. Your program would have to be a mind-reader to get the initial layout right for any user. I have no problem with the compositor moving the windows from the client's expected positions. It is a *HINT*. Clients can probably assume that if they reuse the positions from a previous run on the same compositor with the same outputs, they will get the same positions. I think also they can assume that if a window has a smaller x value than another it will be further to the left. That is all I am trying to get. I think an x/y position in some global space will fulfill these requirements and anything more complex is unnecessary and confusing, as well as making it impossible to port these positions to other operating systems. I think it is acceptable that the actual client api is output+x+y along with another api that says the x+y of the corner of each output is this. Then the client can do the conversion from global x/y to per-output x/y. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] desktop-shell: Fix black edges on scaled desktop pattern
On 05/09/2014 02:11 AM, Pekka Paalanen wrote: On Thu, 08 May 2014 20:00:35 -0700 Bill Spitzak spit...@gmail.com wrote: Filter sampling outside the source image can leak black into the edges of the desktop image. This is most easily seen by scaling the default tiled image with this weston.ini: # no background-image and no background-color background-type=scale-crop --- clients/desktop-shell.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c index 4880888..e121cc7 100644 --- a/clients/desktop-shell.c +++ b/clients/desktop-shell.c @@ -724,6 +724,7 @@ background_draw(struct widget *widget, void *data) case BACKGROUND_SCALE: cairo_matrix_init_scale(matrix, sx, sy); cairo_pattern_set_matrix(pattern, matrix); + cairo_pattern_set_extend(pattern, CAIRO_EXTEND_PAD); break; case BACKGROUND_SCALE_CROP: s = (sx sy) ? sx : sy; @@ -733,6 +734,7 @@ background_draw(struct widget *widget, void *data) cairo_matrix_init_translate(matrix, tx, ty); cairo_matrix_scale(matrix, s, s); cairo_pattern_set_matrix(pattern, matrix); + cairo_pattern_set_extend(pattern, CAIRO_EXTEND_PAD); break; case BACKGROUND_TILE: cairo_pattern_set_extend(pattern, CAIRO_EXTEND_REPEAT); Ok, I see the problem, and I see this fix should be good, but for some reason this patch does not apply at all. I had to do the change manually. Anyway, it works right and fixes the issue. Hm, it does apply with --ignore-whitespace, though. When I save the raw email, there seems to be extra spaces in the beginning of all context lines. Maybe Format=flowed messes it up? git send-email did not work (as it wants a mta set up) so I tried to do it by hand. Fail. If I set up a mta here at work it will look like the email came from work. That is not a problem for the mailing list, right? (assuming I change the from address to my gmail account). ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston] libinput-seat: literal values for WESTON_LIBINPUT_LOG_PRIORITY
Only accept specific literal values from the environment variable WESTON_LIBINPUT_LOG_PRIORITY... debug, info, or error. Signed-off-by: U. Artie Eoff ullysses.a.e...@intel.com --- src/libinput-seat.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/libinput-seat.c b/src/libinput-seat.c index a38d470..d59ae42 100644 --- a/src/libinput-seat.c +++ b/src/libinput-seat.c @@ -271,8 +271,15 @@ udev_input_init(struct udev_input *input, struct weston_compositor *c, struct ud libinput_log_set_handler(libinput_log_func, NULL); log_priority = getenv(WESTON_LIBINPUT_LOG_PRIORITY); + if (log_priority) { - libinput_log_set_priority(strtol(log_priority, NULL, 10)); + if (strcmp(log_priority, debug) == 0) { + libinput_log_set_priority(LIBINPUT_LOG_PRIORITY_DEBUG); + } else if (strcmp(log_priority, info) == 0) { + libinput_log_set_priority(LIBINPUT_LOG_PRIORITY_INFO); + } else if (strcmp(log_priority, error) == 0) { + libinput_log_set_priority(LIBINPUT_LOG_PRIORITY_ERROR); + } } input-libinput = libinput_udev_create_for_seat(libinput_interface, input, -- 1.9.0 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] desktop-shell: Fix black edges on scaled desktop pattern
On Fri, May 09, 2014 at 11:13:54AM -0700, Bill Spitzak wrote: On 05/09/2014 02:11 AM, Pekka Paalanen wrote: On Thu, 08 May 2014 20:00:35 -0700 Bill Spitzak spit...@gmail.com wrote: Filter sampling outside the source image can leak black into the edges of the desktop image. This is most easily seen by scaling the default tiled image with this weston.ini: # no background-image and no background-color background-type=scale-crop --- clients/desktop-shell.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c index 4880888..e121cc7 100644 --- a/clients/desktop-shell.c +++ b/clients/desktop-shell.c @@ -724,6 +724,7 @@ background_draw(struct widget *widget, void *data) case BACKGROUND_SCALE: cairo_matrix_init_scale(matrix, sx, sy); cairo_pattern_set_matrix(pattern, matrix); + cairo_pattern_set_extend(pattern, CAIRO_EXTEND_PAD); break; case BACKGROUND_SCALE_CROP: s = (sx sy) ? sx : sy; @@ -733,6 +734,7 @@ background_draw(struct widget *widget, void *data) cairo_matrix_init_translate(matrix, tx, ty); cairo_matrix_scale(matrix, s, s); cairo_pattern_set_matrix(pattern, matrix); + cairo_pattern_set_extend(pattern, CAIRO_EXTEND_PAD); break; case BACKGROUND_TILE: cairo_pattern_set_extend(pattern, CAIRO_EXTEND_REPEAT); Ok, I see the problem, and I see this fix should be good, but for some reason this patch does not apply at all. I had to do the change manually. Anyway, it works right and fixes the issue. Hm, it does apply with --ignore-whitespace, though. When I save the raw email, there seems to be extra spaces in the beginning of all context lines. Maybe Format=flowed messes it up? git send-email did not work (as it wants a mta set up) so I tried to do it by hand. Fail. If I set up a mta here at work it will look like the email came from work. That is not a problem for the mailing list, right? (assuming I change the from address to my gmail account). If you are using gmail, you can just use Googles SMTP server directly. The example configuration in the manual [0] even is a @gmail.com address setup. Jonas [0] http://git-scm.com/docs/git-send-email ___ 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
Re: [PATCH] clients: Initialize label in keyboard handling code
On Wed, May 07, 2014 at 01:11:07AM +, Bryce W. Harrington wrote: Quells warning: clients/keyboard.c: In function ‘keyboard_handle_key.isra.5’: clients/keyboard.c:556:11: warning: ‘label’ may be used uninitialized in this function [-Wuninitialized] Signed-off-by: Bryce Harrington b.harring...@samsung.com Thanks Bryce, patch applied. Kristian --- clients/keyboard.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/keyboard.c b/clients/keyboard.c index cd1ad58..7c11cec 100644 --- a/clients/keyboard.c +++ b/clients/keyboard.c @@ -530,7 +530,7 @@ append(char *s1, const char *s2) static void keyboard_handle_key(struct keyboard *keyboard, uint32_t time, const struct key *key, struct input *input, enum wl_pointer_button_state state) { - const char *label; + const char *label = NULL; switch(keyboard-state) { case KEYBOARD_STATE_DEFAULT : -- 1.7.9.5 ___ 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
Re: [PATCH 3/5] clients: Use calloc instead of malloc/memset=0
On Wed, May 07, 2014 at 02:13:11AM +, Bryce W. Harrington wrote: Signed-off-by: Bryce Harrington b.harring...@samsung.com --- clients/editor.c |4 +--- clients/subsurfaces.c |8 ++-- clients/window.c | 13 ++--- 3 files changed, 5 insertions(+), 20 deletions(-) diff --git a/clients/editor.c b/clients/editor.c index 6ed76d4..b439d9e 100644 --- a/clients/editor.c +++ b/clients/editor.c @@ -559,9 +559,7 @@ text_entry_create(struct editor *editor, const char *text) { struct text_entry *entry; - entry = xmalloc(sizeof *entry); - memset(entry, 0, sizeof *entry); - + entry = xcalloc(1, sizeof *entry); We have zalloc for when we just want malloc+memset. Kristian entry-widget = widget_add_widget(editor-widget, entry); entry-window = editor-window; entry-text = strdup(text); diff --git a/clients/subsurfaces.c b/clients/subsurfaces.c index 15af9aa..a683787 100644 --- a/clients/subsurfaces.c +++ b/clients/subsurfaces.c @@ -492,9 +492,7 @@ triangle_create(struct window *window, struct egl_state *egl) { struct triangle *tri; - tri = xmalloc(sizeof *tri); - memset(tri, 0, sizeof *tri); - + tri = xcalloc(1, sizeof *tri); tri-egl = egl; tri-widget = window_add_subsurface(window, tri, int_to_mode(option_triangle_mode)); @@ -714,9 +712,7 @@ demoapp_create(struct display *display) { struct demoapp *app; - app = xmalloc(sizeof *app); - memset(app, 0, sizeof *app); - + app = xcalloc(1, sizeof *app); app-egl = egl_state_create(display_get_display(display)); app-display = display; diff --git a/clients/window.c b/clients/window.c index cfc1260..2212351 100644 --- a/clients/window.c +++ b/clients/window.c @@ -1139,12 +1139,7 @@ shm_surface_create(struct display *display, struct wl_surface *wl_surface, struct shm_surface *surface; DBG_OBJ(wl_surface, \n); - surface = xmalloc(sizeof *surface); - memset(surface, 0, sizeof *surface); - - if (!surface) - return NULL; - + surface = xcalloc(1, sizeof *surface); surface-base.prepare = shm_surface_prepare; surface-base.swap = shm_surface_swap; surface-base.acquire = shm_surface_acquire; @@ -4336,11 +4331,7 @@ surface_create(struct window *window) struct display *display = window-display; struct surface *surface; - surface = xmalloc(sizeof *surface); - memset(surface, 0, sizeof *surface); - if (!surface) - return NULL; - + surface = xcalloc(1, sizeof *surface); surface-window = window; surface-surface = wl_compositor_create_surface(display-compositor); surface-buffer_scale = 1; -- 1.7.9.5 ___ 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
Re: [PATCH 4/5] clients: Use xzalloc instead of xcalloc when allocating single element
On Wed, May 07, 2014 at 02:13:11AM +, Bryce W. Harrington wrote: Signed-off-by: Bryce Harrington b.harring...@samsung.com --- clients/desktop-shell.c |2 +- clients/editor.c|2 +- clients/fullscreen.c|2 +- clients/subsurfaces.c |6 +++--- clients/window.c|4 ++-- clients/wscreensaver.c |2 +- 6 files changed, 9 insertions(+), 9 deletions(-) Oh, didn't see this one when I replied to the earlier xcalloc patch. I don't think we have a single place where we actually need xcalloc, so I'd rather not add it at all. Kristian diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c index a3b2534..0b8d08b 100644 --- a/clients/desktop-shell.c +++ b/clients/desktop-shell.c @@ -1217,7 +1217,7 @@ create_output(struct desktop *desktop, uint32_t id) { struct output *output; - output = xcalloc(1, sizeof *output); + output = xzalloc(sizeof *output); output-output = display_bind(desktop-display, id, wl_output_interface, 2); output-server_output_id = id; diff --git a/clients/editor.c b/clients/editor.c index b439d9e..bda3e91 100644 --- a/clients/editor.c +++ b/clients/editor.c @@ -559,7 +559,7 @@ text_entry_create(struct editor *editor, const char *text) { struct text_entry *entry; - entry = xcalloc(1, sizeof *entry); + entry = xzalloc(sizeof *entry); entry-widget = widget_add_widget(editor-widget, entry); entry-window = editor-window; entry-text = strdup(text); diff --git a/clients/fullscreen.c b/clients/fullscreen.c index ad7c703..8f41455 100644 --- a/clients/fullscreen.c +++ b/clients/fullscreen.c @@ -480,7 +480,7 @@ output_handler(struct output *output, void *data) if (fsout-output == output) return; - fsout = xcalloc(1, sizeof *fsout); + fsout = xzalloc(sizeof *fsout); fsout-output = output; wl_list_insert(fullscreen-output_list, fsout-link); } diff --git a/clients/subsurfaces.c b/clients/subsurfaces.c index a683787..1ed698b 100644 --- a/clients/subsurfaces.c +++ b/clients/subsurfaces.c @@ -212,7 +212,7 @@ egl_state_create(struct wl_display *display) EGLint major, minor, n; EGLBoolean ret; - egl = xcalloc(1, sizeof *egl); + egl = xzalloc(sizeof *egl); egl-dpy = eglGetDisplay(display); assert(egl-dpy); @@ -492,7 +492,7 @@ triangle_create(struct window *window, struct egl_state *egl) { struct triangle *tri; - tri = xcalloc(1, sizeof *tri); + tri = xzalloc(sizeof *tri); tri-egl = egl; tri-widget = window_add_subsurface(window, tri, int_to_mode(option_triangle_mode)); @@ -712,7 +712,7 @@ demoapp_create(struct display *display) { struct demoapp *app; - app = xcalloc(1, sizeof *app); + app = xzalloc(sizeof *app); app-egl = egl_state_create(display_get_display(display)); app-display = display; diff --git a/clients/window.c b/clients/window.c index 2212351..a103530 100644 --- a/clients/window.c +++ b/clients/window.c @@ -1139,7 +1139,7 @@ shm_surface_create(struct display *display, struct wl_surface *wl_surface, struct shm_surface *surface; DBG_OBJ(wl_surface, \n); - surface = xcalloc(1, sizeof *surface); + surface = xzalloc(sizeof *surface); surface-base.prepare = shm_surface_prepare; surface-base.swap = shm_surface_swap; surface-base.acquire = shm_surface_acquire; @@ -4331,7 +4331,7 @@ surface_create(struct window *window) struct display *display = window-display; struct surface *surface; - surface = xcalloc(1, sizeof *surface); + surface = xzalloc(sizeof *surface); surface-window = window; surface-surface = wl_compositor_create_surface(display-compositor); surface-buffer_scale = 1; diff --git a/clients/wscreensaver.c b/clients/wscreensaver.c index 1070c07..d87d040 100644 --- a/clients/wscreensaver.c +++ b/clients/wscreensaver.c @@ -175,7 +175,7 @@ create_wscreensaver_instance(struct wscreensaver *screensaver, struct ModeInfo *mi; struct rectangle drawarea; - mi = xcalloc(1, sizeof *mi); + mi = xzalloc(sizeof *mi); if (demo_mode) mi-window = window_create(screensaver-display); -- 1.7.9.5 ___ 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
Re: [PATCH 5/5] clients: Use xstrdup instead of strdup
On Wed, May 07, 2014 at 02:13:11AM +, Bryce W. Harrington wrote: Signed-off-by: Bryce Harrington b.harring...@samsung.com --- clients/editor.c | 12 ++-- clients/image.c|4 ++-- clients/keyboard.c | 12 ++-- clients/terminal.c |2 +- 4 files changed, 15 insertions(+), 15 deletions(-) This one looks good, but doesn't apply without the rest of the series. Kristian diff --git a/clients/editor.c b/clients/editor.c index bda3e91..ece8b1d 100644 --- a/clients/editor.c +++ b/clients/editor.c @@ -258,7 +258,7 @@ text_input_preedit_string(void *data, } text_entry_set_preedit(entry, text, entry-preedit_info.cursor); - entry-preedit.commit = strdup(commit); + entry-preedit.commit = xstrdup(commit); entry-preedit.attr_list = pango_attr_list_ref(entry-preedit_info.attr_list); clear_pending_preedit(entry); @@ -562,7 +562,7 @@ text_entry_create(struct editor *editor, const char *text) entry = xzalloc(sizeof *entry); entry-widget = widget_add_widget(editor-widget, entry); entry-window = editor-window; - entry-text = strdup(text); + entry-text = xstrdup(text); entry-active = 0; entry-cursor = strlen(text); entry-anchor = entry-cursor; @@ -686,7 +686,7 @@ text_entry_update_layout(struct text_entry *entry) strcpy(text + entry-cursor + strlen(entry-preedit.text), entry-text + entry-cursor); } else { - text = strdup(entry-text); + text = xstrdup(entry-text); } if (entry-cursor != entry-anchor) { @@ -809,7 +809,7 @@ text_entry_commit_and_reset(struct text_entry *entry) char *commit = NULL; if (entry-preedit.commit) - commit = strdup(entry-preedit.commit); + commit = xstrdup(entry-preedit.commit); text_entry_reset_preedit(entry); if (commit) { @@ -832,7 +832,7 @@ text_entry_set_preedit(struct text_entry *entry, if (!preedit_text) return; - entry-preedit.text = strdup(preedit_text); + entry-preedit.text = xstrdup(preedit_text); entry-preedit.cursor = preedit_cursor; text_entry_update_layout(entry); @@ -1345,7 +1345,7 @@ main(int argc, char *argv[]) editor.entry = text_entry_create(editor, Entry); editor.entry-click_to_show = click_to_show; if (preferred_language) - editor.entry-preferred_language = strdup(preferred_language); + editor.entry-preferred_language = xstrdup(preferred_language); editor.editor = text_entry_create(editor, Numeric); editor.editor-content_purpose = WL_TEXT_INPUT_CONTENT_PURPOSE_NUMBER; editor.editor-click_to_show = click_to_show; diff --git a/clients/image.c b/clients/image.c index cba68c5..b4a7bb8 100644 --- a/clients/image.c +++ b/clients/image.c @@ -362,12 +362,12 @@ image_create(struct display *display, const char *filename, image = xzalloc(sizeof *image); - copy = strdup(filename); + copy = xstrdup(filename); b = basename(copy); snprintf(title, sizeof title, Wayland Image - %s, b); free(copy); - image-filename = strdup(filename); + image-filename = xstrdup(filename); image-image = load_cairo_surface(filename); if (!image-image) { diff --git a/clients/keyboard.c b/clients/keyboard.c index cd1ad58..6b1e7a0 100644 --- a/clients/keyboard.c +++ b/clients/keyboard.c @@ -440,12 +440,12 @@ virtual_keyboard_commit_preedit(struct virtual_keyboard *keyboard) keyboard-surrounding_text = surrounding_text; keyboard-surrounding_cursor += strlen(keyboard-preedit_string); } else { - keyboard-surrounding_text = strdup(keyboard-preedit_string); + keyboard-surrounding_text = xstrdup(keyboard-preedit_string); keyboard-surrounding_cursor = strlen(keyboard-preedit_string); } free(keyboard-preedit_string); - keyboard-preedit_string = strdup(); + keyboard-preedit_string = xstrdup(); } static void @@ -757,7 +757,7 @@ handle_surrounding_text(void *data, struct virtual_keyboard *keyboard = data; free(keyboard-surrounding_text); - keyboard-surrounding_text = strdup(text); + keyboard-surrounding_text = xstrdup(text); keyboard-surrounding_cursor = cursor; } @@ -772,7 +772,7 @@ handle_reset(void *data, if (strlen(keyboard-preedit_string)) { free(keyboard-preedit_string); - keyboard-preedit_string = strdup(); + keyboard-preedit_string = xstrdup(); } } @@ -840,7 +840,7 @@ handle_preferred_language(void *data, keyboard-preferred_language = NULL; if (language) - keyboard-preferred_language = strdup(language); + keyboard-preferred_language = xstrdup(language); }
Re: [PATCH v2] doc: Added API documentation for wl_display_create function.
On Wed, May 07, 2014 at 09:37:45AM +0530, Srivardhan Hebbar wrote: Signed-off-by: Srivardhan Hebbar sri.heb...@samsung.com --- src/wayland-server.c |9 + 1 file changed, 9 insertions(+) diff --git a/src/wayland-server.c b/src/wayland-server.c index f2b1b42..57b65ce 100644 --- a/src/wayland-server.c +++ b/src/wayland-server.c @@ -788,6 +788,15 @@ bind_display(struct wl_client *client, destroy_client_display_resource); } +/** Create Wayland display object. + * + * \param None + * \return The Wayland display object. Null if failed to create + * + * This creates the wl_display object. + * + * \memberof wl_display + */ That's better thanks. Patch applied. Kristian WL_EXPORT struct wl_display * wl_display_create(void) { -- 1.7.9.5 ___ 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
Re: [PATCH weston] window.c: Set the input region of the tooltip to empty
On Wed, May 07, 2014 at 11:47:08AM +0300, Pekka Paalanen wrote: On Tue, 6 May 2014 14:40:53 -0700 Kristian Høgsberg hoegsb...@gmail.com wrote: On Mon, May 05, 2014 at 05:02:15PM +0300, Ander Conselvan de Oliveira wrote: From: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com Otherwise it might receive touch events. I think a better approach is to just hide the tooltip if it (or the panel) gets touch events. I don't think I agree with Pekka that we can't use subsurfaces for this, but I guess I'm not sure what the problem he sees there is. The panel is not a window, so sub-surfaces can be made to work there well enough. It is for normal windows like registered with xdg_shell where sub-surfaces are not suitable for tooltips. A sub-surface is a part of the window, not another window. A tooltip is not expected to change the window geometry, but a sub-surface does count into the window geometry. Extruding sub-surfaces therefore affect the (parent) window geometry. It is in the protocol spec. No, sure if no other geometry information is available. For xdg-shell windows, the window geometry overrides that though. And I don't think there's a clear distinction between being part of the window or being a separate window here, and I don't see why it would useful... Kristian Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 3/3] shell: Fix crash when restoring focus state during workspace change
On Wed, May 07, 2014 at 11:57:28AM +0300, Ander Conselvan de Oliveira wrote: From: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com The check to avoid calling weston_keyboard_set_focus() for a seat that didn't have a keyboard in restore_focus_state() was cheking the wrong seat (the one from the previous loop). That caused a crash when switching workspaces if there was an extra seat that didn't have a keyboard. https://bugs.freedesktop.org/show_bug.cgi?id=78349 Thanks Ander, all three committed. Kristian --- desktop-shell/shell.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c index fac3120..ea7b3cd 100644 --- a/desktop-shell/shell.c +++ b/desktop-shell/shell.c @@ -731,7 +731,7 @@ restore_focus_state(struct desktop_shell *shell, struct workspace *ws) wl_list_for_each_safe(seat, next_seat, pending_seat_list, link) { wl_list_insert(shell-compositor-seat_list, seat-link); - if (state-seat-keyboard == NULL) + if (seat-keyboard == NULL) continue; weston_keyboard_set_focus(seat-keyboard, NULL); -- 1.8.3.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
Re: [PATCH weston] shell: Don't allow maximized surfaces to be moved with touch
On Wed, May 07, 2014 at 02:22:23PM +0300, Ander Conselvan de Oliveira wrote: From: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com Moving a maximized surface with the pointer is already not possible, so make the behavior with touch consistent. https://bugs.freedesktop.org/show_bug.cgi?id=78208 Thanks, committed. Kristian --- desktop-shell/shell.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c index ea7b3cd..db55ea9 100644 --- a/desktop-shell/shell.c +++ b/desktop-shell/shell.c @@ -1453,7 +1453,7 @@ surface_touch_move(struct shell_surface *shsurf, struct weston_seat *seat) if (!shsurf) return -1; - if (shsurf-state.fullscreen) + if (shsurf-state.fullscreen || shsurf-state.maximized) return 0; move = malloc(sizeof *move); -- 1.8.3.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
[PATCH] desktop-shell: Fix black edges on scaled desktop pattern
Filter sampling outside the source image can leak black into the edges of the desktop image. This is most easily seen by scaling the default tiled image with this weston.ini: # no background-image and no background-color background-type=scale-crop --- clients/desktop-shell.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c index 4880888..e121cc7 100644 --- a/clients/desktop-shell.c +++ b/clients/desktop-shell.c @@ -724,6 +724,7 @@ background_draw(struct widget *widget, void *data) case BACKGROUND_SCALE: cairo_matrix_init_scale(matrix, sx, sy); cairo_pattern_set_matrix(pattern, matrix); + cairo_pattern_set_extend(pattern, CAIRO_EXTEND_PAD); break; case BACKGROUND_SCALE_CROP: s = (sx sy) ? sx : sy; @@ -733,6 +734,7 @@ background_draw(struct widget *widget, void *data) cairo_matrix_init_translate(matrix, tx, ty); cairo_matrix_scale(matrix, s, s); cairo_pattern_set_matrix(pattern, matrix); + cairo_pattern_set_extend(pattern, CAIRO_EXTEND_PAD); break; case BACKGROUND_TILE: cairo_pattern_set_extend(pattern, CAIRO_EXTEND_REPEAT); -- 1.7.9.5 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] desktop-shell: Fix black edges on scaled desktop pattern
Thanks, it looks like that setup worked, patch sent correctly now. On 05/09/2014 11:52 AM, Jonas Ådahl wrote: If you are using gmail, you can just use Googles SMTP server directly. The example configuration in the manual [0] even is a @gmail.com address setup. Jonas [0] http://git-scm.com/docs/git-send-email ___ 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
Re: [PATCH] desktop-shell: Fix black edges on scaled desktop pattern
On Thu, May 08, 2014 at 08:00:35PM -0700, Bill Spitzak wrote: Filter sampling outside the source image can leak black into the edges of the desktop image. This is most easily seen by scaling the default tiled image with this weston.ini: # no background-image and no background-color background-type=scale-crop Thanks, that's a nice detail to get right. Patch applied. Kristian --- clients/desktop-shell.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c index 4880888..e121cc7 100644 --- a/clients/desktop-shell.c +++ b/clients/desktop-shell.c @@ -724,6 +724,7 @@ background_draw(struct widget *widget, void *data) case BACKGROUND_SCALE: cairo_matrix_init_scale(matrix, sx, sy); cairo_pattern_set_matrix(pattern, matrix); + cairo_pattern_set_extend(pattern, CAIRO_EXTEND_PAD); break; case BACKGROUND_SCALE_CROP: s = (sx sy) ? sx : sy; @@ -733,6 +734,7 @@ background_draw(struct widget *widget, void *data) cairo_matrix_init_translate(matrix, tx, ty); cairo_matrix_scale(matrix, s, s); cairo_pattern_set_matrix(pattern, matrix); + cairo_pattern_set_extend(pattern, CAIRO_EXTEND_PAD); break; case BACKGROUND_TILE: cairo_pattern_set_extend(pattern, CAIRO_EXTEND_REPEAT); -- 1.7.9.5 ___ 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
Re: [PATCH] event: Cheking for NULL before dereferencing the pointer.
On Fri, May 09, 2014 at 02:56:14PM +0530, Srivardhan wrote: -Original Message- From: wayland-devel [mailto:wayland-devel- boun...@lists.freedesktop.org] On Behalf Of Hardening Sent: Friday, May 09, 2014 1:08 PM To: wayland-devel@lists.freedesktop.org Subject: Re: [PATCH] event: Cheking for NULL before dereferencing the pointer. Le 09/05/2014 08:43, Srivardhan Hebbar a écrit : Checking for NULL before dereferencing the wl_event_source pointer so as to avoid crash. Signed-off-by: Srivardhan Hebbar sri.heb...@samsung.com --- src/event-loop.c |7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/event-loop.c b/src/event-loop.c index 9790cde..b62d16e 100644 --- a/src/event-loop.c +++ b/src/event-loop.c @@ -312,7 +312,12 @@ wl_event_source_check(struct wl_event_source *source) WL_EXPORT int wl_event_source_remove(struct wl_event_source *source) { - struct wl_event_loop *loop = source-loop; + struct wl_event_loop *loop; + + if (source == NULL) + return 0; + + loop = source-loop; /* We need to explicitly remove the fd, since closing the fd * isn't enough in case we've dup'ed the fd. */ Hello Srivardhan, do you have a case where this check is hit ? I may be wrong but having no loop associated with a source event is not supposed to happen. So my guess is that a caller of wl_event_source_remove has forgotten to nullify the event source after the call. Hi, I was going through the code of Weston. In the main function in compositor.c wl_event_loop_add_signal() is called to allocate the memory for signals[](Line no: 4196. struct wl_event_source *signals[4]). The function returns NULL if memory allocation failed. After that there is no NULL check for 'signals'. In the end while clearing up, this function is called. So if memory allocation failed then a NULL is passed to this function. Hence adding code to check for the NULL. You caught a problem here, but the issue is that we don't check for NULL where we allocate the source. Passing a NULL pointer to wl_event_source_remove() is a programmer error and we don't want to silently fail. Kristian Thank-you, Hebbar Regards. -- 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 ___ 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
Re: [PATCH weston] rpi: build fix for compute_rects debug
On Fri, May 09, 2014 at 03:08:06PM +0300, Pekka Paalanen wrote: From: Pekka Paalanen pekka.paala...@collabora.co.uk See 918f2dd4cfb8b177f67b45653efbbe4325cbe9dc Thanks Pekka, applied. Kristian Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk --- src/rpi-renderer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rpi-renderer.c b/src/rpi-renderer.c index 3a7f65c..c222eb6 100644 --- a/src/rpi-renderer.c +++ b/src/rpi-renderer.c @@ -858,8 +858,8 @@ rpir_view_compute_rects(struct rpir_view *view, src_height = int_max(src_height, 0); DBG(rpir_view %p %dx%d: p1 %f, %f; p2 %f, %f\n, view, - view-view-geometry.width, - view-view-geometry.height, + view-view-surface-width, + view-view-surface-height, p1.f[0], p1.f[1], p2.f[0], p2.f[1]); DBG(src rect %d;%d, %d;%d, %d;%dx%d;%d\n, src_x 16, src_x 0x, -- 1.8.5.5 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] vaapi-recorder: Don't loop trying to write on out of space condition
On Fri, May 09, 2014 at 03:57:38PM +0300, Ander Conselvan de Oliveira wrote: From: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com The error handling for the function that writes the encoded frame on the disk was bogus, always assuming the buffer supplied to the encoder was too small. That would cause a bigger buffer to be allocated and another attempt to encode the frame was done. In the case of a failure to write to disk (due to ENOSPC, for instance) that would cause an endless loop. Thanks committed. I added the bugzilla link to the commit message for reference. Kristian --- Possibly related to https://bugs.freedesktop.org/show_bug.cgi?id=69330 --- src/compositor-drm.c | 27 +++ src/vaapi-recorder.c | 42 +- src/vaapi-recorder.h | 2 +- 3 files changed, 53 insertions(+), 18 deletions(-) diff --git a/src/compositor-drm.c b/src/compositor-drm.c index 5f59789..7d514e4 100644 --- a/src/compositor-drm.c +++ b/src/compositor-drm.c @@ -2558,6 +2558,18 @@ planes_binding(struct weston_seat *seat, uint32_t time, uint32_t key, void *data #ifdef BUILD_VAAPI_RECORDER static void +recorder_destroy(struct drm_output *output) +{ + vaapi_recorder_destroy(output-recorder); + output-recorder = NULL; + + output-base.disable_planes--; + + wl_list_remove(output-recorder_frame_listener.link); + weston_log([libva recorder] done\n); +} + +static void recorder_frame_notify(struct wl_listener *listener, void *data) { struct drm_output *output; @@ -2579,7 +2591,12 @@ recorder_frame_notify(struct wl_listener *listener, void *data) return; } - vaapi_recorder_frame(output-recorder, fd, output-current-stride); + ret = vaapi_recorder_frame(output-recorder, fd, +output-current-stride); + if (ret 0) { + weston_log([libva recorder] aborted: %m\n); + recorder_destroy(output); + } } static void * @@ -2637,13 +2654,7 @@ recorder_binding(struct weston_seat *seat, uint32_t time, uint32_t key, weston_log([libva recorder] initialized\n); } else { - vaapi_recorder_destroy(output-recorder); - output-recorder = NULL; - - output-base.disable_planes--; - - wl_list_remove(output-recorder_frame_listener.link); - weston_log([libva recorder] done\n); + recorder_destroy(output); } } #else diff --git a/src/vaapi-recorder.c b/src/vaapi-recorder.c index 0095a42..921494d 100644 --- a/src/vaapi-recorder.c +++ b/src/vaapi-recorder.c @@ -50,6 +50,7 @@ #include string.h #include unistd.h #include assert.h +#include errno.h #include sys/types.h #include sys/stat.h @@ -93,6 +94,7 @@ struct vaapi_recorder { int width, height; int frame_count; + int error; int destroying; pthread_t worker_thread; pthread_mutex_t mutex; @@ -761,7 +763,13 @@ encoder_create_output_buffer(struct vaapi_recorder *r) return VA_INVALID_ID; } -static int +enum output_write_status { + OUTPUT_WRITE_SUCCESS, + OUTPUT_WRITE_OVERFLOW, + OUTPUT_WRITE_FATAL +}; + +static enum output_write_status encoder_write_output(struct vaapi_recorder *r, VABufferID output_buf) { VACodedBufferSegment *segment; @@ -770,19 +778,22 @@ encoder_write_output(struct vaapi_recorder *r, VABufferID output_buf) status = vaMapBuffer(r-va_dpy, output_buf, (void **) segment); if (status != VA_STATUS_SUCCESS) - return -1; + return OUTPUT_WRITE_FATAL; if (segment-status VA_CODED_BUF_STATUS_SLICE_OVERFLOW_MASK) { r-encoder.output_size *= 2; vaUnmapBuffer(r-va_dpy, output_buf); - return -1; + return OUTPUT_WRITE_OVERFLOW; } count = write(r-output_fd, segment-buf, segment-size); vaUnmapBuffer(r-va_dpy, output_buf); - return count; + if (count 0) + return OUTPUT_WRITE_FATAL; + + return OUTPUT_WRITE_SUCCESS; } static void @@ -792,9 +803,8 @@ encoder_encode(struct vaapi_recorder *r, VASurfaceID input) VABufferID buffers[8]; int count = 0; - - int slice_type; - int ret, i; + int i, slice_type; + enum output_write_status ret; if ((r-frame_count % r-encoder.intra_period) == 0) slice_type = SLICE_TYPE_I; @@ -829,7 +839,10 @@ encoder_encode(struct vaapi_recorder *r, VASurfaceID input) output_buf = VA_INVALID_ID; vaDestroyBuffer(r-va_dpy, buffers[--count]); - } while (ret 0); + } while (ret == OUTPUT_WRITE_OVERFLOW); + + if (ret == OUTPUT_WRITE_FATAL) + r-error = errno; for (i = 0; i count; i++)
Re: [PATCH 1/2] Set to NULL event source after a call to wl_event_source_remove
On Fri, May 09, 2014 at 04:03:51PM +0200, Hardening wrote: This patch sets to NULL event sources after a call to wl_event_source_remove() has been made. We don't generally set freed memory to NULL, unless we rely on testing that to decide whether the pointer points to an object or not. Kristian --- src/clipboard.c | 3 ++- src/compositor-drm.c | 3 +++ src/compositor-rpi.c | 1 + src/compositor-x11.c | 2 ++ src/compositor.c | 6 +- src/evdev-touchpad.c | 1 + src/evdev.c | 4 +++- src/libinput-seat.c | 1 + src/logind-util.c| 2 ++ xwayland/launcher.c | 6 +- xwayland/selection.c | 9 +++-- 11 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/clipboard.c b/src/clipboard.c index 5a3a02d..0e25dc1 100644 --- a/src/clipboard.c +++ b/src/clipboard.c @@ -61,6 +61,7 @@ clipboard_source_unref(struct clipboard_source *source) if (source-event_source) { wl_event_source_remove(source-event_source); + source-event_source = NULL; close(source-fd); } wl_signal_emit(source-base.destroy_signal, @@ -90,8 +91,8 @@ clipboard_source_data(int fd, uint32_t mask, void *data) len = read(fd, p, size); if (len == 0) { wl_event_source_remove(source-event_source); - close(fd); source-event_source = NULL; + close(fd); } else if (len 0) { clipboard_source_unref(source); clipboard-source = NULL; diff --git a/src/compositor-drm.c b/src/compositor-drm.c index 5f59789..0110f41 100644 --- a/src/compositor-drm.c +++ b/src/compositor-drm.c @@ -2379,7 +2379,9 @@ drm_destroy(struct weston_compositor *ec) udev_input_destroy(d-input); wl_event_source_remove(d-udev_drm_source); + d-udev_drm_source = NULL; wl_event_source_remove(d-drm_source); + d-drm_source = NULL; destroy_sprites(d); @@ -2849,6 +2851,7 @@ drm_compositor_create(struct wl_display *display, err_udev_monitor: wl_event_source_remove(ec-udev_drm_source); + ec-udev_drm_source = NULL; udev_monitor_unref(ec-udev_monitor); err_drm_source: wl_event_source_remove(ec-drm_source); diff --git a/src/compositor-rpi.c b/src/compositor-rpi.c index 287451d..cbfb770 100644 --- a/src/compositor-rpi.c +++ b/src/compositor-rpi.c @@ -213,6 +213,7 @@ static void rpi_flippipe_release(struct rpi_flippipe *flippipe) { wl_event_source_remove(flippipe-source); + flippipe-source = NULL; close(flippipe-readfd); close(flippipe-writefd); } diff --git a/src/compositor-x11.c b/src/compositor-x11.c index 56b3228..9f171e7 100644 --- a/src/compositor-x11.c +++ b/src/compositor-x11.c @@ -485,6 +485,7 @@ x11_output_destroy(struct weston_output *output_base) (struct x11_compositor *)output-base.compositor; wl_event_source_remove(output-finish_frame_timer); + output-finish_frame_timer = NULL; if (compositor-use_pixman) { pixman_renderer_output_destroy(output_base); @@ -1424,6 +1425,7 @@ x11_destroy(struct weston_compositor *ec) struct x11_compositor *compositor = (struct x11_compositor *)ec; wl_event_source_remove(compositor-xcb_source); + compositor-xcb_source = NULL; x11_input_destroy(compositor); weston_compositor_shutdown(ec); /* destroys outputs, too */ diff --git a/src/compositor.c b/src/compositor.c index cd1ca9a..6ad3387 100644 --- a/src/compositor.c +++ b/src/compositor.c @@ -3736,8 +3736,12 @@ weston_compositor_shutdown(struct weston_compositor *ec) struct weston_output *output, *next; wl_event_source_remove(ec-idle_source); - if (ec-input_loop_source) + ec-idle_source = NULL; + + if (ec-input_loop_source) { wl_event_source_remove(ec-input_loop_source); + ec-input_loop_source = NULL; + } /* Destroy all outputs associated with this compositor */ wl_list_for_each_safe(output, next, ec-output_list, link) diff --git a/src/evdev-touchpad.c b/src/evdev-touchpad.c index 69f913a..360f87f 100644 --- a/src/evdev-touchpad.c +++ b/src/evdev-touchpad.c @@ -663,6 +663,7 @@ touchpad_destroy(struct evdev_dispatch *dispatch) touchpad-filter-interface-destroy(touchpad-filter); wl_event_source_remove(touchpad-fsm.timer_source); + touchpad-fsm.timer_source = NULL; free(dispatch); } diff --git a/src/evdev.c b/src/evdev.c index 888dfbd..a1bce2c 100644 --- a/src/evdev.c +++ b/src/evdev.c @@ -697,8 +697,10 @@ evdev_device_destroy(struct evdev_device *device) if (dispatch) dispatch-interface-destroy(dispatch); - if (device-source) + if (device-source) { wl_event_source_remove(device-source); + device-source = NULL; + } if (device-output)
Re: [PATCH 2/2] Handle OOM with signal events
On Fri, May 09, 2014 at 04:03:52PM +0200, Hardening wrote: This patch handles the case where a signal event source can not be created. --- src/compositor.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/compositor.c b/src/compositor.c index 6ad3387..047df8a 100644 --- a/src/compositor.c +++ b/src/compositor.c @@ -4197,6 +4197,7 @@ int main(int argc, char *argv[]) display = wl_display_create(); loop = wl_display_get_event_loop(display); + memset(signals, 0, sizeof(signals)); We set all entries of signals[4], and they're going to be valid signal source pointers or NULL if the allocation fails. No need to memset. signals[0] = wl_event_loop_add_signal(loop, SIGTERM, on_term_signal, display); signals[1] = wl_event_loop_add_signal(loop, SIGINT, on_term_signal, @@ -4208,6 +4209,9 @@ int main(int argc, char *argv[]) signals[3] = wl_event_loop_add_signal(loop, SIGCHLD, sigchld_handler, NULL); + if (!signals[0] || !signals[1] || !signals[2] || !signals[3]) + goto out_signals; + config = weston_config_parse(weston.ini); if (config != NULL) { weston_log(Using config file '%s'\n, @@ -4321,8 +4325,11 @@ int main(int argc, char *argv[]) wl_signal_emit(ec-destroy_signal, ec); - for (i = ARRAY_LENGTH(signals); i;) - wl_event_source_remove(signals[--i]); + out_signals: + for (i = ARRAY_LENGTH(signals); i; i--) { + if (signals[i-1]) We can just add the if condition to the existing loop, no need to iterate in reverse. Kristian + wl_event_source_remove(signals[i-1]); + } weston_compositor_xkb_destroy(ec); -- 1.8.1.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
Re: [PATCH weston] libinput-seat: literal values for WESTON_LIBINPUT_LOG_PRIORITY
On Fri, May 09, 2014 at 11:24:40AM -0700, U. Artie Eoff wrote: Only accept specific literal values from the environment variable WESTON_LIBINPUT_LOG_PRIORITY... debug, info, or error. Signed-off-by: U. Artie Eoff ullysses.a.e...@intel.com Thanks Artie, I think we can squeeze that in with the RC2. Kristian --- src/libinput-seat.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/libinput-seat.c b/src/libinput-seat.c index a38d470..d59ae42 100644 --- a/src/libinput-seat.c +++ b/src/libinput-seat.c @@ -271,8 +271,15 @@ udev_input_init(struct udev_input *input, struct weston_compositor *c, struct ud libinput_log_set_handler(libinput_log_func, NULL); log_priority = getenv(WESTON_LIBINPUT_LOG_PRIORITY); + if (log_priority) { - libinput_log_set_priority(strtol(log_priority, NULL, 10)); + if (strcmp(log_priority, debug) == 0) { + libinput_log_set_priority(LIBINPUT_LOG_PRIORITY_DEBUG); + } else if (strcmp(log_priority, info) == 0) { + libinput_log_set_priority(LIBINPUT_LOG_PRIORITY_INFO); + } else if (strcmp(log_priority, error) == 0) { + libinput_log_set_priority(LIBINPUT_LOG_PRIORITY_ERROR); + } } input-libinput = libinput_udev_create_for_seat(libinput_interface, input, -- 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
Re: [PATCH] desktop-shell: Fix black edges on scaled desktop pattern
On Fri, May 09, 2014 at 01:52:33PM -0700, Bill Spitzak wrote: Thanks, it looks like that setup worked, patch sent correctly now. Great, it's good to have git send-email working. As it turns out, I didn't have any problems applying your initial email, but I do have the fix whitespace option Pekka mentioned in my git config. Kristian On 05/09/2014 11:52 AM, Jonas Ådahl wrote: If you are using gmail, you can just use Googles SMTP server directly. The example configuration in the manual [0] even is a @gmail.com address setup. Jonas [0] http://git-scm.com/docs/git-send-email ___ 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
Re: [PATCH wayland 6/6] scanner: Generate macros for getting the 'since' version of an event
On Thu, May 08, 2014 at 11:39:49PM +0200, Jonas Ådahl wrote: This could be useful for compositors who need to be able to not send events if the client bound a version lower than the newest provided. Event version numbers are exposed as [INTERFACE_NAME]_[EVENT_NAME]_SINCE_VERSION for example wl_output.scale will have the version macro WL_OUTPUT_SCALE_SINCE_VERSION. Yeah, that's a little more readable I guess. This and previous patches applied, thanks. Kristian Signed-off-by: Jonas Ådahl jad...@gmail.com --- src/scanner.c | 13 + 1 file changed, 13 insertions(+) diff --git a/src/scanner.c b/src/scanner.c index 28fadb0..80c466e 100644 --- a/src/scanner.c +++ b/src/scanner.c @@ -579,6 +579,18 @@ emit_opcodes(struct wl_list *message_list, struct interface *interface) } static void +emit_opcode_versions(struct wl_list *message_list, struct interface *interface) +{ + struct message *m; + + wl_list_for_each(m, message_list, link) + printf(#define %s_%s_SINCE_VERSION\t%d\n, +interface-uppercase_name, m-uppercase_name, m-since); + + printf(\n); +} + +static void emit_type(struct arg *a) { switch (a-type) { @@ -1004,6 +1016,7 @@ emit_header(struct protocol *protocol, int server) if (server) { emit_structs(i-request_list, i); emit_opcodes(i-event_list, i); + emit_opcode_versions(i-event_list, i); emit_event_wrappers(i-event_list, i); } else { emit_structs(i-event_list, i); -- 1.9.1 ___ 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
Re: [PATCH weston 5/5] tests: rename xwayland test
On Wed, May 07, 2014 at 04:26:29PM +0300, Pekka Paalanen wrote: From: Pekka Paalanen pekka.paala...@collabora.co.uk If the test is named xwayland.weston, then the automake test harness keys it off xwayland.log. Making xwayland.log runs the test. The test harness has implicit rules to create a %.log from all of %$TEST_EXTENSIONS. So we have implicit rules to create %.log from %.la and %.log from %.weston. We also build xwayland.so, which produces xwayland.la. When the test harness goes running the xwayland test, it ends up using the %.la rule, which is wrong. It passes xwayland.la as the test name to weston-tests-env, which then loads it as a plugin into Weston and waits for Weston to exit. Which it never does. Fix this by making the test have a different name than the Xwayland plugin. Signed-off-by: Pekka Paalanen pekka.paala...@collabora.co.uk All applied, thanks Pekka. Kristian --- Makefile.am | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile.am b/Makefile.am index a247c3d..177ce2e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -927,10 +927,10 @@ buffer_count_weston_LDADD = libtest-client.la $(EGL_TESTS_LIBS) endif if ENABLE_XWAYLAND_TEST -weston_tests += xwayland.weston -xwayland_weston_SOURCES = tests/xwayland-test.c -xwayland_weston_CFLAGS = $(GCC_CFLAGS) $(XWAYLAND_TEST_CFLAGS) -xwayland_weston_LDADD = libtest-client.la $(XWAYLAND_TEST_LIBS) +weston_tests += xwayland-test.weston +xwayland_test_weston_SOURCES = tests/xwayland-test.c +xwayland_test_weston_CFLAGS = $(GCC_CFLAGS) $(XWAYLAND_TEST_CFLAGS) +xwayland_test_weston_LDADD = libtest-client.la $(XWAYLAND_TEST_LIBS) endif matrix_test_SOURCES =\ -- 1.8.5.5 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston] editor: Fix cursor positioning with pointer and touch
On Thu, May 08, 2014 at 02:55:50PM +0300, Ander Conselvan de Oliveira wrote: The calculation off the vertical offset between the widget coordinates and where the text was rendered was wrong. It was using the constant for horizontal offset for that too. --- clients/editor.c | 33 +++-- 1 file changed, 23 insertions(+), 10 deletions(-) That fixes it here, thanks. I added Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=78411 to the commit message. Kristian diff --git a/clients/editor.c b/clients/editor.c index 3b00833..f3f6141 100644 --- a/clients/editor.c +++ b/clients/editor.c @@ -1011,7 +1011,17 @@ text_entry_draw_cursor(struct text_entry *entry, cairo_t *cr) cairo_stroke(cr); } -static const int text_offset_left = 10; +static int +text_offset_left(struct rectangle *allocation) +{ + return 10; +} + +static int +text_offset_top(struct rectangle *allocation) +{ + return allocation-height / 2; +} static void text_entry_redraw_handler(struct widget *widget, void *data) @@ -1048,7 +1058,9 @@ text_entry_redraw_handler(struct widget *widget, void *data) cairo_set_source_rgba(cr, 0, 0, 0, 1); - cairo_translate(cr, text_offset_left, allocation.height / 2); + cairo_translate(cr, + text_offset_left(allocation), + text_offset_top(allocation)); if (!entry-layout) entry-layout = pango_cairo_create_layout(cr); @@ -1075,6 +1087,7 @@ text_entry_motion_handler(struct widget *widget, { struct text_entry *entry = data; struct rectangle allocation; + int tx, ty; if (!entry-button_pressed) { return CURSOR_IBEAM; @@ -1082,10 +1095,10 @@ text_entry_motion_handler(struct widget *widget, widget_get_allocation(entry-widget, allocation); - text_entry_set_cursor_position(entry, -x - allocation.x - text_offset_left, -y - allocation.y - text_offset_left, -false); + tx = x - allocation.x - text_offset_left(allocation); + ty = y - allocation.y - text_offset_top(allocation); + + text_entry_set_cursor_position(entry, tx, ty, false); return CURSOR_IBEAM; } @@ -1105,8 +1118,8 @@ text_entry_button_handler(struct widget *widget, widget_get_allocation(entry-widget, allocation); input_get_position(input, x, y); - x -= allocation.x + text_offset_left; - y -= allocation.y + text_offset_left; + x -= allocation.x + text_offset_left(allocation); + y -= allocation.y + text_offset_top(allocation); editor = window_get_user_data(entry-window); @@ -1149,8 +1162,8 @@ text_entry_touch_handler(struct widget *widget, struct input *input, widget_get_allocation(entry-widget, allocation); - x = tx - (allocation.x + text_offset_left); - y = ty - (allocation.y + text_offset_left); + x = tx - (allocation.x + text_offset_left(allocation)); + y = ty - (allocation.y + text_offset_top(allocation)); editor = window_get_user_data(entry-window); text_entry_activate(entry, seat); -- 1.8.3.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