From 5997373b55c4d3b0d2843716483f9f72fd7782ee Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Mon, 18 Apr 2016 15:34:39 -0600 Subject: [PATCH] VNC: Fix some obvious logic and coding errors found in early testing --- configs/samv71-xult/vnc/defconfig | 8 +---- graphics/vnc/server/vnc_fbdev.c | 51 +++++++++++++++++++++++-------- graphics/vnc/server/vnc_server.c | 28 +++++++++++++---- graphics/vnc/server/vnc_server.h | 14 +++++++-- graphics/vnc/server/vnc_updater.c | 28 ++++++++--------- 5 files changed, 87 insertions(+), 42 deletions(-) diff --git a/configs/samv71-xult/vnc/defconfig b/configs/samv71-xult/vnc/defconfig index 3f66d98714..5e5f76362a 100644 --- a/configs/samv71-xult/vnc/defconfig +++ b/configs/samv71-xult/vnc/defconfig @@ -1363,13 +1363,7 @@ CONFIG_NSH_IOBUFFER_SIZE=512 # CONFIG_SYSTEM_INSTALL is not set # CONFIG_SYSTEM_FLASH_ERASEALL is not set # CONFIG_SYSTEM_HEX2BIN is not set -CONFIG_SYSTEM_I2CTOOL=y -CONFIG_I2CTOOL_MINBUS=0 -CONFIG_I2CTOOL_MAXBUS=0 -CONFIG_I2CTOOL_MINADDR=0x03 -CONFIG_I2CTOOL_MAXADDR=0x77 -CONFIG_I2CTOOL_MAXREGADDR=0xff -CONFIG_I2CTOOL_DEFFREQ=400000 +# CONFIG_SYSTEM_I2CTOOL is not set # CONFIG_SYSTEM_HEXED is not set # CONFIG_SYSTEM_NETDB is not set # CONFIG_SYSTEM_RAMTEST is not set diff --git a/graphics/vnc/server/vnc_fbdev.c b/graphics/vnc/server/vnc_fbdev.c index 46bcc47e53..89a976c2c3 100644 --- a/graphics/vnc/server/vnc_fbdev.c +++ b/graphics/vnc/server/vnc_fbdev.c @@ -46,6 +46,7 @@ #include #include +#include #include #include "vnc_server.h" @@ -136,7 +137,7 @@ static struct vnc_fbinfo_s g_fbinfo[RFB_MAX_DISPLAYS]; * for the semaphores. */ -sem_t g_fbsem[RFB_MAX_DISPLAYS]; +struct fb_startup_s g_fbstartup[RFB_MAX_DISPLAYS]; /**************************************************************************** * Private Functions @@ -398,6 +399,9 @@ static int up_setcursor(FAR struct fb_vtable_s *vtable, static inline int vnc_wait_server(int display) { + int errcode; + int result; + /* Check if there has been a session allocated yet. This is one of the * first things that the VNC server will do with the kernel thread is * started. But we might be here before the thread has gotten that far. @@ -412,20 +416,38 @@ static inline int vnc_wait_server(int display) while (g_vnc_sessions[display] == NULL || g_vnc_sessions[display]->state != VNCSERVER_RUNNING) - { - /* The server is not yet running. Wait for the server to post the FB - * semaphore. In certain error situations, the server may post the - * semaphore, then reset it to zero. There are are certainly race - * conditions here, but I think none that are fatal. - */ + { + /* The server is not yet running. Wait for the server to post the FB + * semaphore. In certain error situations, the server may post the + * semaphore, then reset it to zero. There are are certainly race + * conditions here, but I think none that are fatal. + */ - while (sem_wait(&g_fbsem[display]) < 0) - { - /* sem_wait() should fail only if it is interrupt by a signal. */ + while (sem_wait(&g_fbstartup[display].fbsem) < 0) + { + errcode = get_errno(); - DEBUGASSERT(get_errno() == EINTR); - } - } + /* sem_wait() should fail only if it is interrupt by a signal. */ + + DEBUGASSERT(errcode == EINTR); + if (errcode != EINTR) + { + DEBUGASSERT(errcode > 0); + return -errcode; + } + } + + /* We were awakened. A result of -EBUSY means that the negotiation + * is not complete. Why would we be awakened in that case? Some + * counting semaphore screw-up? + */ + + result = g_fbstartup[display].result; + if (result != -EBUSY) + { + return result; + } + } return OK; } @@ -466,6 +488,9 @@ int up_fbinitialize(int display) /* Check if the server is already running */ + g_fbstartup[display].result = -EBUSY; + sem_reset(&g_fbstartup[display].fbsem, 0); + if (g_vnc_sessions[display] != NULL) { DEBUGASSERT(g_vnc_sessions[display]->state >= VNCSERVER_INITIALIZED); diff --git a/graphics/vnc/server/vnc_server.c b/graphics/vnc/server/vnc_server.c index 4fb802aad6..b83ebe6b9e 100644 --- a/graphics/vnc/server/vnc_server.c +++ b/graphics/vnc/server/vnc_server.c @@ -107,6 +107,12 @@ static void vnc_reset_session(FAR struct vnc_session_s *session, } /* [Re-]initialize the session. */ + + memset(&session->connect, 0, sizeof(struct socket)); + session->connect.s_crefs = 1; + memset(&session->listen, 0, sizeof(struct socket)); + session->listen.s_crefs = 1; + /* Put all of the pre-allocated update structures into the freelist */ sq_init(&session->updqueue); @@ -235,14 +241,16 @@ int vnc_server(int argc, FAR char *argv[]) if (argc != 2) { gdbg("ERROR: Unexpected number of arguments: %d\n", argc); - return EXIT_FAILURE; + ret = -EINVAL; + goto errout_with_post; } display = atoi(argv[1]); if (display < 0 || display >= RFB_MAX_DISPLAYS) { gdbg("ERROR: Invalid display number: %d\n", display); - return EXIT_FAILURE; + ret = -EINVAL; + goto errout_with_post; } /* Allocate the framebuffer memory. We rely on the fact that @@ -254,7 +262,8 @@ int vnc_server(int argc, FAR char *argv[]) { gdbg("ERROR: Failed to allocate framebuffer memory: %lu KB\n", (unsigned long)(RFB_SIZE / 1024)); - return -ENOMEM; + ret = -ENOMEM; + goto errout_with_post; } /* Allocate a session structure for this display */ @@ -263,6 +272,7 @@ int vnc_server(int argc, FAR char *argv[]) if (session == NULL) { gdbg("ERROR: Failed to allocate session\n"); + ret = -ENOMEM; goto errout_with_fb; } @@ -276,12 +286,13 @@ int vnc_server(int argc, FAR char *argv[]) for (; ; ) { - /* Release the last sesstion and [Re-]initialize the session structure + /* Release the last session and [Re-]initialize the session structure * for the next connection. */ vnc_reset_session(session, fb); - sem_reset(&g_fbsem[display], 0); + g_fbstartup[display].result = -EBUSY; + sem_reset(&g_fbstartup[display].fbsem, 0); /* Establish a connection with the VNC client */ @@ -318,7 +329,8 @@ int vnc_server(int argc, FAR char *argv[]) * updates. */ - sem_post(&g_fbsem[display]); + g_fbstartup[display].result = OK; + sem_post(&g_fbstartup[display].fbsem); /* Run the VNC receiver on this trhead. The VNC receiver handles * all Client-to-Server messages. The VNC receiver function does @@ -341,6 +353,10 @@ int vnc_server(int argc, FAR char *argv[]) errout_with_fb: kmm_free(fb); + +errout_with_post: + g_fbstartup[display].result = ret; + sem_post(&g_fbstartup[display].fbsem); return EXIT_FAILURE; } diff --git a/graphics/vnc/server/vnc_server.h b/graphics/vnc/server/vnc_server.h index 6129060c0b..41acacd822 100644 --- a/graphics/vnc/server/vnc_server.h +++ b/graphics/vnc/server/vnc_server.h @@ -146,7 +146,7 @@ /* Local framebuffer characteristics in bytes */ -#define RFB_BYTESPERPIXEL ((RFB_BITSPERPIXEL + 7) >> 8) +#define RFB_BYTESPERPIXEL ((RFB_BITSPERPIXEL + 7) >> 3) #define RFB_STRIDE (RFB_BYTESPERPIXEL * CONFIG_VNCSERVER_SCREENWIDTH) #define RFB_SIZE (RFB_STRIDE * CONFIG_VNCSERVER_SCREENHEIGHT) @@ -229,6 +229,16 @@ struct vnc_session_s uint8_t outbuf[VNCSERVER_UPDATE_BUFSIZE]; }; +/* This structure is used to communicate start-up status between the server + * the framebuffer driver. + */ + +struct fb_startup_s +{ + sem_t fbsem; /* Framebuffer driver will wait on this */ + int16_t result; /* OK: successfully initialized */ +}; + /**************************************************************************** * Public Data ****************************************************************************/ @@ -249,7 +259,7 @@ EXTERN FAR struct vnc_session_s *g_vnc_sessions[RFB_MAX_DISPLAYS]; /* Used to synchronize the server thread with the framebuffer driver. */ -EXTERN sem_t g_fbsem[RFB_MAX_DISPLAYS]; +EXTERN struct fb_startup_s g_fbstartup[RFB_MAX_DISPLAYS]; /**************************************************************************** * Public Function Prototypes diff --git a/graphics/vnc/server/vnc_updater.c b/graphics/vnc/server/vnc_updater.c index 067eba160b..1a76c17665 100644 --- a/graphics/vnc/server/vnc_updater.c +++ b/graphics/vnc/server/vnc_updater.c @@ -338,7 +338,7 @@ uint32_t vnc_convert_rgb32_888(uint32_t rgb) ****************************************************************************/ static size_t vnc_copy16(FAR struct vnc_session_s *session, - nxgl_coord_t row, nxgl_coord_t col, + nxgl_coord_t row, nxgl_coord_t col, nxgl_coord_t height, nxgl_coord_t width, vnc_convert16_t convert) { @@ -357,14 +357,14 @@ static size_t vnc_copy16(FAR struct vnc_session_s *session, /* Source rectangle start address (left/top)*/ - srcleft = (FAR uint16_t *)(session->fb + RFB_STRIDE * y + RFB_BYTESPERPIXEL * x); + srcleft = (FAR uint16_t *)(session->fb + RFB_STRIDE * row + RFB_BYTESPERPIXEL * col); /* Transfer each row from the source buffer into the update buffer */ - for (y = 0; y < row; y++) + for (y = 0; y < height; y++) { src = srcleft; - for (y = 0; y < row; y++) + for (x = 0; x < width; x++) { *dest++ = convert(*src); src++; @@ -390,12 +390,12 @@ static size_t vnc_copy16(FAR struct vnc_session_s *session, /* Source rectangle start address */ - srcleft = (FAR uint32_t *)(session->fb + RFB_STRIDE * y + RFB_BYTESPERPIXEL * x); + srcleft = (FAR uint32_t *)(session->fb + RFB_STRIDE * row + RFB_BYTESPERPIXEL * col); - for (y = 0; y < row; y++) + for (y = 0; y < height; y++) { src = srcleft; - for (y = 0; y < row; y++) + for (x = 0; x < width; x++) { *dest++ = convert(*src); src++; @@ -428,7 +428,7 @@ static size_t vnc_copy16(FAR struct vnc_session_s *session, ****************************************************************************/ static size_t vnc_copy32(FAR struct vnc_session_s *session, - nxgl_coord_t row, nxgl_coord_t col, + nxgl_coord_t row, nxgl_coord_t col, nxgl_coord_t height, nxgl_coord_t width, vnc_convert32_t convert) { @@ -447,14 +447,14 @@ static size_t vnc_copy32(FAR struct vnc_session_s *session, /* Source rectangle start address (left/top)*/ - srcleft = (FAR uint16_t *)(session->fb + RFB_STRIDE * y + RFB_BYTESPERPIXEL * x); + srcleft = (FAR uint16_t *)(session->fb + RFB_STRIDE * row + RFB_BYTESPERPIXEL * col); /* Transfer each row from the source buffer into the update buffer */ - for (y = 0; y < row; y++) + for (y = 0; y < height; y++) { src = srcleft; - for (y = 0; y < row; y++) + for (x = 0; x < width; x++) { *dest++ = convert(*src); src++; @@ -480,12 +480,12 @@ static size_t vnc_copy32(FAR struct vnc_session_s *session, /* Source rectangle start address */ - srcleft = (FAR uint32_t *)(session->fb + RFB_STRIDE * y + RFB_BYTESPERPIXEL * x); + srcleft = (FAR uint32_t *)(session->fb + RFB_STRIDE * row + RFB_BYTESPERPIXEL * col); - for (y = 0; y < row; y++) + for (y = 0; y < height; y++) { src = srcleft; - for (y = 0; y < row; y++) + for (x = 0; x < width; x++) { *dest++ = convert(*src); src++;