From 56b5b9f7ad7b0f4949e484be6a56b5e7bcf760e4 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Wed, 6 Mar 2019 10:16:47 -0600 Subject: [PATCH] graphics/nxterm: Remove the nxterm_unregister interface. The correct way to unregister a device is to unlink it. --- TODO | 4 +- configs/sam4e-ek/nxwm/defconfig | 1 - configs/samv71-xult/vnxwm/defconfig | 1 - configs/shenzhou/nxwm/defconfig | 1 - configs/stm3240g-eval/nxterm/defconfig | 1 - configs/stm32f429i-disco/nxwm/defconfig | 1 - graphics/nxterm/Make.defs | 7 +- graphics/nxterm/nxterm.h | 46 +++++---- graphics/nxterm/nxterm_driver.c | 127 ++++++++++++++++++++++-- graphics/nxterm/nxterm_unregister.c | 19 ++-- include/nuttx/nx/nxterm.h | 20 ---- 11 files changed, 162 insertions(+), 66 deletions(-) diff --git a/TODO b/TODO index 1f2820f073..6a6b0b730f 100644 --- a/TODO +++ b/TODO @@ -2486,8 +2486,8 @@ o Graphics Subsystem (graphics/) Within the OS, NxTK currently used only in graphics/nxterm. With some effort, I think NxTerm could be moved to apps/ as well. - Currently, nxterm violates POSIX interface. nxtk_register() and - nx_register() called from application. see apps/examples/nxterm + Currently, nxterm violates POSIX interface. nxterm_redraw() and + nxterm_kbdin() are called from applications. see apps/examples/nxterm and apps/graphics/NxWidgets/nxwm./src/cnxterm.cxx. Status: Open diff --git a/configs/sam4e-ek/nxwm/defconfig b/configs/sam4e-ek/nxwm/defconfig index b1e3316af4..76bb27a56a 100644 --- a/configs/sam4e-ek/nxwm/defconfig +++ b/configs/sam4e-ek/nxwm/defconfig @@ -57,7 +57,6 @@ CONFIG_NET_BROADCAST=y CONFIG_NET_ICMP=y CONFIG_NET_ICMP_SOCKET=y CONFIG_NET_MAX_LISTENPORTS=16 -CONFIG_NET_SOCKOPTS=y CONFIG_NET_TCP=y CONFIG_NET_TCPBACKLOG=y CONFIG_NET_TCP_CONNS=16 diff --git a/configs/samv71-xult/vnxwm/defconfig b/configs/samv71-xult/vnxwm/defconfig index 9df71a4d35..c945bd6d1e 100644 --- a/configs/samv71-xult/vnxwm/defconfig +++ b/configs/samv71-xult/vnxwm/defconfig @@ -65,7 +65,6 @@ CONFIG_NETUTILS_WEBCLIENT=y CONFIG_NET_ARP_SEND=y CONFIG_NET_ICMP=y CONFIG_NET_ICMP_SOCKET=y -CONFIG_NET_SOCKOPTS=y CONFIG_NET_STATISTICS=y CONFIG_NET_TCP=y CONFIG_NET_TCPBACKLOG=y diff --git a/configs/shenzhou/nxwm/defconfig b/configs/shenzhou/nxwm/defconfig index e350c0aaa6..981a17c270 100644 --- a/configs/shenzhou/nxwm/defconfig +++ b/configs/shenzhou/nxwm/defconfig @@ -48,7 +48,6 @@ CONFIG_NETUTILS_WEBCLIENT=y CONFIG_NET_ICMP=y CONFIG_NET_ICMP_SOCKET=y CONFIG_NET_MAX_LISTENPORTS=16 -CONFIG_NET_SOCKOPTS=y CONFIG_NET_STATISTICS=y CONFIG_NET_TCP=y CONFIG_NET_TCPBACKLOG=y diff --git a/configs/stm3240g-eval/nxterm/defconfig b/configs/stm3240g-eval/nxterm/defconfig index 3085d9b008..a5b9ff9984 100644 --- a/configs/stm3240g-eval/nxterm/defconfig +++ b/configs/stm3240g-eval/nxterm/defconfig @@ -52,7 +52,6 @@ CONFIG_NET_BROADCAST=y CONFIG_NET_ICMP=y CONFIG_NET_ICMP_SOCKET=y CONFIG_NET_MAX_LISTENPORTS=40 -CONFIG_NET_SOCKOPTS=y CONFIG_NET_STATISTICS=y CONFIG_NET_TCP=y CONFIG_NET_TCPBACKLOG=y diff --git a/configs/stm32f429i-disco/nxwm/defconfig b/configs/stm32f429i-disco/nxwm/defconfig index a2cb4f4544..3b769e8832 100644 --- a/configs/stm32f429i-disco/nxwm/defconfig +++ b/configs/stm32f429i-disco/nxwm/defconfig @@ -6,7 +6,6 @@ # modifications. # # CONFIG_ARCH_FPU is not set -# CONFIG_FB_CMAP is not set # CONFIG_NXFONTS_DISABLE_16BPP is not set # CONFIG_NXTK_DEFAULT_BORDERCOLORS is not set # CONFIG_NX_DISABLE_16BPP is not set diff --git a/graphics/nxterm/Make.defs b/graphics/nxterm/Make.defs index 469d4bfde2..37aa7b3a5a 100644 --- a/graphics/nxterm/Make.defs +++ b/graphics/nxterm/Make.defs @@ -37,8 +37,11 @@ ifeq ($(CONFIG_NXTERM),y) CSRCS += nx_register.c nxterm_driver.c nxterm_font.c nxterm_putc.c CSRCS += nxterm_redraw.c nxterm_register.c nxterm_scroll.c -CSRCS += nxterm_vt100.c nxterm_unregister.c nxtk_register.c -CSRCS += nxtool_register.c +CSRCS += nxterm_vt100.c nxtk_register.c nxtool_register.c + +ifneq ($(CONFIG_DISABLE_PSEUDOFS_OPERATIONS),y) +CSRCS += nxterm_unregister.c +endif ifeq ($(CONFIG_NXTERM_NXKBDIN),y) CSRCS += nxterm_kbdin.c diff --git a/graphics/nxterm/nxterm.h b/graphics/nxterm/nxterm.h index ea3cb1ef0c..708307047f 100644 --- a/graphics/nxterm/nxterm.h +++ b/graphics/nxterm/nxterm.h @@ -122,43 +122,47 @@ struct nxterm_bitmap_s struct nxterm_state_s { FAR const struct nxterm_operations_s *ops; /* Window operations */ - FAR void *handle; /* The window handle */ - FAR struct nxterm_window_s wndo; /* Describes the window and font */ - sem_t exclsem; /* Forces mutually exclusive access */ + FAR void *handle; /* The window handle */ + FAR struct nxterm_window_s wndo; /* Describes the window and font */ + sem_t exclsem; /* Forces mutually exclusive access */ #ifdef CONFIG_DEBUG_FEATURES - pid_t holder; /* Deadlock avoidance */ + pid_t holder; /* Deadlock avoidance */ #endif - uint8_t minor; /* Device minor number */ +#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS + bool unlinked; /* True: Driver has been unlinked */ + uint8_t orefs; /* Open reference count */ +#endif + uint8_t minor; /* Device minor number */ /* Text output support */ - uint8_t fheight; /* Max height of a font in pixels */ - uint8_t fwidth; /* Max width of a font in pixels */ - uint8_t spwidth; /* The width of a space */ + uint8_t fheight; /* Max height of a font in pixels */ + uint8_t fwidth; /* Max width of a font in pixels */ + uint8_t spwidth; /* The width of a space */ - uint16_t maxchars; /* Size of the bm[] array */ - uint16_t nchars; /* Number of chars in the bm[] array */ + uint16_t maxchars; /* Size of the bm[] array */ + uint16_t nchars; /* Number of chars in the bm[] array */ - struct nxgl_point_s fpos; /* Next display position */ + struct nxgl_point_s fpos; /* Next display position */ /* VT100 escape sequence processing */ - char seq[VT100_MAX_SEQUENCE]; /* Buffered characters */ - uint8_t nseq; /* Number of buffered characters */ + char seq[VT100_MAX_SEQUENCE]; /* Buffered characters */ + uint8_t nseq; /* Number of buffered characters */ /* Font cache data storage */ - FCACHE fcache; /* Font cache handle */ + FCACHE fcache; /* Font cache handle */ struct nxterm_bitmap_s cursor; struct nxterm_bitmap_s bm[CONFIG_NXTERM_MXCHARS]; /* Keyboard input support */ #ifdef CONFIG_NXTERM_NXKBDIN - sem_t waitsem; /* Supports waiting for input data */ - uint8_t nwaiters; /* Number of threads waiting for data */ - uint8_t head; /* rxbuffer head/input index */ - uint8_t tail; /* rxbuffer tail/output index */ + sem_t waitsem; /* Supports waiting for input data */ + uint8_t nwaiters; /* Number of threads waiting for data */ + uint8_t head; /* rxbuffer head/input index */ + uint8_t tail; /* rxbuffer tail/output index */ uint8_t rxbuffer[CONFIG_NXTERM_KBDBUFSIZE]; @@ -184,6 +188,7 @@ extern const struct file_operations g_nxterm_drvrops; /**************************************************************************** * Public Function Prototypes ****************************************************************************/ + /* Semaphore helpers */ #ifdef CONFIG_DEBUG_FEATURES @@ -194,11 +199,14 @@ int nxterm_sempost(FAR struct nxterm_state_s *priv); # define nxterm_sempost(p) nxsem_post(&p->exclsem) #endif -/* Common device registration */ +/* Common device registration/un-registration */ FAR struct nxterm_state_s *nxterm_register(NXTERM handle, FAR struct nxterm_window_s *wndo, FAR const struct nxterm_operations_s *ops, int minor); +#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS +void nxterm_unregister(FAR struct nxterm_state_s *priv); +#endif #ifdef CONFIG_NXTERM_NXKBDIN ssize_t nxterm_read(FAR struct file *filep, FAR char *buffer, size_t len); diff --git a/graphics/nxterm/nxterm_driver.c b/graphics/nxterm/nxterm_driver.c index 824f674cfd..080848051a 100644 --- a/graphics/nxterm/nxterm_driver.c +++ b/graphics/nxterm/nxterm_driver.c @@ -56,8 +56,12 @@ ****************************************************************************/ static int nxterm_open(FAR struct file *filep); +static int nxterm_close(FAR struct file *filep); static ssize_t nxterm_write(FAR struct file *filep, FAR const char *buffer, size_t buflen); +#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS +static int nxterm_unlink(FAR struct inode *inode); +#endif /**************************************************************************** * Public Data @@ -70,15 +74,19 @@ static ssize_t nxterm_write(FAR struct file *filep, FAR const char *buffer, const struct file_operations g_nxterm_drvrops = { nxterm_open, /* open */ - 0, /* close */ + nxterm_close, /* close */ nxterm_read, /* read */ nxterm_write, /* write */ - 0, /* seek */ - 0 /* ioctl */ + 0, /* seek */ + 0 /* ioctl */ #ifndef CONFIG_DISABLE_POLL , nxterm_poll /* poll */ #endif +#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS + , + nxterm_unlink /* unlink */ +#endif }; #else /* CONFIG_NXTERM_NXKBDIN */ @@ -86,14 +94,18 @@ const struct file_operations g_nxterm_drvrops = const struct file_operations g_nxterm_drvrops = { nxterm_open, /* open */ - 0, /* close */ - 0, /* read */ + nxterm_close, /* close */ + 0, /* read */ nxterm_write, /* write */ - 0, /* seek */ - 0 /* ioctl */ + 0, /* seek */ + 0 /* ioctl */ #ifndef CONFIG_DISABLE_POLL , - 0 /* poll */ + 0 /* poll */ +#endif +#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS + , + nxterm_unlink /* unlink */ #endif }; @@ -130,12 +142,66 @@ static int nxterm_open(FAR struct file *filep) } #endif +#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS + /* Increment the count of open file references */ + + DEBUGASSERT(priv->orefs != UINT8_MAX); + priv->orefs++; +#endif + /* Assign the driver structure to the file */ filep->f_priv = priv; return OK; } +/**************************************************************************** + * Name: nxterm_close + ****************************************************************************/ + +static int nxterm_close(FAR struct file *filep) +{ +#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS + FAR struct nxterm_state_s *priv; + int ret; + + /* Recover our private state structure */ + + DEBUGASSERT(filep != NULL && filep->f_priv != NULL); + priv = (FAR struct nxterm_state_s *)filep->f_priv; + + /* Get exclusive access */ + + ret = nxterm_semwait(priv); + if (ret < 0) + { + return ret; + } + + /* Has the driver been unlinked? Was this the last open refernce to the + * terminal driver? + */ + + DEBUGASSERT(priv->orefs > 0); + if (priv->unlinked && priv->orefs <= 1) + { + /* Yes.. Unregister the terminal device */ + + nxterm_unregister(priv); + } + else + { + /* No.. Just decrement the count of open file references */ + + priv->orefs--; + } + + nxterm_sempost(priv); +#endif + + return OK; +} + /**************************************************************************** * Name: nxterm_write ****************************************************************************/ @@ -151,7 +217,7 @@ static ssize_t nxterm_write(FAR struct file *filep, FAR const char *buffer, /* Recover our private state structure */ - DEBUGASSERT(filep && filep->f_priv); + DEBUGASSERT(filep != NULL && filep->f_priv != NULL); priv = (FAR struct nxterm_state_s *)filep->f_priv; /* Get exclusive access */ @@ -248,6 +314,49 @@ static ssize_t nxterm_write(FAR struct file *filep, FAR const char *buffer, return (ssize_t)buflen; } +/**************************************************************************** + * Name: nxterm_unlink + ****************************************************************************/ + +#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS +static int nxterm_unlink(FAR struct inode *inode) +{ + FAR struct nxterm_state_s *priv; + int ret; + + DEBUGASSERT(inode != NULLL && inode->i_private != NULL); + priv = inode->i_private; + + /* Get exclusive access */ + + ret = nxterm_semwait(priv); + if (ret < 0) + { + return ret; + } + + /* Are there open references to the terminal driver? */ + + if (priv->orefs > 0) + { + /* Yes.. Just mark the driver unlinked. Resources will be cleaned up + * when the final reference is close. + */ + + priv->unlinked = true; + } + else + { + /* No.. Unregister the terminal device now */ + + nxterm_unregister(priv); + } + + nxterm_sempost(priv); + return OK; +} +#endif + /**************************************************************************** * Public Functions ****************************************************************************/ diff --git a/graphics/nxterm/nxterm_unregister.c b/graphics/nxterm/nxterm_unregister.c index 2439406ea1..baf0d741e5 100644 --- a/graphics/nxterm/nxterm_unregister.c +++ b/graphics/nxterm/nxterm_unregister.c @@ -50,6 +50,8 @@ #include "nxterm.h" +#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS + /**************************************************************************** * Public Functions ****************************************************************************/ @@ -58,27 +60,24 @@ * Name: nxterm_unregister * * Description: - * Un-register to NX console device. + * Un-register an NX console device. * * Input Parameters: - * handle - A handle previously returned by nx_register, nxtk_register, or - * nxtool_register. + * priv - NxTerm private state structure instance. * * Returned Value: * None * ****************************************************************************/ -void nxterm_unregister(NXTERM handle) +void nxterm_unregister(FAR struct nxterm_state_s *priv) { - FAR struct nxterm_state_s *priv; char devname[NX_DEVNAME_SIZE]; - DEBUGASSERT(handle); + DEBUGASSERT(priv != NULL); - /* Get the reference to the driver structure from the handle */ + /* Destroy semaphores */ - priv = (FAR struct nxterm_state_s *)handle; nxsem_destroy(&priv->exclsem); #ifdef CONFIG_NXTERM_NXKBDIN nxsem_destroy(&priv->waitsem); @@ -95,5 +94,7 @@ void nxterm_unregister(NXTERM handle) /* Free the private data structure */ - kmm_free(handle); + kmm_free(priv); } + +#endif /* !CONFIG_DISABLE_PSEUDOFS_OPERATIONS */ diff --git a/include/nuttx/nx/nxterm.h b/include/nuttx/nx/nxterm.h index a7c8d648f1..a70f36c2bd 100644 --- a/include/nuttx/nx/nxterm.h +++ b/include/nuttx/nx/nxterm.h @@ -298,26 +298,6 @@ NXTERM nxtk_register(NXTKWINDOW hfwnd, FAR struct nxterm_window_s *wndo, NXTERM nxtool_register(NXTKWINDOW hfwnd, FAR struct nxterm_window_s *wndo, int minor); -/**************************************************************************** - * Name: nxterm_unregister - * - * Description: - * Un-register to NX console device. - * - * This is an internal NuttX interface and should not be called directly - * from applications. - * - * Input Parameters: - * handle - A handle previously returned by nx_register, nxtk_register, or - * nxtool_register. - * - * Returned Value: - * None - * - ****************************************************************************/ - -void nxterm_unregister(NXTERM handle); - /**************************************************************************** * Name: nxterm_redraw *