You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by gn...@apache.org on 2020/01/07 15:02:05 UTC

[incubator-nuttx-apps] branch master updated: Various fixes (#6)

This is an automated email from the ASF dual-hosted git repository.

gnutt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx-apps.git


The following commit(s) were added to refs/heads/master by this push:
     new 5c936ce  Various fixes (#6)
5c936ce is described below

commit 5c936ce0e4151115e537feba04653af667a29d93
Author: Alin Jerpelea <al...@sony.com>
AuthorDate: Tue Jan 7 09:01:23 2020 -0600

    Various fixes (#6)
    
    Author: Gregory Nutt <gn...@nuttx.org>
    
        Run all .c and .h affected by this PR through nxstyle.
    
    Author: Alin Jerpelea <al...@sony.com>
    
        * system/usbmsc: Fix accessing uninitialized pointer
        * fsutils/inifile: Fix a memory leak in inifile error case
        * fsutils/mksmartfs: Fix uninitialized return code
        * system/zmodem: Fix a compile error in zmodem debug enabled
        * nshlib/nsh_fscmds.c: Add syntax check to cp command
    
        If the destication of NutShell cp command is the same with the source,
        it may cause the file corruption. Add the syntax check of argument to
        avoid this problem.
---
 fsutils/inifile/inifile.c     | 20 ++++++++------
 fsutils/mksmartfs/mksmartfs.c | 17 ++++++++----
 nshlib/nsh_fscmds.c           | 64 +++++++++++++++++++++++++++----------------
 system/usbmsc/usbmsc_main.c   | 23 ++++++++++------
 system/zmodem/zm_send.c       | 37 ++++++++++++++-----------
 5 files changed, 99 insertions(+), 62 deletions(-)

diff --git a/fsutils/inifile/inifile.c b/fsutils/inifile/inifile.c
index 1057b93..6707abf 100644
--- a/fsutils/inifile/inifile.c
+++ b/fsutils/inifile/inifile.c
@@ -106,7 +106,7 @@ struct inifile_state_s
 {
   FILE *instream;
   int   nextch;
-  char  line[CONFIG_FSUTILS_INIFILE_MAXLINE+1];
+  char  line[CONFIG_FSUTILS_INIFILE_MAXLINE + 1];
 };
 
 /****************************************************************************
@@ -185,7 +185,7 @@ static int inifile_read_line(FAR struct inifile_state_s *priv)
   while ((nbytes < CONFIG_FSUTILS_INIFILE_MAXLINE) &&
          (priv->nextch != EOF) &&
          (priv->nextch != '\n'))
-   {
+    {
       /* Always ignore carriage returns */
 
       if (priv->nextch != '\r')
@@ -242,7 +242,8 @@ static int inifile_read_noncomment_line(FAR struct inifile_state_s *priv)
 
   /* Read until either a (1) no further lines are found in
    * the file, or (2) a line that does not begin with a semi-colon
-   * is found */
+   * is found.
+   */
 
   do nbytes = inifile_read_line(priv);
   while (nbytes > 0 && priv->line[0] == ';');
@@ -354,7 +355,7 @@ static bool inifile_read_variable(FAR struct inifile_state_s *priv,
    * the section is found, or (3) a valid variable assignment is found.
    */
 
-  for (;;)
+  for (; ; )
     {
       /* Read the next line in the buffer */
 
@@ -365,9 +366,9 @@ static bool inifile_read_variable(FAR struct inifile_state_s *priv,
        */
 
       if (!nbytes || priv->line[0] == '[')
-       {
-         return false;
-       }
+        {
+          return false;
+        }
 
       /* Search for the '=' delimiter.  NOTE  the line is guaranteed to
        * be NULL terminated by inifile_read_noncomment_line().
@@ -389,7 +390,7 @@ static bool inifile_read_variable(FAR struct inifile_state_s *priv,
            * a NULL string
            */
 
-          varinfo->variable = (char*)priv->line;
+          varinfo->variable = (FAR char *)priv->line;
           varinfo->value    = (ptr + 1);
           return true;
         }
@@ -418,7 +419,7 @@ static FAR char *
 
   iniinfo("variable=\"%s\"\n", variable);
 
-  for (;;)
+  for (; ; )
     {
       /* Get the next variable from this section. */
 
@@ -527,6 +528,7 @@ INIHANDLE inifile_initialize(FAR const char *inifile_name)
   else
     {
       inidbg("ERROR: Could not open \"%s\"\n", inifile_name);
+      free(priv);
       return (INIHANDLE)NULL;
     }
 }
diff --git a/fsutils/mksmartfs/mksmartfs.c b/fsutils/mksmartfs/mksmartfs.c
index 9066d13..733feb8 100644
--- a/fsutils/mksmartfs/mksmartfs.c
+++ b/fsutils/mksmartfs/mksmartfs.c
@@ -95,7 +95,8 @@
 int issmartfs(FAR const char *pathname)
 {
   struct smart_format_s fmt;
-  int ret, fd;
+  int fd;
+  int ret;
 
   /* Find the inode of the block driver identified by 'source' */
 
@@ -121,6 +122,7 @@ int issmartfs(FAR const char *pathname)
     }
 
 out:
+
   /* Close the driver */
 
   close(fd);
@@ -157,22 +159,25 @@ out:
  ****************************************************************************/
 
 #ifdef CONFIG_SMARTFS_MULTI_ROOT_DIRS
-int mksmartfs(FAR const char *pathname, uint16_t sectorsize, uint8_t nrootdirs)
+int mksmartfs(FAR const char *pathname, uint16_t sectorsize,
+              uint8_t nrootdirs)
 #else
 int mksmartfs(FAR const char *pathname, uint16_t sectorsize)
 #endif
 {
   struct smart_format_s fmt;
-  int ret, fd;
-  int x;
-  uint8_t type;
   struct smart_read_write_s request;
+  uint8_t type;
+  int fd;
+  int x;
+  int ret;
 
   /* Find the inode of the block driver indentified by 'source' */
 
   fd = open(pathname, O_RDWR);
   if (fd < 0)
     {
+      ret = -ENOENT;
       goto errout;
     }
 
@@ -227,11 +232,13 @@ int mksmartfs(FAR const char *pathname, uint16_t sectorsize)
     }
 
 errout_with_driver:
+
   /* Close the driver */
 
   close(fd);
 
 errout:
+
   /* Release all allocated memory */
 
   /* Return any reported errors */
diff --git a/nshlib/nsh_fscmds.c b/nshlib/nsh_fscmds.c
index 89db3f0..cc9cdb0 100644
--- a/nshlib/nsh_fscmds.c
+++ b/nshlib/nsh_fscmds.c
@@ -56,7 +56,6 @@
 #include <errno.h>
 #include <debug.h>
 
-
 #if !defined(CONFIG_DISABLE_MOUNTPOINT)
 #  ifdef CONFIG_FS_READABLE /* Need at least one filesytem in configuration */
 #    include <sys/mount.h>
@@ -240,47 +239,47 @@ static int ls_handler(FAR struct nsh_vtbl_s *vtbl, FAR const char *dirpath,
 
           if ((buf.st_mode & S_IRUSR) != 0)
             {
-              details[1]='r';
+              details[1] = 'r';
             }
 
           if ((buf.st_mode & S_IWUSR) != 0)
             {
-              details[2]='w';
+              details[2] = 'w';
             }
 
           if ((buf.st_mode & S_IXUSR) != 0)
             {
-              details[3]='x';
+              details[3] = 'x';
             }
 
           if ((buf.st_mode & S_IRGRP) != 0)
             {
-              details[4]='r';
+              details[4] = 'r';
             }
 
           if ((buf.st_mode & S_IWGRP) != 0)
             {
-              details[5]='w';
+              details[5] = 'w';
             }
 
           if ((buf.st_mode & S_IXGRP) != 0)
             {
-              details[6]='x';
+              details[6] = 'x';
             }
 
           if ((buf.st_mode & S_IROTH) != 0)
             {
-              details[7]='r';
+              details[7] = 'r';
             }
 
           if ((buf.st_mode & S_IWOTH) != 0)
             {
-              details[8]='w';
+              details[8] = 'w';
             }
 
           if ((buf.st_mode & S_IXOTH) != 0)
             {
-              details[9]='x';
+              details[9] = 'x';
             }
 
           nsh_output(vtbl, " %s", details);
@@ -561,7 +560,7 @@ int cmd_cp(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
 
           /* Construct the full path to the new file */
 
-          allocpath = nsh_getdirpath(vtbl, destpath, basename(argv[1]) );
+          allocpath = nsh_getdirpath(vtbl, destpath, basename(argv[1]));
           if (!allocpath)
             {
               nsh_error(vtbl, g_fmtcmdoutofmemory, argv[0]);
@@ -581,6 +580,14 @@ int cmd_cp(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
         }
     }
 
+  /* Check if the destination does not match the source */
+
+  if (strcmp(destpath, srcpath) == 0)
+    {
+      nsh_error(vtbl, g_fmtsyntax, argv[0]);
+      goto errout_with_allocpath;
+    }
+
   /* Now open the destination */
 
   wrfd = open(destpath, oflags, 0666);
@@ -592,7 +599,7 @@ int cmd_cp(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
 
   /* Now copy the file */
 
-  for (;;)
+  for (; ; )
     {
       int nbytesread;
       int nbyteswritten;
@@ -644,9 +651,10 @@ int cmd_cp(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
                 }
               else
                 {
-                 /* Read error */
+                  /* Read error */
 
-                  nsh_error(vtbl, g_fmtcmdfailed, argv[0], "write", NSH_ERRNO);
+                  nsh_error(vtbl, g_fmtcmdfailed, argv[0], "write",
+                            NSH_ERRNO);
                 }
 
               goto errout_with_wrfd;
@@ -678,6 +686,7 @@ errout_with_srcpath:
     {
       nsh_freefullpath(srcpath);
     }
+
 errout:
   return ret;
 }
@@ -862,7 +871,8 @@ int cmd_losmart(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
   /* Get the losetup options:  Two forms are supported:
    *
    *   losmart -d <loop-device>
-   *   losmart [-m minor-number] [-o <offset>] [-e erasesize] [-s sectsize] [-r] <filename>
+   *   losmart [-m minor-number] [-o <offset>] [-e erasesize] [-s sectsize]
+   *           [-r] <filename>
    *
    * NOTE that the -o and -r options are accepted with the -d option, but
    * will be ignored.
@@ -955,7 +965,8 @@ int cmd_losmart(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
     {
       /* Tear down the loop device. */
 
-      ret = ioctl(fd, SMART_LOOPIOC_TEARDOWN, (unsigned long)((uintptr_t) loopdev));
+      ret = ioctl(fd, SMART_LOOPIOC_TEARDOWN,
+                 (unsigned long)((uintptr_t) loopdev));
       if (ret < 0)
         {
           nsh_error(vtbl, g_fmtcmdfailed, argv[0], "ioctl", NSH_ERRNO);
@@ -973,7 +984,8 @@ int cmd_losmart(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
       setup.offset    = offset;     /* An offset that may be applied to the device */
       setup.readonly  = readonly;   /* True: Read access will be supported only */
 
-      ret = ioctl(fd, SMART_LOOPIOC_SETUP, (unsigned long)((uintptr_t)&setup));
+      ret = ioctl(fd, SMART_LOOPIOC_SETUP,
+                  (unsigned long)((uintptr_t)&setup));
       if (ret < 0)
         {
           nsh_error(vtbl, g_fmtcmdfailed, argv[0], "ioctl", NSH_ERRNO);
@@ -1091,7 +1103,7 @@ int cmd_ls(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
       switch (option)
         {
           case 'l':
-            lsflags |= (LSFLAGS_SIZE|LSFLAGS_LONG);
+            lsflags |= (LSFLAGS_SIZE | LSFLAGS_LONG);
             break;
 
           case 'R':
@@ -1169,7 +1181,8 @@ int cmd_ls(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
        * file
        */
 
-      ret = ls_handler(vtbl, fullpath, NULL, (FAR void *)((uintptr_t)lsflags));
+      ret = ls_handler(vtbl, fullpath, NULL,
+                       (FAR void *)((uintptr_t)lsflags));
     }
   else
     {
@@ -1178,7 +1191,7 @@ int cmd_ls(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
       nsh_output(vtbl, "%s:\n", fullpath);
 
       ret = nsh_foreach_direntry(vtbl, "ls", fullpath, ls_handler,
-                                 (FAR void*)((uintptr_t)lsflags));
+                                 (FAR void *)((uintptr_t)lsflags));
       if (ret == OK && (lsflags & LSFLAGS_RECURSIVE) != 0)
         {
           /* Then recurse to list each directory within the directory */
@@ -1282,7 +1295,7 @@ int cmd_mkfatfs(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
 
   /* There should be exactly one parameter left on the command-line */
 
-  if (optind == argc-1)
+  if (optind == argc - 1)
     {
       fullpath = nsh_getfullpath(vtbl, argv[optind]);
       if (fullpath == NULL)
@@ -1480,7 +1493,8 @@ int cmd_mksmartfs(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
                 nsh_error(vtbl, "Sector size must be 256-16384\n");
                 return EINVAL;
               }
-            if (sectorsize & (sectorsize-1))
+
+            if (sectorsize & (sectorsize - 1))
               {
                 nsh_error(vtbl, "Sector size must be power of 2\n");
                 return EINVAL;
@@ -1523,7 +1537,8 @@ int cmd_mksmartfs(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
 #endif
           if (ret < 0)
             {
-              nsh_error(vtbl, g_fmtcmdfailed, argv[0], "mksmartfs", NSH_ERRNO);
+              nsh_error(vtbl, g_fmtcmdfailed, argv[0], "mksmartfs",
+                        NSH_ERRNO);
             }
         }
 
@@ -1594,6 +1609,7 @@ int cmd_readlink(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
   ssize_t len;
 
   /* readlink <link> */
+
   /* Get the fullpath to the directory */
 
   fullpath = nsh_getfullpath(vtbl, argv[1]);
@@ -1734,7 +1750,7 @@ int cmd_cmp(FAR struct nsh_vtbl_s *vtbl, int argc, char **argv)
    * files.
    */
 
-  for (;;)
+  for (; ; )
     {
       char buf1[128];
       char buf2[128];
diff --git a/system/usbmsc/usbmsc_main.c b/system/usbmsc/usbmsc_main.c
index d4f01b2..c78ce5e 100644
--- a/system/usbmsc/usbmsc_main.c
+++ b/system/usbmsc/usbmsc_main.c
@@ -387,6 +387,7 @@ static int usbmsc_enumerate(struct usbtrace_s *trace, void *arg)
           break;
         }
     }
+
   return OK;
 }
 #endif
@@ -435,7 +436,7 @@ static void usbmsc_disconnect(FAR void *handle)
 int main(int argc, FAR char *argv[])
 {
   struct boardioc_usbdev_ctrl_s ctrl;
-  FAR void *handle;
+  FAR void *handle = NULL;
   int ret;
 
   /* If this program is implemented as the NSH 'msconn' command, then we
@@ -443,8 +444,8 @@ int main(int argc, FAR char *argv[])
    * called re-entrantly.
    */
 
-  /* Check if there is a non-NULL USB mass storage device handle (meaning that the
-   * USB mass storage device is already configured).
+  /* Check if there is a non-NULL USB mass storage device handle (meaning
+   * that the USB mass storage device is already configured).
    */
 
   if (g_usbmsc.mshandle)
@@ -482,7 +483,8 @@ int main(int argc, FAR char *argv[])
   ret = boardctl(BOARDIOC_USBDEV_CONTROL, (uintptr_t)&ctrl);
   if (ret < 0)
     {
-      printf("mcsonn_main: boardctl(BOARDIOC_USBDEV_CONTROL) failed: %d\n", -ret);
+      printf("mcsonn_main: boardctl(BOARDIOC_USBDEV_CONTROL) failed: %d\n",
+             -ret);
       return EXIT_FAILURE;
     }
 
@@ -490,12 +492,17 @@ int main(int argc, FAR char *argv[])
 
   /* Then exports the LUN(s) */
 
-  printf("mcsonn_main: Configuring with NLUNS=%d\n", CONFIG_SYSTEM_USBMSC_NLUNS);
+  printf("mcsonn_main: Configuring with NLUNS=%d\n",
+         CONFIG_SYSTEM_USBMSC_NLUNS);
   ret = usbmsc_configure(CONFIG_SYSTEM_USBMSC_NLUNS, &handle);
   if (ret < 0)
     {
       printf("mcsonn_main: usbmsc_configure failed: %d\n", -ret);
-      usbmsc_disconnect(handle);
+      if (handle)
+        {
+          usbmsc_disconnect(handle);
+        }
+
       return EXIT_FAILURE;
     }
 
@@ -567,8 +574,8 @@ int main(int argc, FAR char *argv[])
 
   check_test_memory_usage("After usbmsc_exportluns()");
 
-  /* Return the USB mass storage device handle so it can be used by the 'msconn'
-   * command.
+  /* Return the USB mass storage device handle so it can be used by the
+   * 'msconn' command.
    */
 
   printf("mcsonn_main: Connected\n");
diff --git a/system/zmodem/zm_send.c b/system/zmodem/zm_send.c
index 6efbfbb..24fb605 100644
--- a/system/zmodem/zm_send.c
+++ b/system/zmodem/zm_send.c
@@ -130,6 +130,7 @@ enum zmodem_state_e
 /****************************************************************************
  * Private Function Prototypes
  ****************************************************************************/
+
 /* Transition actions */
 
 static int zms_zrinit(FAR struct zm_state_s *pzm);
@@ -163,7 +164,8 @@ static int zms_error(FAR struct zm_state_s *pzm);
 /* Internal helpers */
 
 static int zms_startfiledata(FAR struct zms_state_s *pzms);
-static int zms_sendfile(FAR struct zms_state_s *pzms, FAR const char *filename,
+static int zms_sendfile(FAR struct zms_state_s *pzms,
+                        FAR const char *filename,
                         FAR const char *rfilename, uint8_t f0, uint8_t f1);
 
 /****************************************************************************
@@ -173,6 +175,7 @@ static int zms_sendfile(FAR struct zms_state_s *pzms, FAR const char *filename,
 /****************************************************************************
  * Private Data
  ****************************************************************************/
+
 /* Events handled in state ZMS_START - ZRQINIT sent, waiting for ZRINIT from
  * receiver
  */
@@ -414,7 +417,7 @@ static int zms_zrinit(FAR struct zm_state_s *pzm)
    *   F0 and F1 contain the  bitwise OR of the receiver capability flags
    *   P0 and ZP1 contain the size of the receiver's buffer in bytes (or 0
    *     if nonstop I/O is allowed.
-  */
+   */
 
   pzms->rcvmax = (uint16_t)pzm->hdrdata[2] << 8 | (uint16_t)pzm->hdrdata[1];
   rcaps        = (uint16_t)pzm->hdrdata[3] << 8 | (uint16_t)pzm->hdrdata[4];
@@ -494,7 +497,7 @@ static int zms_zrinit(FAR struct zm_state_s *pzm)
   else
 #  endif
     {
-      zmdbg("ZMS_STATE %d->%d\n", pzm->state, );
+      zmdbg("ZMS_STATE %d->%d\n", pzm->state, ZMS_DONE);
       pzm->state = ZMS_DONE;
       return ZM_XFRDONE;
     }
@@ -522,9 +525,9 @@ static int zms_attention(FAR struct zm_state_s *pzm)
 
   if (pzm->state == ZMS_SENDING || pzm->state == ZMS_SENDWAIT)
     {
-     /* Enter a wait state and see what they want.  Next header *should* be
-      * ZRPOS.
-      */
+      /* Enter a wait state and see what they want.  Next header *should* be
+       * ZRPOS.
+       */
 
       zmdbg("ZMS_STATE %d->%d: Interrupt\n", pzm->state, ZMS_SENDWAIT);
 
@@ -627,7 +630,7 @@ static int zms_stderrdata(FAR struct zm_state_s *pzm)
   zmdbg("ZMS_STATE %d\n", pzm->state);
 
   pzm->pktbuf[pzm->pktlen] = '\0';
-  fprintf(stderr, "Message: %s", (char*)pzm->pktbuf);
+  fprintf(stderr, "Message: %s", (FAR char *)pzm->pktbuf);
   return OK;
 }
 
@@ -671,10 +674,10 @@ static int zms_sendzsinit(FAR struct zm_state_s *pzm)
   pzm->state = ZMS_INITACK;
 
   /* Send the ZSINIT header (optional)
-  *
-  * Paragraph 11.3  ZSINIT.  "The Sender sends flags followed by a binary
-  * data subpacket terminated with ZCRCW."
-  */
+   *
+   * Paragraph 11.3  ZSINIT.  "The Sender sends flags followed by a binary
+   * data subpacket terminated with ZCRCW."
+   */
 
   ret = zm_sendbinhdr(pzm, ZSINIT, g_zeroes);
   if (ret >= 0)
@@ -895,7 +898,7 @@ static int zms_sendpacket(FAR struct zm_state_s *pzm)
       zmdbg("sndsize: %d unacked: %d rcvmax: %d\n",
             sndsize, unacked, pzms->rcvmax);
 
-     if (pzms->rcvmax != 0)
+      if (pzms->rcvmax != 0)
         {
           /* If we were to send 'sndsize' more bytes, would that exceed recvmax? */
 
@@ -1024,7 +1027,7 @@ static int zms_sendpacket(FAR struct zm_state_s *pzm)
             }
         }
 
-       /* Save the ZDLE in the transmit buffer */
+      /* Save the ZDLE in the transmit buffer */
 
       *ptr++ = ZDLE;
 
@@ -1221,7 +1224,8 @@ static int zms_sendnak(FAR struct zm_state_s *pzm)
       return -errorcode;
     }
 
-  zmdbg("ZMS_STATE %d: offset: %ld\n", pzm->state, (unsigned long)pzms->offset);
+  zmdbg("ZMS_STATE %d: offset: %ld\n",
+        pzm->state, (unsigned long)pzms->offset);
 
   return zms_sendpacket(pzm);
 }
@@ -1446,7 +1450,8 @@ static int zms_startfiledata(FAR struct zms_state_s *pzms)
 
 /* Called by user to begin transmission of a file */
 
-static int zms_sendfile(FAR struct zms_state_s *pzms, FAR const char *filename,
+static int zms_sendfile(FAR struct zms_state_s *pzms,
+                        FAR const char *filename,
                         FAR const char *rfilename, uint8_t f0, uint8_t f1)
 {
   struct stat buf;
@@ -1705,7 +1710,7 @@ int zms_release(ZMSHANDLE handle)
 
   /* Send "OO" */
 
-  nwritten = zm_remwrite(pzms->cmn.remfd, (FAR const uint8_t*)"OO", 2);
+  nwritten = zm_remwrite(pzms->cmn.remfd, (FAR const uint8_t *)"OO", 2);
   if (nwritten < 0)
     {
       zmdbg("ERROR: zm_remwrite failed: %d\n", (int)nwritten);