On 15 July 2015 at 19:11, Bryce Harrington <[email protected]> wrote: > On Wed, Jul 15, 2015 at 01:09:03PM +0100, Emil Velikov wrote: >> Hello gents, >> >> On 15 July 2015 at 09:51, Marek Chalupa <[email protected]> wrote: >> > Reviewed-by: Marek Chalupa <[email protected]> >> > >> > (http://lists.freedesktop.org/archives/wayland-devel/2015-May/021952.html) >> > >> > On 05/16/2015 07:38 AM, Dima Ryazanov wrote: >> >> >> >> snprintf does not allocate memory, so we can never get an out-of-memory >> >> error. >> >> >> >> (Also, the error handler would free xwl_output after it was already >> >> registered >> >> as an event listener.) >> >> >> >> Signed-off-by: Dima Ryazanov <[email protected]> >> >> --- >> >> hw/xwayland/xwayland-output.c | 6 +----- >> >> 1 file changed, 1 insertion(+), 5 deletions(-) >> >> >> >> diff --git a/hw/xwayland/xwayland-output.c b/hw/xwayland/xwayland-output.c >> >> index 9baf4eb..41937b8 100644 >> >> --- a/hw/xwayland/xwayland-output.c >> >> +++ b/hw/xwayland/xwayland-output.c >> >> @@ -165,11 +165,7 @@ xwl_output_create(struct xwl_screen *xwl_screen, >> >> uint32_t id) >> >> &wl_output_interface, 2); >> >> wl_output_add_listener(xwl_output->output, &output_listener, >> >> xwl_output); >> >> >> >> - if (snprintf(name, sizeof name, "XWAYLAND%d", serial++) < 0) { >> >> - ErrorF("create_output ENOMEM\n"); >> >> - free(xwl_output); >> >> - return NULL; >> >> - } >> >> + snprintf(name, sizeof name, "XWAYLAND%d", serial++); >> >> >> man snprintf does mention >> >> " If an output error is encountered, a negative value is returned." >> >> It does not explicitly state under what conditions. Not sure if we >> should really care here or not but at the very least the error message >> is inaccurate :-) > > Well, snprintf errors if the size of the buffer was too small for what > it needs to write to. That can't happen here because the buffer is > large (256) and the most that's going to be written is an int. > Obviously that'll be the case in many(all?) implementations, but this is not guaranteed by the standard.
> So, the only way this would fail is if the code changed, which would be > a programming error. In which case maybe an assert() would be more > appropriate? > From a quick look only a few places in xserver check the return value - xquarts, os and xkb. In (most of) these cases the buffer size may not sufficient. So I think one can safely ignore my comment and don't bother with the assert :-) Cheers, Emil _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
