Re: [PATCH v1] Added touch support to wayland backend
Can anyone please comment on it? :)
BR
imran
On Wed, Dec 17, 2014 at 4:11 PM, Imran Zaman imran.za...@gmail.com wrote:
Nested westons (with wayland backend as child weston) is one possible
use case for the needed touch support.
Signed-off-by: Imran Zaman imran.za...@gmail.com
---
src/compositor-wayland.c | 59
1 file changed, 59 insertions(+)
diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
index 65bce39..f125210 100644
--- a/src/compositor-wayland.c
+++ b/src/compositor-wayland.c
@@ -1581,6 +1581,53 @@ static const struct wl_keyboard_listener
keyboard_listener = {
};
static void
+input_handle_touch_down(void *data, struct wl_touch *wl_touch,
+ uint32_t serial, uint32_t time,
+ struct wl_surface *surface, int32_t id, wl_fixed_t
x_w,
+ wl_fixed_t y_w)
+{
+ struct wayland_input *input = data;
+ notify_touch(input-base, time, id, x_w, y_w, WL_TOUCH_DOWN);
+}
+
+static void
+input_handle_touch_up(void *data, struct wl_touch *wl_touch,
+ uint32_t serial, uint32_t time, int32_t id)
+{
+ struct wayland_input *input = data;
+ notify_touch(input-base, time, id, 0, 0, WL_TOUCH_UP);
+}
+
+static void
+input_handle_touch_motion(void *data, struct wl_touch *wl_touch,
+ uint32_t time, int32_t id, wl_fixed_t x_w,
+ wl_fixed_t y_w)
+{
+ struct wayland_input *input = data;
+ notify_touch(input-base, time, id, x_w, y_w, WL_TOUCH_MOTION);
+}
+
+static void
+input_handle_touch_frame(void *data, struct wl_touch *wl_touch)
+{
+ struct wayland_input *input = data;
+ notify_touch_frame(input-base);
+}
+
+static void
+input_handle_touch_cancel(void *data, struct wl_touch *wl_touch)
+{
+}
+
+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,
+};
+
+static void
input_handle_capabilities(void *data, struct wl_seat *seat,
enum wl_seat_capability caps)
{
@@ -1606,6 +1653,18 @@ input_handle_capabilities(void *data, struct
wl_seat *seat,
wl_keyboard_destroy(input-parent.keyboard);
input-parent.keyboard = NULL;
}
+
+ if ((caps WL_SEAT_CAPABILITY_TOUCH) !input-parent.touch) {
+ input-parent.touch = wl_seat_get_touch(seat);
+ weston_seat_init_touch (input-base);
+ wl_touch_set_user_data(input-parent.touch, input);
+ wl_touch_add_listener(input-parent.touch,
+ touch_listener, input);
+ } else if (!(caps WL_SEAT_CAPABILITY_TOUCH)
input-parent.touch) {
+ weston_seat_release_touch (input-base);
+ wl_touch_destroy(input-parent.touch);
+ input-parent.touch = NULL;
+ }
}
static void
--
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] fullscreen-shell: Fix modeset on transformed outputs
Hi Ekstrand!
I have tested it in nested compositors case; unfortunately it is not enough
and doesn't work :-)
The reason is that drm compositor (choose_mode) function rejects the mode
if width and height are swapped so similar patch is needed for choose_mode
in drm compositor based on transform.
I tested the setup as below..
- System compositor (drm backend) with weston.ini file containing
transform=90 for the output.
- Child compositor (wayland backend)
- child compositor receives e.g. 1920x1080 + transform= 90 info.
- it creates the surface and DOESN'T do any transformation (other than
swapping width and height) as it is disabled.
- then surface is presented to system compositor's fullscreen shell
- fullscreen shell uses the outputs' data received from system compositor
when module is initialized and surface data received from child
compositor which may not be in sync..
- fullscreen shell then ask system compositor to switch mode
- system compositor verifies the mode first (if not it fails)
- then system compositor does the transformation
Hope it gives u some idea to update the patch. I would be eager to test if
you update the patch :-)
BR
imran
On Thu, Jan 8, 2015 at 6:57 PM, Jason Ekstrand ja...@jlekstrand.net wrote:
Previously, we blindly created a mode for the output based on surface size
and completely ignoring the output transform. This caused modesets to fail
on outputs that were transformed by 90 or 270 degrees. We should be
swapping the width and the height in this case.
---
fullscreen-shell/fullscreen-shell.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/fullscreen-shell/fullscreen-shell.c
b/fullscreen-shell/fullscreen-shell.c
index 35e6d8f..5fd7cd6 100644
--- a/fullscreen-shell/fullscreen-shell.c
+++ b/fullscreen-shell/fullscreen-shell.c
@@ -463,9 +463,28 @@ fs_output_configure_for_mode(struct fs_output *fsout,
surf_x, surf_y,
surf_width, surf_height);
+ /* The actual output mode is in physical units. We need to
+* transform the surface size to physical unit size by flipping ans
+* possibly scaling it.
+*/
+ switch (fsout-output-transform) {
+ case WL_OUTPUT_TRANSFORM_90:
+ case WL_OUTPUT_TRANSFORM_FLIPPED_90:
+ case WL_OUTPUT_TRANSFORM_270:
+ case WL_OUTPUT_TRANSFORM_FLIPPED_270:
+ mode.width = surf_height * fsout-output-native_scale;
+ mode.height = surf_width * fsout-output-native_scale;
+ break;
+
+ case WL_OUTPUT_TRANSFORM_NORMAL:
+ case WL_OUTPUT_TRANSFORM_FLIPPED:
+ case WL_OUTPUT_TRANSFORM_180:
+ case WL_OUTPUT_TRANSFORM_FLIPPED_180:
+ default:
+ mode.width = surf_width * fsout-output-native_scale;
+ mode.height = surf_height * fsout-output-native_scale;
+ }
mode.flags = 0;
- mode.width = surf_width * fsout-output-native_scale;
- mode.height = surf_height * fsout-output-native_scale;
mode.refresh = fsout-pending.framerate;
ret = weston_output_mode_switch_to_temporary(fsout-output, mode,
--
2.2.0
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v1] Added touch support to wayland backend
Nested westons (with wayland backend as child weston) is one possible
use case for the needed touch support.
Signed-off-by: Imran Zaman imran.za...@gmail.com
---
src/compositor-wayland.c | 59
1 file changed, 59 insertions(+)
diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
index 65bce39..f125210 100644
--- a/src/compositor-wayland.c
+++ b/src/compositor-wayland.c
@@ -1581,6 +1581,53 @@ static const struct wl_keyboard_listener
keyboard_listener = {
};
static void
+input_handle_touch_down(void *data, struct wl_touch *wl_touch,
+ uint32_t serial, uint32_t time,
+ struct wl_surface *surface, int32_t id, wl_fixed_t x_w,
+ wl_fixed_t y_w)
+{
+ struct wayland_input *input = data;
+ notify_touch(input-base, time, id, x_w, y_w, WL_TOUCH_DOWN);
+}
+
+static void
+input_handle_touch_up(void *data, struct wl_touch *wl_touch,
+ uint32_t serial, uint32_t time, int32_t id)
+{
+ struct wayland_input *input = data;
+ notify_touch(input-base, time, id, 0, 0, WL_TOUCH_UP);
+}
+
+static void
+input_handle_touch_motion(void *data, struct wl_touch *wl_touch,
+ uint32_t time, int32_t id, wl_fixed_t x_w,
+ wl_fixed_t y_w)
+{
+ struct wayland_input *input = data;
+ notify_touch(input-base, time, id, x_w, y_w, WL_TOUCH_MOTION);
+}
+
+static void
+input_handle_touch_frame(void *data, struct wl_touch *wl_touch)
+{
+ struct wayland_input *input = data;
+ notify_touch_frame(input-base);
+}
+
+static void
+input_handle_touch_cancel(void *data, struct wl_touch *wl_touch)
+{
+}
+
+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,
+};
+
+static void
input_handle_capabilities(void *data, struct wl_seat *seat,
enum wl_seat_capability caps)
{
@@ -1606,6 +1653,18 @@ input_handle_capabilities(void *data, struct wl_seat
*seat,
wl_keyboard_destroy(input-parent.keyboard);
input-parent.keyboard = NULL;
}
+
+ if ((caps WL_SEAT_CAPABILITY_TOUCH) !input-parent.touch) {
+ input-parent.touch = wl_seat_get_touch(seat);
+ weston_seat_init_touch (input-base);
+ wl_touch_set_user_data(input-parent.touch, input);
+ wl_touch_add_listener(input-parent.touch,
+ touch_listener, input);
+ } else if (!(caps WL_SEAT_CAPABILITY_TOUCH) input-parent.touch) {
+ weston_seat_release_touch (input-base);
+ wl_touch_destroy(input-parent.touch);
+ input-parent.touch = NULL;
+ }
}
static void
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v1] Added string conversion utility functions
Interesting read.. have some arguments about the raised points.. but
lets not go in that direction..
Can you guys suggest to do some changes or shall i drop the patch
altogether? if you are happy with current implementation lets live
with it.. I noticed bug when weston was not working properly due to
improper error handling of strtol in one place, so I thought it would
be good idea to fix it wherever it is used (and hence this patch) but
seems that it is not always the case :-)
BR
imran
p.s: To me it was really simple to understand that in MOST (=NOT ALL)
of the cases str-to-number conversion was done using strtol/strtoul
but without proper error handling. I moved it to single place to make
the life easier IMHO...
On Mon, Dec 1, 2014 at 8:30 PM, Bill Spitzak spit...@gmail.com wrote:
On 12/01/2014 04:10 AM, Pekka Paalanen wrote:
other = strtol(pid, end, 0);
if (end != pid + 10) {
weston_log(can't parse lock file %s\n,
lockfile);
close(fd);
errno = EEXIST;
return -1;
}
'pid' is a fixed size string read in from the lock file, which is
converted into a number of type pid_t. Because the number is assumed to
be printed by %10d\n, the file should have at least 11 bytes
available, and we assume all the 10 characters form a valid number
(with leading spaces). It's all just a way to avoid dealing with
unexpected EOF when reading the file, and to avoid not knowing in
advance how many characters long the number is.
This code still wants to parse the whole string as a single number, but
it also knows the number will end in a newline instead of nul. It
wouldn't be difficult to replace that newline with nul before parsing,
if you really want to convert this code to use a helper. But while
doing so, you have to ask yourself: does this actually make the code
any easier to understand or more correct?
I believe you are correct, and this is a good indication that blindly
inserting the replacement function is not a good idea.
The original code failed if there was a digit at offset 11 (as well as other
reasons for failing). The proposed replacement failed if offset 11 was
anything other than NUL. This is different. When I said the endptr was not
needed, my proposal actually has the exact same mistake.
This does bring up a question as to whether the helper function should eat
trailing whitespace.
But also that you can't drop the wrapper in everywhere.
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v1] Added string conversion utility functions
Thanks Bill for your comments. Please see my comments inline.
On Fri, Nov 21, 2014 at 9:31 PM, Bill Spitzak spit...@gmail.com wrote:
There is no need to pass endptr, because your functions will always fail if
it is not set to point to the terminating null.
[IZ] endptr is passed other than null; please see the whole patch. why
do u think it will fail?
I also suspect that the base is never used. In addition a possible future
fix would be to accept 0x for hex but a leading zero is decimal, not octal,
which means this function would generate the base.
[IZ] Base is used in weston code as well, please see the whole patch.
I am also worried about passing the out ptr. Depending on the sizes of int,
long, and long long this will easily result in code that compiles on one
platform but not on another. Returning the value would avoid this and reduce
the number of functions. Instead pass a pointer to a variable to store the
error into, or use errno?
Would prefer you not test the addresses of out parameters for null. It
should crash at this point, as this is a programming error, not a runtime
error. Returning false will make it hard to figure out what is wrong.
[IZ] can u please elaborate it bit more as I can add more test cases
to cover the corner case if I understand clearly what do you want to
highlight?
It is useful to pass -1 for strtoul and this should continue to work. It is
the only easily-recognized value for largest number possible. May be ok to
not allow other negative numbers however.
[IZ] IMO its good to keep the APIs simple and homogeneous with
appropriate data structures needed for the input
BR
imran
On 11/21/2014 07:38 AM, Imran Zaman wrote:
+static bool
+convert_strtol(const char *str, char **endptr, int base, long *val)
+{
+ char *end = NULL;
+ long v;
+ int prev_errno = errno;
+
+ if (!str || !val)
+ return false;
Please do not do the above tests!
+ if (!endptr)
+ endptr = end;
+
+ errno = 0;
+ v = strtol(str, endptr, base);
+ if (errno != 0 || *endptr == str || **endptr != '\0')
+ return false;
+
+ errno = prev_errno;
+ *val = v;
+ return true;
+}
+
+static bool
+convert_strtoul (const char *str, char **endptr, int base, unsigned long
*val)
+{
+ char *end = NULL;
+ unsigned long v;
+ int i = 0;
+ int prev_errno = errno;
+
+ if (!str || !val)
+ return false;
+ /* check for negative numbers */
+ while (str[i]) {
+ if (!isspace(str[i])) {
+ if (str[i] == '-')
+ return false;
+ else
+ break;
+ }
+ i++;
+ }
You could do the above test afterwards and check for and allow the -1 value.
+
+ if (!endptr)
+ endptr = end;
+
+ errno = 0;
+ v = strtoul(str, endptr, base);
+ if (errno != 0 || *endptr == str || **endptr != '\0')
+ return false;
+
+ errno = prev_errno;
+ *val = v;
+ return true;
+}
+
+WL_EXPORT bool
+weston_strtoi(const char *str, char **endptr, int base, int *val)
+{
+ long v;
+
+ if (!convert_strtol(str, endptr, base, v) || v INT_MAX
+ || v INT_MIN)
+ return false;
+
+ *val = (int)v;
+ return true;
+}
+
+WL_EXPORT bool
+weston_strtol(const char *str, char **endptr, int base, long *val)
+{
+ return convert_strtol(str, endptr, base, val);
+}
+
+WL_EXPORT bool
+weston_strtoui(const char *str, char **endptr, int base, unsigned int
*val)
+{
+ unsigned long v;
+
+ if (!convert_strtoul(str, endptr, base, v) || v UINT_MAX)
+ return false;
+
+ *val = (unsigned int)v;
+ return true;
+}
+
+WL_EXPORT bool
+weston_strtoul(const char *str, char **endptr, int base, unsigned long
*val)
+{
+ return convert_strtoul(str, endptr, base, val);
+}
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Wayland and Weston in Patchwork
Hi Somehow I dont see the patch below in patch work: http://lists.freedesktop.org/archives/wayland-devel/2014-November/018410.html Any idea what is wrong with it or there is some sort of filtering or ? BR imran On Thu, Nov 20, 2014 at 12:47 AM, Bill Spitzak spit...@gmail.com wrote: On 11/19/2014 07:30 AM, Daniel Stone wrote: I don't think there's any reason to be as precious with Patchwork access as we are with others. We need all the help with triage we can get, all the changes are cosmetic and logged, and if anyone screws around with it and misrepresents mailing list consensus for any reason, then we can gently slap them around with a wet trout. Bill, I've added you as an admin now, so you should be able to change these. I took a look and that certainly looks like it will work. What I want to do is mass-change my patches to superseded. Since everybody seems to contribute large sets of patches I think everybody is interested in this. It does seem it should not be too hard of a patchwork modification to make this box work for everybody, just skipping changes on patches that don't belong to them, or popping up an error if they are not allowed. But it seems pretty safe to make everybody an administrator too. ___ 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] increase server listen queue to 8
Actually we were connecting multiple simultaneous child/session
westons to system weston and sometime it didn't succeed. Nevertheless
we are not sure that this can be one of the reasons and we couldnt
spot the problem precisely (we were testing it in Tizen with kernel
version: 3.14.19-10.4)
We thought this can be one of the reasons if multiple clients ask for
simultaneous connections.
As you said its harmless, its better to increase the length anyways..
Thanks for the links. I can push the patch again after rebasing and
changing queue length to 128.
BR
imran
On Wed, Nov 19, 2014 at 12:22 PM, Pekka Paalanen ppaala...@gmail.com wrote:
On Wed, 15 Oct 2014 16:43:34 +0300
Imran Zaman imran.za...@gmail.com wrote:
Hi
This will allow more than 1 simultaneous client connections to the server
without the possibility of connection refused error. possible use case
is multiple session
compositors can connect to the system compositor simultaneously.
diff --git a/src/wayland-server.c b/src/wayland-server.c
index 674aeca..e3b7d9f 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -1126,7 +1126,7 @@ _wl_display_add_socket(struct wl_display
*display, struct wl_socket *s)
return -1;
}
- if (listen(s-fd, 1) 0) {
+ if (listen(s-fd, 8) 0) {
wl_log(listen() failed with error: %m\n);
return -1;
}
This patch is obviously format-broken, but the change itself seems
harmless to me. Since no-one has commented on it, I suppose no-one is
against it.
I also found:
http://utcc.utoronto.ca/~cks/space/blog/unix/ListenBacklogMeaning
which suggests that the number is fairly arbitrary.
I couldn't find a clear explanation what the backlog argument means for
unix domain sockets, either. However, I did find a random comment:
http://stackoverflow.com/questions/19221105/connect-with-unix-domain-socket-and-full-backlog
That suggests that on Linux you might rather see EAGAIN than
ECONNREFUSED. Sounds like it's not completely true?
So, since we have no reason to artificially limit the queue, let's go
with something big. Say, 128.
Would you like to spin a proper patch, or shall I just rewrite this in
my own name?
Would be nice to hear how you actually hit the connection refused, and
on what OS/kernel.
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: Wayland and Weston in Patchwork
Exactly git send-email did ask for utf-8 and i guess i entered y.. shall I not? and no idea why git is making it multipart.. Shall i try to resend the patch with utf-8 question as 'no'? BR imran On Mon, Nov 24, 2014 at 3:50 PM, Pekka Paalanen ppaala...@gmail.com wrote: On Mon, 24 Nov 2014 15:33:51 +0200 Imran Zaman imran.za...@gmail.com wrote: Hi Somehow I dont see the patch below in patch work: http://lists.freedesktop.org/archives/wayland-devel/2014-November/018410.html Any idea what is wrong with it or there is some sort of filtering or ? Hi, yeah, I tried to ask you about it in IRC. The strange things I see in that email are: Content-Type: multipart/mixed; boundary1810149695== (why multipart?) and Content-Type: text/plain; charset=y I wonder if one of those caused Patchwork to ignore it. Patchwork also seems to ignore patches that contain only binary diffs: http://lists.freedesktop.org/archives/wayland-devel/2014-November/018362.html Imran, did git-send-email ask you what charset to use for these patches, suggesting UTF-8 by default, and you perhaps replied y? That would put the y as charset there, I think. :-) Nothing else comes to mind... Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v2] Increase listen queue to 128
This will allow more than 1 simultaneous client connections to the server
without the possibility of connection refused error.
Signed-off-by: Imran Zaman imran.za...@gmail.com
---
src/wayland-server.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/wayland-server.c b/src/wayland-server.c
index c40d300..c845dd6 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -1139,7 +1139,7 @@ _wl_display_add_socket(struct wl_display *display, struct
wl_socket *s)
return -1;
}
- if (listen(s-fd, 1) 0) {
+ if (listen(s-fd, 128) 0) {
wl_log(listen() failed with error: %m\n);
return -1;
}
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v1] Added string conversion utility functions
On Tue, Nov 25, 2014 at 1:15 AM, Bill Spitzak spit...@gmail.com wrote: On 11/24/2014 11:32 AM, Imran Zaman wrote: [IZ2] This is the case where endptr is used in patch. I can remove endptr but it seems its used so i have to keep the existing functionality in place. + if (!weston_strtoi(pid, end, 0, other) || end != pid + 10) Use strlen(pid) != 10 instead. This will not exactly replace the implemented logic e.g. pid=123abcdefg. strlen(pid) = 10 end = 3 (end != pid + 10) So cant use the solution which u propose... [IZ2] Sorry still not able to grasp what you want to highlight. Can you please give an example? If I write the following code: if (strtol(string, 0, 0, 0)) ... I want it to crash so I notice that I passed 0 for the val output pointer. There is no reason to every pass null here so it must be a programmer mistake. As you have written it this will act like there is a syntax error in string, which could lead a person trying to debug their code down the wrong path. Sorry, why is it a programmer mistake and how would it crash? Let me add the exact test code and will come back to you. It should not crash for sure IMO. Also if the pointer is passed as (structure-member) it is going to crash anyway if structure is null and the offset to member is non-zero. I would think most errors causing the pointer to be invalid will be of this form so this is not preventing any crash. This cant be prevented anyway. we are using pointers everywhere in the whole weston code base. So its the user of the function who have to pass valid pointer or NULL. I suppose null could be used as I am only testing if it parses and don't want the number but it is not implemented this way, and I kind of doubt much code is interested in that test. At least one case above is using endptr.. if we can replace it with exactly similarly logic, I can remove it otherwies I dont see any harm in its usage ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v1] Added string conversion utility functions
Change-Id: I50900852311604a8c31313bbfb1d137c495d2269
Signed-off-by: Imran Zaman imran.za...@gmail.com
---
Makefile.am | 14 +-
clients/multi-resource.c| 8 +-
clients/terminal.c | 8 +-
clients/wscreensaver-glue.c | 6 +-
shared/config-parser.c | 10 +-
shared/option-parser.c | 10 +-
shared/str-util.c | 133 ++
shared/str-util.h | 43 ++
src/compositor-rdp.c| 3 +-
src/compositor.c| 12 +-
src/libbacklight.c | 4 +-
tests/strutil-test.c| 322
xwayland/launcher.c | 5 +-
13 files changed, 542 insertions(+), 36 deletions(-)
create mode 100644 shared/str-util.c
create mode 100644 shared/str-util.h
create mode 100644 tests/strutil-test.c
diff --git a/Makefile.am b/Makefile.am
index 2fc171e..a78a8d2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -802,6 +802,8 @@ libshared_la_SOURCES = \
shared/config-parser.c \
shared/option-parser.c \
shared/config-parser.h \
+ shared/str-util.c \
+ shared/str-util.h \
shared/os-compatibility.c \
shared/os-compatibility.h
@@ -838,6 +840,7 @@ TESTS = $(shared_tests) $(module_tests) $(weston_tests)
shared_tests = \
config-parser.test \
+ strutil.test\
vertex-clip.test
module_tests = \
@@ -912,6 +915,9 @@ libtest_runner_la_CFLAGS = $(GCC_CFLAGS)
$(COMPOSITOR_CFLAGS)
config_parser_test_SOURCES = tests/config-parser-test.c
config_parser_test_LDADD = libshared.la libtest-runner.la $(COMPOSITOR_LIBS)
+strutil_test_SOURCES = tests/strutil-test.c
+strutil_test_LDADD = libshared.la libtest-runner.la $(COMPOSITOR_LIBS)
+
vertex_clip_test_SOURCES = \
tests/vertex-clip-test.c\
src/vertex-clipping.c \
@@ -988,9 +994,11 @@ matrix_test_LDADD = -lm -lrt
if BUILD_SETBACKLIGHT
noinst_PROGRAMS += setbacklight
-setbacklight_SOURCES = \
- tests/setbacklight.c\
- src/libbacklight.c \
+setbacklight_SOURCES = \
+ tests/setbacklight.c\
+ shared/str-util.c \
+ shared/str-util.h \
+ src/libbacklight.c \
src/libbacklight.h
setbacklight_CFLAGS = $(AM_CFLAGS) $(SETBACKLIGHT_CFLAGS)
setbacklight_LDADD = $(SETBACKLIGHT_LIBS)
diff --git a/clients/multi-resource.c b/clients/multi-resource.c
index 0dc2c74..5d8d2ed 100644
--- a/clients/multi-resource.c
+++ b/clients/multi-resource.c
@@ -39,6 +39,7 @@
#include wayland-client.h
#include ../shared/os-compatibility.h
+#include ../shared/str-util.h
struct device {
enum { KEYBOARD, POINTER } type;
@@ -443,14 +444,11 @@ create_device(struct display *display, const char
*time_desc, int type)
return -1;
}
- errno = 0;
- start_time = strtoul(time_desc, tail, 10);
- if (errno)
+ if (!weston_strtoi(time_desc, tail, 10, start_time))
goto error;
if (*tail == ':') {
- end_time = strtoul(tail + 1, tail, 10);
- if (errno || *tail != '\0')
+ if (!weston_strtoi(tail + 1, tail, 10, end_time))
goto error;
} else if (*tail != '\0') {
goto error;
diff --git a/clients/terminal.c b/clients/terminal.c
index 7c37101..cef8437 100644
--- a/clients/terminal.c
+++ b/clients/terminal.c
@@ -43,6 +43,7 @@
#include wayland-client.h
#include ../shared/config-parser.h
+#include ../shared/str-util.h
#include window.h
static int option_fullscreen;
@@ -1277,11 +1278,12 @@ static void
handle_osc(struct terminal *terminal)
{
char *p;
- int code;
+ int code = -1;
terminal-escape[terminal-escape_length++] = '\0';
p = terminal-escape[2];
- code = strtol(p, p, 10);
+
+ weston_strtoi(p, p, 10, code);
if (*p == ';') p++;
switch (code) {
@@ -1324,7 +1326,7 @@ handle_escape(struct terminal *terminal)
p++;
i++;
} else {
- args[i] = strtol(p, p, 10);
+ weston_strtoi(p, p, 10, args[i]);
set[i] = 1;
}
}
diff --git a/clients/wscreensaver-glue.c b/clients/wscreensaver-glue.c
index 55d0a8c..ce5f92b 100644
--- a/clients/wscreensaver-glue.c
+++ b/clients/wscreensaver-glue.c
@@ -21,6 +21,7 @@
*/
#include wscreensaver-glue.h
+#include ../shared/str-util.h
double frand(double f)
{
@@ -70,6 +71,7 @@ read_xpm_color(uint32_t *ctable, const
Re: [PATCH wayland 2/2] support specifying custom directories for the client and server
... corrected jussi email address..
BR
irman
On Wed, Nov 19, 2014 at 12:56 PM, Pekka Paalanen ppaala...@gmail.com wrote:
On Wed, 15 Oct 2014 17:36:27 +0300
Imran Zaman imran.za...@gmail.com wrote:
Hi
support for adjusting socket access rights to allow group of users to
connect to the socket.
This is used for nested compositor architectures.
-
diff --git a/src/wayland-server.c b/src/wayland-server.c
index ce1eca8..b1ca5e6 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -39,6 +39,8 @@
#include fcntl.h
#include sys/file.h
#include sys/stat.h
+#include sys/types.h
+#include grp.h
#include ffi.h
#include wayland-private.h
@@ -1079,6 +1081,10 @@ wl_socket_init_for_display_name(struct
wl_socket *s, const char *name)
{
int name_size;
const char *runtime_dir;
+ const char *socket_mode_str;
+ const char *socket_group_str;
+ const struct group *socket_group;
+ unsigned socket_mode;
runtime_dir = getenv(WAYLAND_SERVER_DIR);
if (runtime_dir == NULL)
@@ -1133,6 +1139,18 @@ _wl_display_add_socket(struct wl_display
*display, struct wl_socket *s)
return -1;
}
+ socket_group_str = getenv(WAYLAND_SERVER_GROUP);
+ if (socket_group_str != NULL) {
+ socket_group = getgrnam(socket_group_str);
+ if (socket_group != NULL)
+ chown(s-addr.sun_path, -1, socket_group-gr_gid);
+ }
+ socket_mode_str = getenv(WAYLAND_SERVER_MODE);
+ if (socket_mode_str != NULL) {
+ if (sscanf(socket_mode_str, %o, socket_mode) 0)
+ chmod(s-addr.sun_path, socket_mode);
+ }
+
s-source = wl_event_loop_add_fd(display-loop, s-fd,
WL_EVENT_READABLE,
socket_data, display);
Hi,
I'm not sure what to think of these two patches. Are environment
variables WAYLAND_CLIENT_DIR, WAYLAND_SERVER_DIR, and the above really
the right API for this?
Your system compositor has to be specially written to be a system
compositor and open multiple listening sockets anyway, so I think this
is the wrong way for the server side stuff. For the server side, I
don't think we should have anything of this controlled via environment
variables but new API functions. What those API functions should be is
a good question.
I believe you don't need any additions in libwayland-server, in fact.
You have very special requirements in how and where to create the
sockets, so you can do that all yourself, up to the point where you have
a socket fd listening for new connections. Then you can use
wl_event_loop_add_fd() (if you even use the libwayland-server event
loops stuff) to monitor the fd. When a client connects, you get a
callback, call accept(), and pass the new fd to wl_client_create().
Hmm, wait, why am I assuming multiple listening sockets... What is your
architecture here, exactly?
I have very hard time deciding if we should allow the environment to
overwrite the server and client assumptions on where the socket is. It
would be easier for me to accept new API functions that operate with
absolute paths or existing fds explicitly, but those of course require
both servers and clients to be written to use them.
Comments, anyone?
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] wl_strtol and wl_strtoul utility functions are added
I have already taken off that patch from the patchwork list... and pushed a simpler version... http://lists.freedesktop.org/archives/wayland-devel/2014-November/018030.html BR imran On Wed, Nov 5, 2014 at 5:08 PM, Pekka Paalanen ppaala...@gmail.com wrote: On Wed, 15 Oct 2014 22:04:46 +0300 Imran Zaman imran.za...@gmail.com wrote: The reason is that strtol is used at many places in weston/wayland.. and its not covering all the error cases everywhere (i.e. its buggy).. so its better to encapsulate it in a function with all the input and output checks... it can be moved to weston if its sound such a big deal... libwayland-* are not generic utility libraries. We will not export this kind of functions, even if it might seem useful on a first glance. Libwayland is not a bucket where you throw everything you can imagine to be useful also elsewhere. Internal helpers would be a different matter. On Wed, Oct 15, 2014 at 9:42 PM, Jason Ekstrand ja...@jlekstrand.net wrote: I don't see how this belongs in libwayland. Sure, we use strtol twice, but I don't think that warrants adding 100 lines of wrapper functions and test cases. --Jason Ekstrand On Wed, Oct 15, 2014 at 6:16 AM, Rémi Denis-Courmont r...@remlab.net wrote: Le 2014-10-15 16:14, Imran Zaman a écrit : Hi The patch is used to replace strtol and strtoul with wl_strtol and wl_strtoul with inputs and result checks. I don't know where Wayland developers stand on this, but I would rather the client library function calls not clobber errno to zero. Yeah, that's probably a good rule. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v2] Added more error checks when strtol function is used
Signed-off-by: Imran Zaman imran.za...@gmail.com
---
src/scanner.c| 5 -
src/wayland-client.c | 6 +-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/src/scanner.c b/src/scanner.c
index 5e5152b..9ba34e8 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -405,9 +405,12 @@ start_element(void *data, const char *element_name, const
char **atts)
message-destructor = 0;
if (since != NULL) {
+ int prev_errno = errno, new_errno;
errno = 0;
version = strtol(since, end, 0);
- if (errno == EINVAL || end == since || *end != '\0')
+ new_errno = errno;
+ errno = prev_errno;
+ if (new_errno != 0 || end == since || *end != '\0')
fail(ctx-loc,
invalid integer (%s)\n, since);
} else {
diff --git a/src/wayland-client.c b/src/wayland-client.c
index b0f77b9..f4f559e 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -829,8 +829,12 @@ wl_display_connect(const char *name)
connection = getenv(WAYLAND_SOCKET);
if (connection) {
+ int prev_errno = errno, new_errno;
+ errno = 0;
fd = strtol(connection, end, 0);
- if (*end != '\0')
+ new_errno = errno;
+ errno = prev_errno;
+ if (new_errno != 0 || connection == end || *end != '\0')
return NULL;
flags = fcntl(fd, F_GETFD);
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v3] Added more error checks when strtol function is used
Signed-off-by: Imran Zaman imran.za...@gmail.com
---
src/scanner.c| 4 +++-
src/wayland-client.c | 5 -
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/scanner.c b/src/scanner.c
index 5e5152b..fa8e0c0 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -405,11 +405,13 @@ start_element(void *data, const char *element_name, const
char **atts)
message-destructor = 0;
if (since != NULL) {
+ int prev_errno = errno;
errno = 0;
version = strtol(since, end, 0);
- if (errno == EINVAL || end == since || *end != '\0')
+ if (errno != 0 || end == since || *end != '\0')
fail(ctx-loc,
invalid integer (%s)\n, since);
+ errno = prev_errno;
} else {
version = 1;
}
diff --git a/src/wayland-client.c b/src/wayland-client.c
index b0f77b9..01629e0 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -829,9 +829,12 @@ wl_display_connect(const char *name)
connection = getenv(WAYLAND_SOCKET);
if (connection) {
+ int prev_errno = errno;
+ errno = 0;
fd = strtol(connection, end, 0);
- if (*end != '\0')
+ if (errno != 0 || connection == end || *end != '\0')
return NULL;
+ errno = prev_errno;
flags = fcntl(fd, F_GETFD);
if (flags != -1)
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v1] Added more error checks when strtol function is used
pushed v3 to patchwork with the change mentioned and marked the older
versions as superceeded
http://patchwork.freedesktop.org/patch/36297/
BR
imran
On Wed, Nov 5, 2014 at 5:27 PM, Pekka Paalanen ppaala...@gmail.com wrote:
On Tue, 4 Nov 2014 15:55:06 +0200
Imran Zaman imran.za...@gmail.com wrote:
Signed-off-by: Imran Zaman imran.za...@gmail.com
---
src/scanner.c| 2 +-
src/wayland-client.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/scanner.c b/src/scanner.c
index 5e5152b..2ed9775 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -407,7 +407,7 @@ start_element(void *data, const char *element_name,
const char **atts)
if (since != NULL) {
errno = 0;
version = strtol(since, end, 0);
- if (errno == EINVAL || end == since || *end !=
'\0')
+ if (errno != 0 || end == since || *end != '\0')
fail(ctx-loc,
invalid integer (%s)\n, since);
} else {
diff --git a/src/wayland-client.c b/src/wayland-client.c
index b0f77b9..c99dbeb 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -829,8 +829,9 @@ wl_display_connect(const char *name)
connection = getenv(WAYLAND_SOCKET);
if (connection) {
+ errno = 0;
fd = strtol(connection, end, 0);
- if (*end != '\0')
+ if (errno != 0 || connection == end || *end != '\0')
return NULL;
flags = fcntl(fd, F_GETFD);
If we follow the don't clobber errno with zero rule of thumb, here
you'd need to save errno first, do strtol, and if strtol succeeded,
restore errno.
Otherwise looks good.
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v1] wayland-api: added name/seatname properties to the wl_output
Pekka, thanks a lot for the detailed explanation. Lets see which way we go. BR imran On Wed, Nov 5, 2014 at 6:32 PM, Pekka Paalanen ppaala...@gmail.com wrote: On Fri, 17 Oct 2014 12:37:37 +0300 Imran Zaman imran.za...@gmail.com wrote: In a multi-seat configuration, clients may need to filter out the outputs based on the (udev) seat it is hooked to or based on the name of the output. Since version of the output is increased, the change does not affect the current implementation and is optional whoever wants to use the properties of the output (e.g. its very similar that input which has the name property attached to it). Signed-off-by: Imran Zaman imran.za...@gmail.com I explained this to you in IRC, and I will document it here again. The seatname event is the wrong approach to solving the issue, and will not be accepted. The other event, output name, might be useful though, but we need to look at it separately and see what use cases it serves. First, we need to define some concepts. A physical seat is a set of physical input and output devices. Each physical seat would have a different person working on it. Every person has his own monitors, keyboards, mice, etc. Most often, these seats are completely isolated, in that one person cannot touch another person's desktop or apps. People like privacy. Multi-seat means multiple physical seats, which are all served by the same machine. Implementing the isolation is the major issue here, and also what I understand is the primary problem you are trying to solve. Wayland's wl_seat is not a physical seat. It is not a seat at all. wl_seat is just a collection of input devices (no output devices!). Several mice under the same wl_seat act as a single pointer. Several keyboards under the same wl_seat act as a single keyboard. A physical seat may contain multiple outputs (monitors) and multiple wl_seats. All these wl_seats will share the same desktop and user. That desktop expands to all the outputs of the physical seat. Multiple wl_seats on the same server is not multi-seat, it is more like multi-pointer and multi-keyboard. You get one pointer per wl_seat, and one keyboard focus per wl_seat. Each wl_seat may be typing to a different window at the same time, within the same desktop. For multi-seat, isolation is best achieved by running one display server for each physical seat. It is simple and easier to secure. Unfortunately, it currently also means that you need a separate graphics device for each physical seat, because we cannot reliably share the KMS resources of one graphics card. So, you choose to run a single display server covering all physical seats, so that you can split the KMS resources of the one graphics card in the server, rather than in the kernel. Then your major problem is implementing isolation, and that is what Weston is not written to support yet. Your aim must be achieving a similar environment for all applications and desktop components as what a separate display server instance for each physical seat would provide. It makes no sense for the display server to advertise input or output devices that do not belong to that physical seat. Therefore the seatname label is useless. The solution is not to label all input and output devices so that applications would then pick the ones from correct physical seat. The solution is to not even advertise unrelated devices at all to the users' applications. This is why Weston does not even open input and output devices that are not intended for the physical seat Weston will be controlling. That is why adding seatname event to wl_output is completely wrong. Implementing that isolation in Weston will take a *lot* of effort. It's probably not feasible to implement the isolation in a single display server. It would be *so* much easier if each user had his own session compositor instance. So, we need to make that happen. You would run one Weston instance as the system compositor, and then each user (physical seat) would run a session compositor. This leads to the very problem you are trying to solve here, but we need to solve it differently. We need a new shell protocol. Maybe the fullscreen shell could be extended, maybe you need to design a new one based on it. The system compositor might not advertise *any* wl_outputs or wl_seats. Instead, it advertises this new shell interface. Session compositors use the shell interface to discover the available monitors (not wl_outputs) on the physical seat, and dedicate one wl_surface for each monitor. The shell interface would also enumerate the available input devices, and for example pass opened evdev file descriptors to the session compositors. Passing open evdev file descriptors allows the system compositor to stay in charge of input devices, while stepping off from the input event path. Session compositors would never go opening evdev devices themselves
Re: Wayland and Weston in Patchwork
Thats really good initiative Pekka/Daniel. Thanks. BR imran On Tue, Nov 4, 2014 at 1:14 PM, Pekka Paalanen ppaala...@gmail.com wrote: Hi all, some of you already know that we now have http://patchwork.freedesktop.org/project/wayland/list/ to keep track of the patches sent to wayland-devel mailing list. Thanks to Daniel for setting that up. :-) We initialized the patch list with my review backlog. If you are waiting for review on a patch, that is not for libinput, and you do not see it listed in Patchwork at all (try disabling the filters too), rebase and re-send it, please. For contributors: We have a filter, that prevents libinput patches from appearing on patchwork, because libinput developers did not feel the need for it. I seem to recall the filter works by looking at the email subject if it contains libinput in the [subject-prefix]. Otherwise, all wayland, weston, and wayland-web patches should appear on that list when they hit the mailing list. If you find that a patch does not soon appear there while it does appear on the mailing list archives[1], please send an email to me and Daniel (cc'd) because something is wrong. When you send patches to wayland-devel@, you can register your email address in Pathwork, which allows you to manage the state of your own patches. If you re-send or send revised versions of your patches, I would hope you also take the time to mark your old patches in Patchwork as superseded as appropriate. You change the patch status by clicking the patch link in the list, and on the following page you should have box titled Patch Properties near the top. If you don't see that, you don't have permissions to change it (e.g. not logged in, or unregistered email address). If you want to become a Patchwork maintainer, i.e. have the rights to change also other people's patch states, ask me or Daniel. The old announcement of Patchwork for Mesa contains more information: http://lists.freedesktop.org/archives/mesa-dev/2013-November/049293.html Just replace the mesa project name with wayland where needed. It also includes a command line client for Patchwork. For Wayland/Weston maintainers: There is a git hook in place on the upstream repositories of Wayland and Weston, that will automatically mark a patch in Patchwork as Accepted if you push it to master unmodified. If you edit anything except the commit message, the automatic update will likely fail and you need to update the status manually. When you do a push with successful automatic update, you see something like: $ git push origin master Counting objects: 8, done. Delta compression using up to 4 threads. Compressing objects: 100% (8/8), done. Writing objects: 100% (8/8), 1.46 KiB | 0 bytes/s, done. Total 8 (delta 7), reused 0 (delta 0) remote: Updating patchwork state for http://patchwork.freedesktop.org/project/wayland/list/ remote: I: patch #34560 updated using rev 391820b0d6d9fcd99e12cd32623a476da64c89ce remote: I: 1 patch(es) updated to state Accepted. To ssh://git.freedesktop.org/git/wayland/wayland 4a661c5..391820b master - master When it fails, it will say so. Shortcomings: There are several features we would like to see in Patchwork but AFAIK are not there (yet?). Patchwork does not recognize re-submissions so that it could automatically set a patch as Superseded. It does not maintain patch sets. You can create Bundles, but so far those are just named collections of individual patches, and you need to create them manually. If you want to see only Wayland patches, or only Weston patches, etc., you need to set up a filter based on the patch subject, that relies on everyone using the subject-prefix properly. Subject-prefix is not always set like this, so the filtered lists will miss something. You can configure the filters for the list in the blue header box, clicking Filters. We are just starting to use Patchwork here, so we are still learning the process. Even with its shortcomings, I believe Patchwork is an improvement to the previous situation. At least the review backlog is now public, and if we keep the list in Patchwork maintained, patches will not get lost. Yeah, we also broke the commit announcements through the irc bot while getting this up, will see about fixing that. Thanks, pq [1] http://lists.freedesktop.org/archives/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 v3] wayland-util: added wl_strtol/wl_strtoul utility functions
I will push new patch with minor fix to the strtol function in wayland and move this old patch (after segfault fix) to weston so that it does not end up in libwayland APIs. Consequently I changed its property in patchwork BR imran On Wed, Oct 29, 2014 at 10:27 AM, Imran Zaman imran.za...@gmail.com wrote: On Wed, Oct 29, 2014 at 10:09 AM, Giulio Camuffo giuliocamu...@gmail.com wrote: 2014-10-29 8:45 GMT+02:00 Imran Zaman imran.za...@gmail.com: Daniel! As per your logic, I see wl_list APIs exposed etc, which shouldn't be part of libwayland as well. similarly, wl_fixed_to_double and wl_array shouldn't be part of the window system. Isnt it? I can make inline functions if that helps. wl_list is used in the server side API, so it's a bit different. However, I'd agree that it'd be better if it wasn't exposed but we can't remove it now. wl_fixed is a wayland specific type so all the wl_fixed_* functions need to be part of the API. On the other hand wl_strtol would just be a function, there are is no other API that depends on it. Btw here is an API patch, which has not be reviewed till now. http://lists.freedesktop.org/archives/wayland-devel/2014-October/017833.html Yes, like there are many other patches waiting for reviews. You need to have patience, it's not like we are ignoring it. But, if I may add, the way you are reacting to a comment to this patch doesn't encourage to review your other ones. Neither the random/comments to the patch are encouraging :-) e.g. AOL. We're a window system, not a replacement libc. Its your choice to review it or not; I did not put the API patch link here just because it has not been reviewed. I have lots of patience but Tizen may need something urgent or else we may need to fork wayland/weston in Tizen. I put it in the thread because Daniel said that we did not have any further progress/discussion on that end. Anyways I take the patch off, as it does not sound like it should be here to save everyone's time. If the time allows, I will remove it from public APIs in addition to one critical bug fix and resubmit the patch. -- Giulio BR imran On Tue, Oct 28, 2014 at 6:36 PM, Daniel Stone dan...@fooishbar.org wrote: Hi, On 28 October 2014 15:40, Imran Zaman imran.za...@gmail.com wrote: You guys should check the reason why the patch is there rather than throwing out random thoughts or blunt comments. I did this patch mainly because weston/wayland has been using strtol/strtoul functions in number of places with buggy error checks, and duplicate code everywhere. Weston and wayland go together; so in bigger picture, its a very useful patch IMO.. I hardly find any patches with proper tests, but I did it so to make it more effective. But I guess in wayland/weston community, only maintainers are allowed to push patches others are strongly discouraged to do so. I guess its better to encourage people/community for giving helping hand. Anyways we will now only push patches (including multi-seat support) in Tizen weston/wayland rather than wasting time in upstreamn weston/wayland as it seems to be long bureaucratic process to overcome with virtually no success. That's not what we've said. No-one said 'don't take the patch'; all we said is 'please don't expose it as part of libwayland-*'s _public_ API'. I like the idea of the patch, I like how it's caught a number of buggy spots where we've open-coded the same thing, and I like that it's well-tested. For internal usage, it's great! I just don't want to expose string manipulation functions in the public API of a window system. You're right that the test infrastructure is in a pretty bad state. Anything which helps that is more than welcome, and you may have seen a bunch of patches from Derek Foreman (not a maintainer) on this list, which are progressing well and go a long way towards improving our test suite into something that will be really useful day to day. Any further contributions along those lines - from anyone - are totally welcome. As for your multiseat patches, the last discussions I remember involved some pretty fundamental disagreements about the whole architecture, particularly input support. I haven't seen any more patches or discussion since the last IRC chat, though. Cheers, Daniel On Tue, Oct 28, 2014 at 6:36 PM, Daniel Stone dan...@fooishbar.org wrote: Hi, On 28 October 2014 15:40, Imran Zaman imran.za...@gmail.com wrote: You guys should check the reason why the patch is there rather than throwing out random thoughts or blunt comments. I did this patch mainly because weston/wayland has been using strtol/strtoul functions in number of places with buggy error checks, and duplicate code everywhere. Weston and wayland go together; so in bigger picture, its a very useful patch IMO.. I hardly find any patches with proper tests
[PATCH v1] Added more error checks when strtol function is used
Signed-off-by: Imran Zaman imran.za...@gmail.com
---
src/scanner.c| 2 +-
src/wayland-client.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/scanner.c b/src/scanner.c
index 5e5152b..2ed9775 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -407,7 +407,7 @@ start_element(void *data, const char *element_name, const
char **atts)
if (since != NULL) {
errno = 0;
version = strtol(since, end, 0);
- if (errno == EINVAL || end == since || *end != '\0')
+ if (errno != 0 || end == since || *end != '\0')
fail(ctx-loc,
invalid integer (%s)\n, since);
} else {
diff --git a/src/wayland-client.c b/src/wayland-client.c
index b0f77b9..c99dbeb 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -829,8 +829,9 @@ wl_display_connect(const char *name)
connection = getenv(WAYLAND_SOCKET);
if (connection) {
+ errno = 0;
fd = strtol(connection, end, 0);
- if (*end != '\0')
+ if (errno != 0 || connection == end || *end != '\0')
return NULL;
flags = fcntl(fd, F_GETFD);
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v3] wayland-util: added wl_strtol/wl_strtoul utility functions
Daniel! As per your logic, I see wl_list APIs exposed etc, which shouldn't be part of libwayland as well. similarly, wl_fixed_to_double and wl_array shouldn't be part of the window system. Isnt it? I can make inline functions if that helps. Btw here is an API patch, which has not be reviewed till now. http://lists.freedesktop.org/archives/wayland-devel/2014-October/017833.html BR imran On Tue, Oct 28, 2014 at 6:36 PM, Daniel Stone dan...@fooishbar.org wrote: Hi, On 28 October 2014 15:40, Imran Zaman imran.za...@gmail.com wrote: You guys should check the reason why the patch is there rather than throwing out random thoughts or blunt comments. I did this patch mainly because weston/wayland has been using strtol/strtoul functions in number of places with buggy error checks, and duplicate code everywhere. Weston and wayland go together; so in bigger picture, its a very useful patch IMO.. I hardly find any patches with proper tests, but I did it so to make it more effective. But I guess in wayland/weston community, only maintainers are allowed to push patches others are strongly discouraged to do so. I guess its better to encourage people/community for giving helping hand. Anyways we will now only push patches (including multi-seat support) in Tizen weston/wayland rather than wasting time in upstreamn weston/wayland as it seems to be long bureaucratic process to overcome with virtually no success. That's not what we've said. No-one said 'don't take the patch'; all we said is 'please don't expose it as part of libwayland-*'s _public_ API'. I like the idea of the patch, I like how it's caught a number of buggy spots where we've open-coded the same thing, and I like that it's well-tested. For internal usage, it's great! I just don't want to expose string manipulation functions in the public API of a window system. You're right that the test infrastructure is in a pretty bad state. Anything which helps that is more than welcome, and you may have seen a bunch of patches from Derek Foreman (not a maintainer) on this list, which are progressing well and go a long way towards improving our test suite into something that will be really useful day to day. Any further contributions along those lines - from anyone - are totally welcome. As for your multiseat patches, the last discussions I remember involved some pretty fundamental disagreements about the whole architecture, particularly input support. I haven't seen any more patches or discussion since the last IRC chat, though. Cheers, Daniel On Tue, Oct 28, 2014 at 6:36 PM, Daniel Stone dan...@fooishbar.org wrote: Hi, On 28 October 2014 15:40, Imran Zaman imran.za...@gmail.com wrote: You guys should check the reason why the patch is there rather than throwing out random thoughts or blunt comments. I did this patch mainly because weston/wayland has been using strtol/strtoul functions in number of places with buggy error checks, and duplicate code everywhere. Weston and wayland go together; so in bigger picture, its a very useful patch IMO.. I hardly find any patches with proper tests, but I did it so to make it more effective. But I guess in wayland/weston community, only maintainers are allowed to push patches others are strongly discouraged to do so. I guess its better to encourage people/community for giving helping hand. Anyways we will now only push patches (including multi-seat support) in Tizen weston/wayland rather than wasting time in upstreamn weston/wayland as it seems to be long bureaucratic process to overcome with virtually no success. That's not what we've said. No-one said 'don't take the patch'; all we said is 'please don't expose it as part of libwayland-*'s _public_ API'. I like the idea of the patch, I like how it's caught a number of buggy spots where we've open-coded the same thing, and I like that it's well-tested. For internal usage, it's great! I just don't want to expose string manipulation functions in the public API of a window system. You're right that the test infrastructure is in a pretty bad state. Anything which helps that is more than welcome, and you may have seen a bunch of patches from Derek Foreman (not a maintainer) on this list, which are progressing well and go a long way towards improving our test suite into something that will be really useful day to day. Any further contributions along those lines - from anyone - are totally welcome. As for your multiseat patches, the last discussions I remember involved some pretty fundamental disagreements about the whole architecture, particularly input support. I haven't seen any more patches or discussion since the last IRC chat, though. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH v3] wayland-util: added wl_strtol/wl_strtoul utility functions
On Wed, Oct 29, 2014 at 10:09 AM, Giulio Camuffo giuliocamu...@gmail.com wrote: 2014-10-29 8:45 GMT+02:00 Imran Zaman imran.za...@gmail.com: Daniel! As per your logic, I see wl_list APIs exposed etc, which shouldn't be part of libwayland as well. similarly, wl_fixed_to_double and wl_array shouldn't be part of the window system. Isnt it? I can make inline functions if that helps. wl_list is used in the server side API, so it's a bit different. However, I'd agree that it'd be better if it wasn't exposed but we can't remove it now. wl_fixed is a wayland specific type so all the wl_fixed_* functions need to be part of the API. On the other hand wl_strtol would just be a function, there are is no other API that depends on it. Btw here is an API patch, which has not be reviewed till now. http://lists.freedesktop.org/archives/wayland-devel/2014-October/017833.html Yes, like there are many other patches waiting for reviews. You need to have patience, it's not like we are ignoring it. But, if I may add, the way you are reacting to a comment to this patch doesn't encourage to review your other ones. Neither the random/comments to the patch are encouraging :-) e.g. AOL. We're a window system, not a replacement libc. Its your choice to review it or not; I did not put the API patch link here just because it has not been reviewed. I have lots of patience but Tizen may need something urgent or else we may need to fork wayland/weston in Tizen. I put it in the thread because Daniel said that we did not have any further progress/discussion on that end. Anyways I take the patch off, as it does not sound like it should be here to save everyone's time. If the time allows, I will remove it from public APIs in addition to one critical bug fix and resubmit the patch. -- Giulio BR imran On Tue, Oct 28, 2014 at 6:36 PM, Daniel Stone dan...@fooishbar.org wrote: Hi, On 28 October 2014 15:40, Imran Zaman imran.za...@gmail.com wrote: You guys should check the reason why the patch is there rather than throwing out random thoughts or blunt comments. I did this patch mainly because weston/wayland has been using strtol/strtoul functions in number of places with buggy error checks, and duplicate code everywhere. Weston and wayland go together; so in bigger picture, its a very useful patch IMO.. I hardly find any patches with proper tests, but I did it so to make it more effective. But I guess in wayland/weston community, only maintainers are allowed to push patches others are strongly discouraged to do so. I guess its better to encourage people/community for giving helping hand. Anyways we will now only push patches (including multi-seat support) in Tizen weston/wayland rather than wasting time in upstreamn weston/wayland as it seems to be long bureaucratic process to overcome with virtually no success. That's not what we've said. No-one said 'don't take the patch'; all we said is 'please don't expose it as part of libwayland-*'s _public_ API'. I like the idea of the patch, I like how it's caught a number of buggy spots where we've open-coded the same thing, and I like that it's well-tested. For internal usage, it's great! I just don't want to expose string manipulation functions in the public API of a window system. You're right that the test infrastructure is in a pretty bad state. Anything which helps that is more than welcome, and you may have seen a bunch of patches from Derek Foreman (not a maintainer) on this list, which are progressing well and go a long way towards improving our test suite into something that will be really useful day to day. Any further contributions along those lines - from anyone - are totally welcome. As for your multiseat patches, the last discussions I remember involved some pretty fundamental disagreements about the whole architecture, particularly input support. I haven't seen any more patches or discussion since the last IRC chat, though. Cheers, Daniel On Tue, Oct 28, 2014 at 6:36 PM, Daniel Stone dan...@fooishbar.org wrote: Hi, On 28 October 2014 15:40, Imran Zaman imran.za...@gmail.com wrote: You guys should check the reason why the patch is there rather than throwing out random thoughts or blunt comments. I did this patch mainly because weston/wayland has been using strtol/strtoul functions in number of places with buggy error checks, and duplicate code everywhere. Weston and wayland go together; so in bigger picture, its a very useful patch IMO.. I hardly find any patches with proper tests, but I did it so to make it more effective. But I guess in wayland/weston community, only maintainers are allowed to push patches others are strongly discouraged to do so. I guess its better to encourage people/community for giving helping hand. Anyways we will now only push patches (including multi-seat support
Re: [PATCH v3] wayland-util: added wl_strtol/wl_strtoul utility functions
You guys should check the reason why the patch is there rather than throwing out random thoughts or blunt comments. I did this patch mainly because weston/wayland has been using strtol/strtoul functions in number of places with buggy error checks, and duplicate code everywhere. Weston and wayland go together; so in bigger picture, its a very useful patch IMO.. I hardly find any patches with proper tests, but I did it so to make it more effective. But I guess in wayland/weston community, only maintainers are allowed to push patches others are strongly discouraged to do so. I guess its better to encourage people/community for giving helping hand. Anyways we will now only push patches (including multi-seat support) in Tizen weston/wayland rather than wasting time in upstreamn weston/wayland as it seems to be long bureaucratic process to overcome with virtually no success. BR imran On Tue, Oct 28, 2014 at 4:50 PM, Daniel Stone dan...@fooishbar.org wrote: Hi, On 28 October 2014 11:21, Giulio Camuffo giuliocamu...@gmail.com wrote: 2014-10-27 18:51 GMT+02:00 Jasper St. Pierre jstpie...@mecheye.net: Can I also suggest that we don't make this public API? These are internal helpers for libwayland, not designed for any consumers. We've been burned by making too much internal helper API public before. +1 I don't think this belongs in the wayland API at all. That means duplicating them in weston, but they will hardly need modifications anyway. AOL. We're a window system, not a replacement libc. Cheers, Dan ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland] increase server listen queue to 8
Hi
This will allow more than 1 simultaneous client connections to the server
without the possibility of connection refused error. possible use case
is multiple session
compositors can connect to the system compositor simultaneously.
diff --git a/src/wayland-server.c b/src/wayland-server.c
index 674aeca..e3b7d9f 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -1126,7 +1126,7 @@ _wl_display_add_socket(struct wl_display
*display, struct wl_socket *s)
return -1;
}
- if (listen(s-fd, 1) 0) {
+ if (listen(s-fd, 8) 0) {
wl_log(listen() failed with error: %m\n);
return -1;
}
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH v3] wayland-util: added wl_strtol/wl_strtoul utility functions
strtol/strtoul utility functions are used extensively in
weston/wayland, and are not bug-free in their current form.
To avoid definition in weston and wayland, its wrapped
in functions with appropriate input and output checks.
Test cases are also updated.
Signed-off-by: Imran Zaman imran.za...@gmail.com
---
src/scanner.c| 5 +-
src/wayland-client.c | 5 +-
src/wayland-util.c | 55
src/wayland-util.h | 4 ++
tests/fixed-test.c | 142 +++
5 files changed, 204 insertions(+), 7 deletions(-)
diff --git a/src/scanner.c b/src/scanner.c
index 809130b..165b12b 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -315,7 +315,6 @@ start_element(void *data, const char *element_name, const
char **atts)
struct description *description;
const char *name, *type, *interface_name, *value, *summary, *since;
const char *allow_null;
- char *end;
int i, version;
ctx-loc.line_number = XML_GetCurrentLineNumber(ctx-parser);
@@ -404,9 +403,7 @@ start_element(void *data, const char *element_name, const
char **atts)
message-destructor = 0;
if (since != NULL) {
- errno = 0;
- version = strtol(since, end, 0);
- if (errno == EINVAL || end == since || *end != '\0')
+ if (!wl_strtol(since, NULL, 0, (long *)version))
fail(ctx-loc,
invalid integer (%s)\n, since);
} else {
diff --git a/src/wayland-client.c b/src/wayland-client.c
index b0f77b9..fd98705 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -824,13 +824,12 @@ wl_display_connect_to_fd(int fd)
WL_EXPORT struct wl_display *
wl_display_connect(const char *name)
{
- char *connection, *end;
+ char *connection;
int flags, fd;
connection = getenv(WAYLAND_SOCKET);
if (connection) {
- fd = strtol(connection, end, 0);
- if (*end != '\0')
+ if (!wl_strtol(connection, NULL, 0, (long *)fd))
return NULL;
flags = fcntl(fd, F_GETFD);
diff --git a/src/wayland-util.c b/src/wayland-util.c
index b099882..a930364 100644
--- a/src/wayland-util.c
+++ b/src/wayland-util.c
@@ -26,6 +26,8 @@
#include stdio.h
#include string.h
#include stdarg.h
+#include errno.h
+#include ctype.h
#include wayland-util.h
#include wayland-private.h
@@ -361,6 +363,59 @@ wl_map_for_each(struct wl_map *map, wl_iterator_func_t
func, void *data)
for_each_helper(map-server_entries, func, data);
}
+WL_EXPORT bool
+wl_strtol(const char *str, char **endptr, int base, long *val)
+{
+ char *end = NULL;
+ long v;
+
+ if (!str || !val)
+ return false;
+ if (!endptr)
+ endptr = end;
+
+ errno = 0;
+ v = strtol(str, endptr, base);
+ if (errno != 0 || *endptr == str || **endptr != '\0')
+ return false;
+
+ *val = v;
+ return true;
+}
+
+WL_EXPORT bool
+wl_strtoul(const char *str, char **endptr, int base, unsigned long *val)
+{
+ char *end = NULL;
+ unsigned long v;
+ int i = 0;
+
+ if (!str || !val)
+ return false;
+
+ /* check for negative numbers */
+ while (str[i]) {
+ if (!isspace(str[i])) {
+ if (str[i] == '-')
+ return false;
+ else
+ break;
+ }
+ i++;
+ }
+
+ if (!endptr)
+ endptr = end;
+
+ errno = 0;
+ v = strtoul(str, endptr, base);
+ if (errno != 0 || *endptr == str || **endptr != '\0')
+ return false;
+
+ *val = v;
+ return true;
+}
+
static void
wl_log_stderr_handler(const char *fmt, va_list arg)
{
diff --git a/src/wayland-util.h b/src/wayland-util.h
index fd32826..c66cc41 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -36,6 +36,7 @@ extern C {
#include stddef.h
#include inttypes.h
#include stdarg.h
+#include stdbool.h
/* GCC visibility */
#if defined(__GNUC__) __GNUC__ = 4
@@ -243,6 +244,9 @@ static inline wl_fixed_t wl_fixed_from_int(int i)
return i * 256;
}
+bool wl_strtol(const char *str, char **endptr, int base, long *val);
+bool wl_strtoul(const char *str, char **endptr, int base, unsigned long *val);
+
/**
* \brief A union representing all of the basic data types that can be passed
* along the wayland wire format.
diff --git a/tests/fixed-test.c b/tests/fixed-test.c
index 739a3b1..ad40467 100644
--- a/tests/fixed-test.c
+++ b/tests/fixed-test.c
@@ -88,3 +88,145 @@ TEST(fixed_int_conversions)
i = wl_fixed_to_int(f);
assert(i == -0x50);
}
+
+TEST(strtol_conversions)
+{
+ bool ret;
+ long val = -1
Re: [PATCH v2] wayland-util: added wl_strtol/wl_strtoul utility functions
I pushed an updated patch v3. Added test cases for overflow and also
check for negative numbers for wl_strtoul..
please review
BR
imran
On Wed, Oct 22, 2014 at 12:13 PM, Marek Chalupa mchqwe...@gmail.com wrote:
Hi,
Personally, I'd rather see these functions private (somebody has already
mentioned that),
but I understand there are reasons for them to be public and maybe in the
end it will have more pros..
Anyway, I have few nitpicks and a questions - see below.
On 16 October 2014 18:11, Imran Zaman imran.za...@gmail.com wrote:
strtol/strtoul utility functions are used extensively in
weston/wayland, and are not bug-free in their current form.
To avoid definition in weston and wayland, its wrapped
in functions with appropriate input and output checks.
Test cases are also updated.
Signed-off-by: Imran Zaman imran.za...@gmail.com
---
src/scanner.c| 5 +--
src/wayland-client.c | 5 +--
src/wayland-util.c | 38
src/wayland-util.h | 4 ++
tests/fixed-test.c | 122
+++
5 files changed, 167 insertions(+), 7 deletions(-)
diff --git a/src/scanner.c b/src/scanner.c
index 809130b..165b12b 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -315,7 +315,6 @@ start_element(void *data, const char *element_name,
const char **atts)
struct description *description;
const char *name, *type, *interface_name, *value, *summary,
*since;
const char *allow_null;
- char *end;
int i, version;
ctx-loc.line_number = XML_GetCurrentLineNumber(ctx-parser);
@@ -404,9 +403,7 @@ start_element(void *data, const char *element_name,
const char **atts)
message-destructor = 0;
if (since != NULL) {
- errno = 0;
- version = strtol(since, end, 0);
- if (errno == EINVAL || end == since || *end !=
'\0')
+ if (!wl_strtol(since, NULL, 0, (long *)version))
fail(ctx-loc,
invalid integer (%s)\n, since);
} else {
diff --git a/src/wayland-client.c b/src/wayland-client.c
index b0f77b9..fd98705 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -824,13 +824,12 @@ wl_display_connect_to_fd(int fd)
WL_EXPORT struct wl_display *
wl_display_connect(const char *name)
{
- char *connection, *end;
+ char *connection;
int flags, fd;
connection = getenv(WAYLAND_SOCKET);
if (connection) {
- fd = strtol(connection, end, 0);
- if (*end != '\0')
+ if (!wl_strtol(connection, NULL, 0, (long *)fd))
return NULL;
flags = fcntl(fd, F_GETFD);
diff --git a/src/wayland-util.c b/src/wayland-util.c
index b099882..dfd2eb1 100644
--- a/src/wayland-util.c
+++ b/src/wayland-util.c
@@ -26,6 +26,8 @@
#include stdio.h
#include string.h
#include stdarg.h
+#include errno.h
+#include limits.h
#include wayland-util.h
#include wayland-private.h
@@ -361,6 +363,42 @@ wl_map_for_each(struct wl_map *map,
wl_iterator_func_t func, void *data)
for_each_helper(map-server_entries, func, data);
}
+WL_EXPORT bool
+wl_strtol(const char *str, char **endptr, int base, long *val)
+{
+ char *end = NULL;
+ long v;
+
+ if (!str || !val) return false;
+ if (!endptr) endptr = end;
The 'return false' and 'endptr =end' should be on new line
http://cgit.freedesktop.org/wayland/wayland/tree/doc/Contributing#n33
+
+ errno = 0;
+ v = strtol(str, endptr, base);
+ if (errno != 0 || *endptr == str || **endptr != '\0')
+ return false;
+
+ *val = v;
+ return true;
+}
+
+WL_EXPORT bool
+wl_strtoul(const char *str, char **endptr, int base, unsigned long *val)
+{
+ char *end = NULL;
+ unsigned long v;
+
+ if (!str || !val) return false;
+ if (!endptr) endptr = end;
The same
+
+ errno = 0;
+ v = strtoul(str, endptr, base);
+ if (errno != 0 || *endptr == str || **endptr != '\0')
+ return false;
+
+ *val = v;
+ return true;
+}
+
static void
wl_log_stderr_handler(const char *fmt, va_list arg)
{
diff --git a/src/wayland-util.h b/src/wayland-util.h
index fd32826..c66cc41 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -36,6 +36,7 @@ extern C {
#include stddef.h
#include inttypes.h
#include stdarg.h
+#include stdbool.h
/* GCC visibility */
#if defined(__GNUC__) __GNUC__ = 4
@@ -243,6 +244,9 @@ static inline wl_fixed_t wl_fixed_from_int(int i)
return i * 256;
}
+bool wl_strtol(const char *str, char **endptr, int base, long *val);
+bool wl_strtoul(const char *str, char **endptr, int base, unsigned long
*val);
+
/**
* \brief
[PATCH v1] wayland-api: added name/seatname properties to the wl_output
In a multi-seat configuration, clients may need to filter out the outputs based on the (udev) seat it is hooked to or based on the name of the output. Since version of the output is increased, the change does not affect the current implementation and is optional whoever wants to use the properties of the output (e.g. its very similar that input which has the name property attached to it). Signed-off-by: Imran Zaman imran.za...@gmail.com --- protocol/wayland.xml | 22 +- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/protocol/wayland.xml b/protocol/wayland.xml index 762482e..7580cdf 100644 --- a/protocol/wayland.xml +++ b/protocol/wayland.xml @@ -1741,7 +1741,7 @@ /request /interface - interface name=wl_output version=2 + interface name=wl_output version=3 description summary=compositor output region An output describes part of the compositor geometry. The compositor works in the 'compositor coordinate system' and an @@ -1879,6 +1879,26 @@ /description arg name=factor type=int summary=scaling factor of output/ /event + +!-- Version 3 of additions -- + +event name=name since=3 + description summary=name of the output + In a multiseat configuration this can be used by the client to help + identify the name of the output and consequently the name can be used to + select the output(s) based on the configuration. + /description + arg name=name type=string/ +/event + +event name=seatname since=3 + description summary=name of the seat the output is constrained to + In a multiseat configuration this can be used by the client to help + identify the seat which the given output is constrained to and consequently + select the output(s) based on the client own seat. + /description + arg name=name type=string/ +/event /interface interface name=wl_region version=1 -- 1.9.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] wl_strtol and wl_strtoul utility functions are added (inlined patch)
Here are my comments:
Giulio Camuffo: if we copy-paste the same code to weston as well,
means we have to write tests etc for two functions in weston as well;
and it will come with maintenance overhead without any benefit of
hiding the APIs in wayland (I can take the responsibility of
maintaining these two APIs :)
Thiago: I can look into strtol_l functions..
Bryce: I will incorporate your changes and also update the test cases.
There are cases when it is used other than thr base 10 as well; let me
check if there is something that can be done for locale-specific
numbers..
I will push the updated patch after the changes..
On Thu, Oct 16, 2014 at 4:05 AM, Bryce Harrington br...@osg.samsung.com wrote:
On Wed, Oct 15, 2014 at 04:18:59PM +0300, Imran Zaman wrote:
Hi
The patch is used to replace strtol and strtoul with wl_strtol and
wl_strtoul with inputs and result checks.
The utility functions are used extensively in wayland and weston so added
These utility...
appropriate
input and output checks; test cases are also updated; will push the patch
for weston as well.
Looks like this'll be a nice cleanup.
diff --git a/src/scanner.c b/src/scanner.c
index 809130b..3e30fe7 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -315,7 +315,6 @@ start_element(void *data, const char *element_name,
const char **atts)
struct description *description;
const char *name, *type, *interface_name, *value, *summary, *since;
const char *allow_null;
- char *end;
int i, version;
ctx-loc.line_number = XML_GetCurrentLineNumber(ctx-parser);
@@ -404,9 +403,7 @@ start_element(void *data, const char *element_name,
const char **atts)
message-destructor = 0;
if (since != NULL) {
- errno = 0;
- version = strtol(since, end, 0);
- if (errno == EINVAL || end == since || *end != '\0')
+ if (!wl_strtol(since, NULL, 0, version))
fail(ctx-loc,
invalid integer (%s)\n, since);
} else {
diff --git a/src/wayland-client.c b/src/wayland-client.c
index b0f77b9..1229b5f 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -824,13 +824,12 @@ wl_display_connect_to_fd(int fd)
WL_EXPORT struct wl_display *
wl_display_connect(const char *name)
{
- char *connection, *end;
+ char *connection;
int flags, fd;
connection = getenv(WAYLAND_SOCKET);
if (connection) {
- fd = strtol(connection, end, 0);
- if (*end != '\0')
+ if (!wl_strtol(connection, NULL, 0, fd))
return NULL;
flags = fcntl(fd, F_GETFD);
diff --git a/src/wayland-util.c b/src/wayland-util.c
index b099882..f8267f3 100644
--- a/src/wayland-util.c
+++ b/src/wayland-util.c
@@ -26,6 +26,9 @@
#include stdio.h
#include string.h
#include stdarg.h
+#include errno.h
+#include limits.h
+#include stdlib.h
Looks like stdlib.h is already included in wayland-util.c
#include wayland-util.h
#include wayland-private.h
@@ -361,6 +364,42 @@ wl_map_for_each(struct wl_map *map, wl_iterator_func_t
func, void *data)
for_each_helper(map-server_entries, func, data);
}
+WL_EXPORT int
+wl_strtol(const char *str, char **endptr, int base, int32_t *val)
For consistency with strtol shouldn't the final arg be type long?
+{
+ char *end = NULL;
+ long v;
+
+ if (!str || !val) return 0;
+ if (!endptr) endptr = end;
+
+ errno = 0;
+ v = strtol(str, endptr, base);
+ if (errno != 0 || *endptr == str || **endptr != '\0')
+ return 0;
+
+ *val = v;
Here's a type conversion between long and int32_t.
(Which are both the same thing, but not guaranteed...)
+ return 1;
+}
Following the recent return type discussions, maybe consider returning
bool (true|false) rather than int. Looks like this function needn't
return anything other than 0/1 so a bool would make it unambiguous.
+
+WL_EXPORT int
+wl_strtoul(const char *str, char **endptr, int base, uint32_t *val)
+{
+ char *end = NULL;
+ unsigned long v;
+
+ if (!str || !val) return 0;
+ if (!endptr) endptr = end;
+
+ errno = 0;
+ v = strtoul(str, endptr, base);
+ if (errno != 0 || *endptr == str || **endptr != '\0')
+ return 0;
+
+ *val = v;
+ return 1;
+}
+
Ditto previous comments.
static void
wl_log_stderr_handler(const char *fmt, va_list arg)
{
diff --git a/src/wayland-util.h b/src/wayland-util.h
index fd32826..b77d4e3 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -243,6 +243,9 @@ static inline wl_fixed_t wl_fixed_from_int(int i)
return i * 256;
}
+int wl_strtol(const char *str, char **endptr, int base, int32_t *val);
+int wl_strtoul(const char *str, char **endptr, int base, uint32_t *val);
+
/**
* \brief A union representing all of the basic data types that can be
passed
* along the wayland wire format.
diff --git a/tests/fixed-test.c b/tests/fixed-test.c
index 739a3b1..349cc48 100644
--- a/tests/fixed-test.c
+++ b/tests/fixed-test.c
@@ -88,3 +88,61 @@ TEST(fixed_int_conversions)
i = wl_fixed_to_int(f);
assert(i == -0x50);
}
Thanks for including
[PATCH v2] wayland-util: added wl_strtol/wl_strtoul utility functions
strtol/strtoul utility functions are used extensively in
weston/wayland, and are not bug-free in their current form.
To avoid definition in weston and wayland, its wrapped
in functions with appropriate input and output checks.
Test cases are also updated.
Signed-off-by: Imran Zaman imran.za...@gmail.com
---
src/scanner.c| 5 +--
src/wayland-client.c | 5 +--
src/wayland-util.c | 38
src/wayland-util.h | 4 ++
tests/fixed-test.c | 122 +++
5 files changed, 167 insertions(+), 7 deletions(-)
diff --git a/src/scanner.c b/src/scanner.c
index 809130b..165b12b 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -315,7 +315,6 @@ start_element(void *data, const char *element_name, const
char **atts)
struct description *description;
const char *name, *type, *interface_name, *value, *summary, *since;
const char *allow_null;
- char *end;
int i, version;
ctx-loc.line_number = XML_GetCurrentLineNumber(ctx-parser);
@@ -404,9 +403,7 @@ start_element(void *data, const char *element_name, const
char **atts)
message-destructor = 0;
if (since != NULL) {
- errno = 0;
- version = strtol(since, end, 0);
- if (errno == EINVAL || end == since || *end != '\0')
+ if (!wl_strtol(since, NULL, 0, (long *)version))
fail(ctx-loc,
invalid integer (%s)\n, since);
} else {
diff --git a/src/wayland-client.c b/src/wayland-client.c
index b0f77b9..fd98705 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -824,13 +824,12 @@ wl_display_connect_to_fd(int fd)
WL_EXPORT struct wl_display *
wl_display_connect(const char *name)
{
- char *connection, *end;
+ char *connection;
int flags, fd;
connection = getenv(WAYLAND_SOCKET);
if (connection) {
- fd = strtol(connection, end, 0);
- if (*end != '\0')
+ if (!wl_strtol(connection, NULL, 0, (long *)fd))
return NULL;
flags = fcntl(fd, F_GETFD);
diff --git a/src/wayland-util.c b/src/wayland-util.c
index b099882..dfd2eb1 100644
--- a/src/wayland-util.c
+++ b/src/wayland-util.c
@@ -26,6 +26,8 @@
#include stdio.h
#include string.h
#include stdarg.h
+#include errno.h
+#include limits.h
#include wayland-util.h
#include wayland-private.h
@@ -361,6 +363,42 @@ wl_map_for_each(struct wl_map *map, wl_iterator_func_t
func, void *data)
for_each_helper(map-server_entries, func, data);
}
+WL_EXPORT bool
+wl_strtol(const char *str, char **endptr, int base, long *val)
+{
+ char *end = NULL;
+ long v;
+
+ if (!str || !val) return false;
+ if (!endptr) endptr = end;
+
+ errno = 0;
+ v = strtol(str, endptr, base);
+ if (errno != 0 || *endptr == str || **endptr != '\0')
+ return false;
+
+ *val = v;
+ return true;
+}
+
+WL_EXPORT bool
+wl_strtoul(const char *str, char **endptr, int base, unsigned long *val)
+{
+ char *end = NULL;
+ unsigned long v;
+
+ if (!str || !val) return false;
+ if (!endptr) endptr = end;
+
+ errno = 0;
+ v = strtoul(str, endptr, base);
+ if (errno != 0 || *endptr == str || **endptr != '\0')
+ return false;
+
+ *val = v;
+ return true;
+}
+
static void
wl_log_stderr_handler(const char *fmt, va_list arg)
{
diff --git a/src/wayland-util.h b/src/wayland-util.h
index fd32826..c66cc41 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -36,6 +36,7 @@ extern C {
#include stddef.h
#include inttypes.h
#include stdarg.h
+#include stdbool.h
/* GCC visibility */
#if defined(__GNUC__) __GNUC__ = 4
@@ -243,6 +244,9 @@ static inline wl_fixed_t wl_fixed_from_int(int i)
return i * 256;
}
+bool wl_strtol(const char *str, char **endptr, int base, long *val);
+bool wl_strtoul(const char *str, char **endptr, int base, unsigned long *val);
+
/**
* \brief A union representing all of the basic data types that can be passed
* along the wayland wire format.
diff --git a/tests/fixed-test.c b/tests/fixed-test.c
index 739a3b1..aa0340c 100644
--- a/tests/fixed-test.c
+++ b/tests/fixed-test.c
@@ -88,3 +88,125 @@ TEST(fixed_int_conversions)
i = wl_fixed_to_int(f);
assert(i == -0x50);
}
+
+TEST(strtol_conversions)
+{
+ bool ret;
+ long val = -1;
+ char *end = NULL;
+
+ ret = wl_strtol(NULL, NULL, 10, val);
+ assert(ret == false);
+ assert(val == -1);
+
+ ret = wl_strtol(NULL, NULL, 10, NULL);
+ assert(ret == false);
+
+ char *str = 12;
+ ret = wl_strtol(str, NULL, 10, val);
+ assert(ret == true);
+ assert(val == 12);
+
+ ret = wl_strtol(str, end, 10, val);
+ assert(end
[PATCH 1/2] Support specifying custom directories for the client and server sockets through environment variables.
This is in order to support nested compositor architectures
where system compositor using drm-backend is shared among
multiple child compositors using wayland-backend.
Signed-off-by: Imran Zaman imran.za...@gmail.com
---
src/wayland-client.c | 5 -
src/wayland-server.c | 5 -
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/wayland-client.c b/src/wayland-client.c
index b0f77b9..07db370 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -700,7 +700,9 @@ connect_to_socket(const char *name)
const char *runtime_dir;
int name_size, fd;
- runtime_dir = getenv(XDG_RUNTIME_DIR);
+ runtime_dir = getenv(WAYLAND_CLIENT_DIR);
+ if (runtime_dir == NULL)
+ runtime_dir = getenv(XDG_RUNTIME_DIR);
if (!runtime_dir) {
wl_log(error: XDG_RUNTIME_DIR not set in the environment.\n);
/* to prevent programs reporting
@@ -723,6 +725,7 @@ connect_to_socket(const char *name)
name_size =
snprintf(addr.sun_path, sizeof addr.sun_path,
%s/%s, runtime_dir, name) + 1;
+ unsetenv(WAYLAND_CLIENT_DIR);
assert(name_size 0);
if (name_size (int)sizeof addr.sun_path) {
diff --git a/src/wayland-server.c b/src/wayland-server.c
index 674aeca..09e8903 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -1080,7 +1080,9 @@ wl_socket_init_for_display_name(struct wl_socket *s,
const char *name)
int name_size;
const char *runtime_dir;
- runtime_dir = getenv(XDG_RUNTIME_DIR);
+ runtime_dir = getenv(WAYLAND_SERVER_DIR);
+ if (runtime_dir == NULL)
+ runtime_dir = getenv(XDG_RUNTIME_DIR);
if (!runtime_dir) {
wl_log(error: XDG_RUNTIME_DIR not set in the environment\n);
@@ -1093,6 +1095,7 @@ wl_socket_init_for_display_name(struct wl_socket *s,
const char *name)
s-addr.sun_family = AF_LOCAL;
name_size = snprintf(s-addr.sun_path, sizeof s-addr.sun_path,
%s/%s, runtime_dir, name) + 1;
+ unsetenv(WAYLAND_SERVER_DIR);
s-display_name = (s-addr.sun_path + name_size - 1) - strlen(name);
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH 2/2] Support for adjusting socket access rights to allow group of users to connect to the socket.
This is used for nested compositor architectures.
Signed-off-by: Imran Zaman imran.za...@gmail.com
---
src/wayland-server.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/src/wayland-server.c b/src/wayland-server.c
index 09e8903..721fabe 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -39,6 +39,8 @@
#include fcntl.h
#include sys/file.h
#include sys/stat.h
+#include sys/types.h
+#include grp.h
#include ffi.h
#include wayland-private.h
@@ -1117,6 +1119,10 @@ static int
_wl_display_add_socket(struct wl_display *display, struct wl_socket *s)
{
socklen_t size;
+ const char *socket_mode_str;
+ const char *socket_group_str;
+ const struct group *socket_group;
+ unsigned socket_mode;
s-fd = wl_os_socket_cloexec(PF_LOCAL, SOCK_STREAM, 0);
if (s-fd 0) {
@@ -1134,6 +1140,27 @@ _wl_display_add_socket(struct wl_display *display,
struct wl_socket *s)
return -1;
}
+ socket_group_str = getenv(WAYLAND_SERVER_GROUP);
+ if (socket_group_str != NULL) {
+ socket_group = getgrnam(socket_group_str);
+ if (socket_group != NULL) {
+ if (chown(s-addr.sun_path,
+ -1, socket_group-gr_gid) != 0)
+ wl_log(chown(\%s\) failed: %s,
+ s-addr.sun_path,
+ strerror(errno));
+ }
+ }
+ socket_mode_str = getenv(WAYLAND_SERVER_MODE);
+ if (socket_mode_str != NULL) {
+ if (sscanf(socket_mode_str, %o, socket_mode) 0)
+ if (chmod(s-addr.sun_path, socket_mode) != 0) {
+ wl_log(chmod(\%s\) failed: %s,
+ s-addr.sun_path,
+ strerror(errno));
+ }
+ }
+
s-source = wl_event_loop_add_fd(display-loop, s-fd,
WL_EVENT_READABLE,
socket_data, display);
--
1.9.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland] wl_strtol and wl_strtoul utility functions are added
Hi
The patch is used to replace strtol and strtoul with wl_strtol and
wl_strtoul with inputs and result checks.
The utility functions are used extensively in wayland and weston so added
appropriate
input and output checks; test cases are also updated; will push the patch
for weston as well.
BR
imran
commit 6be79948c9d408bb4f61cec5fff391f7ed7beb7b
Author: Imran Zaman imran.za...@intel.com
Date: Wed Oct 15 16:02:16 2014 +0300
wayland-util: added wl_strtol/wl_strtoul utility functions
The utility functions are used extensively so added appropriate
checks and test cases.
Signed-off-by: Imran Zaman imran.za...@intel.com
diff --git a/src/scanner.c b/src/scanner.c
index 809130b..3e30fe7 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -315,7 +315,6 @@ start_element(void *data, const char *element_name, const char **atts)
struct description *description;
const char *name, *type, *interface_name, *value, *summary, *since;
const char *allow_null;
- char *end;
int i, version;
ctx-loc.line_number = XML_GetCurrentLineNumber(ctx-parser);
@@ -404,9 +403,7 @@ start_element(void *data, const char *element_name, const char **atts)
message-destructor = 0;
if (since != NULL) {
- errno = 0;
- version = strtol(since, end, 0);
- if (errno == EINVAL || end == since || *end != '\0')
+ if (!wl_strtol(since, NULL, 0, version))
fail(ctx-loc,
invalid integer (%s)\n, since);
} else {
diff --git a/src/wayland-client.c b/src/wayland-client.c
index b0f77b9..1229b5f 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -824,13 +824,12 @@ wl_display_connect_to_fd(int fd)
WL_EXPORT struct wl_display *
wl_display_connect(const char *name)
{
- char *connection, *end;
+ char *connection;
int flags, fd;
connection = getenv(WAYLAND_SOCKET);
if (connection) {
- fd = strtol(connection, end, 0);
- if (*end != '\0')
+ if (!wl_strtol(connection, NULL, 0, fd))
return NULL;
flags = fcntl(fd, F_GETFD);
diff --git a/src/wayland-util.c b/src/wayland-util.c
index b099882..f8267f3 100644
--- a/src/wayland-util.c
+++ b/src/wayland-util.c
@@ -26,6 +26,9 @@
#include stdio.h
#include string.h
#include stdarg.h
+#include errno.h
+#include limits.h
+#include stdlib.h
#include wayland-util.h
#include wayland-private.h
@@ -361,6 +364,42 @@ wl_map_for_each(struct wl_map *map, wl_iterator_func_t func, void *data)
for_each_helper(map-server_entries, func, data);
}
+WL_EXPORT int
+wl_strtol(const char *str, char **endptr, int base, int32_t *val)
+{
+ char *end = NULL;
+ long v;
+
+ if (!str || !val) return 0;
+ if (!endptr) endptr = end;
+
+ errno = 0;
+ v = strtol(str, endptr, base);
+ if (errno != 0 || *endptr == str || **endptr != '\0')
+ return 0;
+
+ *val = v;
+ return 1;
+}
+
+WL_EXPORT int
+wl_strtoul(const char *str, char **endptr, int base, uint32_t *val)
+{
+ char *end = NULL;
+ unsigned long v;
+
+ if (!str || !val) return 0;
+ if (!endptr) endptr = end;
+
+ errno = 0;
+ v = strtoul(str, endptr, base);
+ if (errno != 0 || *endptr == str || **endptr != '\0')
+ return 0;
+
+ *val = v;
+ return 1;
+}
+
static void
wl_log_stderr_handler(const char *fmt, va_list arg)
{
diff --git a/src/wayland-util.h b/src/wayland-util.h
index fd32826..b77d4e3 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -243,6 +243,9 @@ static inline wl_fixed_t wl_fixed_from_int(int i)
return i * 256;
}
+int wl_strtol(const char *str, char **endptr, int base, int32_t *val);
+int wl_strtoul(const char *str, char **endptr, int base, uint32_t *val);
+
/**
* \brief A union representing all of the basic data types that can be passed
* along the wayland wire format.
diff --git a/tests/fixed-test.c b/tests/fixed-test.c
index 739a3b1..349cc48 100644
--- a/tests/fixed-test.c
+++ b/tests/fixed-test.c
@@ -88,3 +88,61 @@ TEST(fixed_int_conversions)
i = wl_fixed_to_int(f);
assert(i == -0x50);
}
+
+TEST(strtol_conversions)
+{
+ int ret;
+ int32_t val = -1;
+ char *end = NULL;
+
+ char *str = 12;
+ ret = wl_strtol(str, NULL, 10, val);
+ assert(ret == 1);
+ assert(val == 12);
+
+ ret = wl_strtol(str, end, 10, val);
+ assert(end != NULL);
+ assert(*end == '\0');
+
+ str = s12; val = -1;
+ ret = wl_strtol(str, NULL, 10, val);
+ assert(ret == 0);
+ assert(val == -1);
+
+ ret = wl_strtol(str, end, 10, val);
+ assert(end == str);
+
+ str = ; val = -1;
+ ret = wl_strtol(str, NULL, 10, val);
+ assert(ret == 0);
+ assert(val == -1);
+}
+
+TEST(strtoul_conversions)
+{
+ int ret;
+ uint32_t val = 0;
+ char *end = NULL;
+
+ char *str = 15;
+ ret = wl_strtoul(str, NULL, 10, val);
+ assert(ret == 1);
+ assert(val == 15);
+
+ ret = wl_strtoul(str, end, 10, val);
+ assert(end != NULL);
+ assert(*end == '\0');
+
+ str = s15; val = 0;
+ ret = wl_strtoul(str, NULL, 10, val);
+ assert(ret == 0);
+ assert(val == 0);
+
+ ret = wl_strtoul(str, end, 10, val);
+ assert(end == str);
+
+ str = ; val = 0;
+ ret = wl_strtoul(str, NULL, 10, val);
+ assert(ret == 0);
+ assert(val
[PATCH wayland] wl_strtol and wl_strtoul utility functions are added (inlined patch)
Hi
The patch is used to replace strtol and strtoul with wl_strtol and
wl_strtoul with inputs and result checks.
The utility functions are used extensively in wayland and weston so added
appropriate
input and output checks; test cases are also updated; will push the patch
for weston as well.
diff --git a/src/scanner.c b/src/scanner.c
index 809130b..3e30fe7 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -315,7 +315,6 @@ start_element(void *data, const char *element_name,
const char **atts)
struct description *description;
const char *name, *type, *interface_name, *value, *summary, *since;
const char *allow_null;
- char *end;
int i, version;
ctx-loc.line_number = XML_GetCurrentLineNumber(ctx-parser);
@@ -404,9 +403,7 @@ start_element(void *data, const char *element_name,
const char **atts)
message-destructor = 0;
if (since != NULL) {
- errno = 0;
- version = strtol(since, end, 0);
- if (errno == EINVAL || end == since || *end != '\0')
+ if (!wl_strtol(since, NULL, 0, version))
fail(ctx-loc,
invalid integer (%s)\n, since);
} else {
diff --git a/src/wayland-client.c b/src/wayland-client.c
index b0f77b9..1229b5f 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -824,13 +824,12 @@ wl_display_connect_to_fd(int fd)
WL_EXPORT struct wl_display *
wl_display_connect(const char *name)
{
- char *connection, *end;
+ char *connection;
int flags, fd;
connection = getenv(WAYLAND_SOCKET);
if (connection) {
- fd = strtol(connection, end, 0);
- if (*end != '\0')
+ if (!wl_strtol(connection, NULL, 0, fd))
return NULL;
flags = fcntl(fd, F_GETFD);
diff --git a/src/wayland-util.c b/src/wayland-util.c
index b099882..f8267f3 100644
--- a/src/wayland-util.c
+++ b/src/wayland-util.c
@@ -26,6 +26,9 @@
#include stdio.h
#include string.h
#include stdarg.h
+#include errno.h
+#include limits.h
+#include stdlib.h
#include wayland-util.h
#include wayland-private.h
@@ -361,6 +364,42 @@ wl_map_for_each(struct wl_map *map, wl_iterator_func_t
func, void *data)
for_each_helper(map-server_entries, func, data);
}
+WL_EXPORT int
+wl_strtol(const char *str, char **endptr, int base, int32_t *val)
+{
+ char *end = NULL;
+ long v;
+
+ if (!str || !val) return 0;
+ if (!endptr) endptr = end;
+
+ errno = 0;
+ v = strtol(str, endptr, base);
+ if (errno != 0 || *endptr == str || **endptr != '\0')
+ return 0;
+
+ *val = v;
+ return 1;
+}
+
+WL_EXPORT int
+wl_strtoul(const char *str, char **endptr, int base, uint32_t *val)
+{
+ char *end = NULL;
+ unsigned long v;
+
+ if (!str || !val) return 0;
+ if (!endptr) endptr = end;
+
+ errno = 0;
+ v = strtoul(str, endptr, base);
+ if (errno != 0 || *endptr == str || **endptr != '\0')
+ return 0;
+
+ *val = v;
+ return 1;
+}
+
static void
wl_log_stderr_handler(const char *fmt, va_list arg)
{
diff --git a/src/wayland-util.h b/src/wayland-util.h
index fd32826..b77d4e3 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -243,6 +243,9 @@ static inline wl_fixed_t wl_fixed_from_int(int i)
return i * 256;
}
+int wl_strtol(const char *str, char **endptr, int base, int32_t *val);
+int wl_strtoul(const char *str, char **endptr, int base, uint32_t *val);
+
/**
* \brief A union representing all of the basic data types that can be
passed
* along the wayland wire format.
diff --git a/tests/fixed-test.c b/tests/fixed-test.c
index 739a3b1..349cc48 100644
--- a/tests/fixed-test.c
+++ b/tests/fixed-test.c
@@ -88,3 +88,61 @@ TEST(fixed_int_conversions)
i = wl_fixed_to_int(f);
assert(i == -0x50);
}
+
+TEST(strtol_conversions)
+{
+ int ret;
+ int32_t val = -1;
+ char *end = NULL;
+
+ char *str = 12;
+ ret = wl_strtol(str, NULL, 10, val);
+ assert(ret == 1);
+ assert(val == 12);
+
+ ret = wl_strtol(str, end, 10, val);
+ assert(end != NULL);
+ assert(*end == '\0');
+
+ str = s12; val = -1;
+ ret = wl_strtol(str, NULL, 10, val);
+ assert(ret == 0);
+ assert(val == -1);
+
+ ret = wl_strtol(str, end, 10, val);
+ assert(end == str);
+
+ str = ; val = -1;
+ ret = wl_strtol(str, NULL, 10, val);
+ assert(ret == 0);
+ assert(val == -1);
+}
+
+TEST(strtoul_conversions)
+{
+ int ret;
+ uint32_t val = 0;
+ char *end = NULL;
+
+ char *str = 15;
+ ret = wl_strtoul(str, NULL, 10, val);
+ assert(ret == 1);
+ assert(val == 15);
+
+ ret = wl_strtoul(str, end, 10, val);
+ assert(end != NULL);
+ assert(*end == '\0');
+
+ str = s15; val = 0;
+ ret = wl_strtoul(str, NULL, 10, val);
+ assert(ret == 0);
+ assert(val == 0);
+
+ ret = wl_strtoul(str, end, 10, val);
+ assert(end == str);
+
+ str = ; val = 0;
+ ret = wl_strtoul(str, NULL, 10, val);
+ assert(ret == 0);
+ assert(val == 0);
+}
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland 1/2] support specifying custom directories for the client and server
Hi
support specifying custom directories for the client and server
sockets through environment variables.
This is in order to support nested compositor architectures where
system compositor using drm-backend is shared among multiple child
compositors using wayland-backend.
---
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 1229b5f..428af68 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -700,7 +700,9 @@ connect_to_socket(const char *name)
const char *runtime_dir;
int name_size, fd;
- runtime_dir = getenv(XDG_RUNTIME_DIR);
+ runtime_dir = getenv(WAYLAND_CLIENT_DIR);
+ if (runtime_dir == NULL)
+ runtime_dir = getenv(XDG_RUNTIME_DIR);
if (!runtime_dir) {
wl_log(error: XDG_RUNTIME_DIR not set in the environment.\n);
/* to prevent programs reporting
diff --git a/src/wayland-server.c b/src/wayland-server.c
index e3b7d9f..ce1eca8 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -1080,7 +1080,9 @@ wl_socket_init_for_display_name(struct wl_socket
*s, const char *name)
int name_size;
const char *runtime_dir;
- runtime_dir = getenv(XDG_RUNTIME_DIR);
+ runtime_dir = getenv(WAYLAND_SERVER_DIR);
+ if (runtime_dir == NULL)
+ runtime_dir = getenv(XDG_RUNTIME_DIR);
if (!runtime_dir) {
wl_log(error: XDG_RUNTIME_DIR not set in the environment\n);
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland 2/2] support specifying custom directories for the client and server
Hi
support for adjusting socket access rights to allow group of users to
connect to the socket.
This is used for nested compositor architectures.
-
diff --git a/src/wayland-server.c b/src/wayland-server.c
index ce1eca8..b1ca5e6 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -39,6 +39,8 @@
#include fcntl.h
#include sys/file.h
#include sys/stat.h
+#include sys/types.h
+#include grp.h
#include ffi.h
#include wayland-private.h
@@ -1079,6 +1081,10 @@ wl_socket_init_for_display_name(struct
wl_socket *s, const char *name)
{
int name_size;
const char *runtime_dir;
+ const char *socket_mode_str;
+ const char *socket_group_str;
+ const struct group *socket_group;
+ unsigned socket_mode;
runtime_dir = getenv(WAYLAND_SERVER_DIR);
if (runtime_dir == NULL)
@@ -1133,6 +1139,18 @@ _wl_display_add_socket(struct wl_display
*display, struct wl_socket *s)
return -1;
}
+ socket_group_str = getenv(WAYLAND_SERVER_GROUP);
+ if (socket_group_str != NULL) {
+ socket_group = getgrnam(socket_group_str);
+ if (socket_group != NULL)
+ chown(s-addr.sun_path, -1, socket_group-gr_gid);
+ }
+ socket_mode_str = getenv(WAYLAND_SERVER_MODE);
+ if (socket_mode_str != NULL) {
+ if (sscanf(socket_mode_str, %o, socket_mode) 0)
+ chmod(s-addr.sun_path, socket_mode);
+ }
+
s-source = wl_event_loop_add_fd(display-loop, s-fd,
WL_EVENT_READABLE,
socket_data, display);
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] wl_strtol and wl_strtoul utility functions are added (inlined patch)
The reason is that strtol is used at many places in weston/wayland..
and its not covering all the error cases everywhere.. so its better to
encapsulate it in a function which i did..
On Wed, Oct 15, 2014 at 9:31 PM, Jasper St. Pierre
jstpie...@mecheye.net wrote:
Why? What's the rationale for this?
On Wed, Oct 15, 2014 at 6:18 AM, Imran Zaman imran.za...@gmail.com wrote:
Hi
The patch is used to replace strtol and strtoul with wl_strtol and
wl_strtoul with inputs and result checks.
The utility functions are used extensively in wayland and weston so added
appropriate
input and output checks; test cases are also updated; will push the patch
for weston as well.
diff --git a/src/scanner.c b/src/scanner.c
index 809130b..3e30fe7 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -315,7 +315,6 @@ start_element(void *data, const char *element_name,
const char **atts)
struct description *description;
const char *name, *type, *interface_name, *value, *summary, *since;
const char *allow_null;
- char *end;
int i, version;
ctx-loc.line_number = XML_GetCurrentLineNumber(ctx-parser);
@@ -404,9 +403,7 @@ start_element(void *data, const char *element_name,
const char **atts)
message-destructor = 0;
if (since != NULL) {
- errno = 0;
- version = strtol(since, end, 0);
- if (errno == EINVAL || end == since || *end != '\0')
+ if (!wl_strtol(since, NULL, 0, version))
fail(ctx-loc,
invalid integer (%s)\n, since);
} else {
diff --git a/src/wayland-client.c b/src/wayland-client.c
index b0f77b9..1229b5f 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -824,13 +824,12 @@ wl_display_connect_to_fd(int fd)
WL_EXPORT struct wl_display *
wl_display_connect(const char *name)
{
- char *connection, *end;
+ char *connection;
int flags, fd;
connection = getenv(WAYLAND_SOCKET);
if (connection) {
- fd = strtol(connection, end, 0);
- if (*end != '\0')
+ if (!wl_strtol(connection, NULL, 0, fd))
return NULL;
flags = fcntl(fd, F_GETFD);
diff --git a/src/wayland-util.c b/src/wayland-util.c
index b099882..f8267f3 100644
--- a/src/wayland-util.c
+++ b/src/wayland-util.c
@@ -26,6 +26,9 @@
#include stdio.h
#include string.h
#include stdarg.h
+#include errno.h
+#include limits.h
+#include stdlib.h
#include wayland-util.h
#include wayland-private.h
@@ -361,6 +364,42 @@ wl_map_for_each(struct wl_map *map,
wl_iterator_func_t func, void *data)
for_each_helper(map-server_entries, func, data);
}
+WL_EXPORT int
+wl_strtol(const char *str, char **endptr, int base, int32_t *val)
+{
+ char *end = NULL;
+ long v;
+
+ if (!str || !val) return 0;
+ if (!endptr) endptr = end;
+
+ errno = 0;
+ v = strtol(str, endptr, base);
+ if (errno != 0 || *endptr == str || **endptr != '\0')
+ return 0;
+
+ *val = v;
+ return 1;
+}
+
+WL_EXPORT int
+wl_strtoul(const char *str, char **endptr, int base, uint32_t *val)
+{
+ char *end = NULL;
+ unsigned long v;
+
+ if (!str || !val) return 0;
+ if (!endptr) endptr = end;
+
+ errno = 0;
+ v = strtoul(str, endptr, base);
+ if (errno != 0 || *endptr == str || **endptr != '\0')
+ return 0;
+
+ *val = v;
+ return 1;
+}
+
static void
wl_log_stderr_handler(const char *fmt, va_list arg)
{
diff --git a/src/wayland-util.h b/src/wayland-util.h
index fd32826..b77d4e3 100644
--- a/src/wayland-util.h
+++ b/src/wayland-util.h
@@ -243,6 +243,9 @@ static inline wl_fixed_t wl_fixed_from_int(int i)
return i * 256;
}
+int wl_strtol(const char *str, char **endptr, int base, int32_t *val);
+int wl_strtoul(const char *str, char **endptr, int base, uint32_t *val);
+
/**
* \brief A union representing all of the basic data types that can be
passed
* along the wayland wire format.
diff --git a/tests/fixed-test.c b/tests/fixed-test.c
index 739a3b1..349cc48 100644
--- a/tests/fixed-test.c
+++ b/tests/fixed-test.c
@@ -88,3 +88,61 @@ TEST(fixed_int_conversions)
i = wl_fixed_to_int(f);
assert(i == -0x50);
}
+
+TEST(strtol_conversions)
+{
+ int ret;
+ int32_t val = -1;
+ char *end = NULL;
+
+ char *str = 12;
+ ret = wl_strtol(str, NULL, 10, val);
+ assert(ret == 1);
+ assert(val == 12);
+
+ ret = wl_strtol(str, end, 10, val);
+ assert(end != NULL);
+ assert(*end == '\0');
+
+ str = s12; val = -1;
+ ret = wl_strtol(str, NULL, 10, val);
+ assert(ret == 0);
+ assert(val == -1);
+
+ ret = wl_strtol(str, end, 10, val);
+ assert(end == str);
+
+ str = ; val = -1;
+ ret = wl_strtol(str, NULL, 10, val);
+ assert(ret == 0);
+ assert(val == -1);
+}
+
+TEST(strtoul_conversions)
+{
+ int ret;
+ uint32_t val = 0;
+ char *end = NULL;
+
+ char *str = 15;
+ ret = wl_strtoul(str, NULL, 10, val);
+ assert(ret == 1);
+ assert(val == 15);
+
+ ret = wl_strtoul(str, end, 10, val);
+ assert(end != NULL);
+ assert(*end == '\0');
+
+ str = s15; val = 0;
+ ret = wl_strtoul(str, NULL, 10, val);
+ assert(ret == 0);
+ assert(val
Re: [PATCH wayland] wl_strtol and wl_strtoul utility functions are added
The reason is that strtol is used at many places in weston/wayland.. and its not covering all the error cases everywhere (i.e. its buggy).. so its better to encapsulate it in a function with all the input and output checks... it can be moved to weston if its sound such a big deal... On Wed, Oct 15, 2014 at 9:42 PM, Jason Ekstrand ja...@jlekstrand.net wrote: I don't see how this belongs in libwayland. Sure, we use strtol twice, but I don't think that warrants adding 100 lines of wrapper functions and test cases. --Jason Ekstrand On Wed, Oct 15, 2014 at 6:16 AM, Rémi Denis-Courmont r...@remlab.net wrote: Le 2014-10-15 16:14, Imran Zaman a écrit : Hi The patch is used to replace strtol and strtoul with wl_strtol and wl_strtoul with inputs and result checks. I don't know where Wayland developers stand on this, but I would rather the client library function calls not clobber errno to zero. -- Rémi Denis-Courmont ___ 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
