xiaoxiang781216 commented on code in PR #8109:
URL: https://github.com/apache/nuttx/pull/8109#discussion_r1072349047


##########
fs/shm/shm_open.c:
##########
@@ -0,0 +1,181 @@
+/****************************************************************************
+ * fs/shm/shm_open.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <stdio.h>
+#include <fcntl.h>
+#include <errno.h>
+
+#include "inode/inode.h"
+#include "shm/shmfs.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static int file_shm_open(FAR struct file *shm, FAR const char *name,
+                         int oflags, mode_t mode)
+{
+  FAR struct inode *inode;
+  struct inode_search_s desc;
+  char fullpath[sizeof(CONFIG_FS_SHMFS_VFS_PATH) + CONFIG_NAME_MAX + 2];
+  int ret;
+
+  /* Make sure that a non-NULL name is supplied */
+
+  if (!shm || !name)
+    {
+      return -EINVAL;
+    }
+
+  /* Remove any number of leading '/' */
+
+  while (*name == '/')
+    {
+      name++;
+    }
+
+  /* Empty name supplied? */
+
+  if (*name == '\0')
+    {
+      return -EINVAL;
+    }
+
+  /* Name too long? */
+
+  if (strnlen(name, CONFIG_NAME_MAX) > CONFIG_NAME_MAX)

Review Comment:
   is it possilbe to > CONFIG_NAME_MAX with strnlen?



##########
fs/shm/shmfs_alloc.c:
##########
@@ -0,0 +1,143 @@
+/****************************************************************************
+ * fs/shm/shmfs_alloc.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdlib.h>
+#include <nuttx/pgalloc.h>
+
+#include "inode/inode.h"
+#include "shm/shmfs.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+FAR struct shmfs_object_s *shmfs_alloc_object(size_t length)
+{
+  FAR struct shmfs_object_s *object;
+  bool allocated = false;
+
+#if defined(CONFIG_BUILD_FLAT)
+  /* in FLAT build, allocate the object metadata and the data in the same
+   * chunk in kernel heap
+   */
+
+  object = kmm_zalloc(sizeof(struct shmfs_object_s) + length);
+  if (object)
+    {
+      object->paddr = (&object->paddr) + 1;

Review Comment:
    object->paddr = object->paddr + 1;



##########
fs/shm/shmfs_alloc.c:
##########
@@ -0,0 +1,143 @@
+/****************************************************************************
+ * fs/shm/shmfs_alloc.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdlib.h>
+#include <nuttx/pgalloc.h>
+
+#include "inode/inode.h"

Review Comment:
   remove inode.h



##########
fs/shm/shm_unlink.c:
##########
@@ -0,0 +1,167 @@
+/****************************************************************************
+ * fs/shm/shm_unlink.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include "shm/shmfs.h"
+#include "inode/inode.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static int file_shm_unlink(FAR const char *name)
+{
+  FAR struct inode *inode;
+  struct inode_search_s desc;
+  char fullpath[sizeof(CONFIG_FS_SHMFS_VFS_PATH) + CONFIG_NAME_MAX + 2];
+  int ret;
+
+  /* Make sure that a non-NULL name is supplied */
+
+  if (!name)
+    {
+      return -ENOENT;
+    }
+
+  /* Remove any number of leading '/' */
+
+  while (*name == '/')

Review Comment:
   don't need inode_nextname already handle this.



##########
fs/shm/shm_open.c:
##########
@@ -0,0 +1,181 @@
+/****************************************************************************
+ * fs/shm/shm_open.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <stdio.h>
+#include <fcntl.h>
+#include <errno.h>
+
+#include "inode/inode.h"
+#include "shm/shmfs.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static int file_shm_open(FAR struct file *shm, FAR const char *name,
+                         int oflags, mode_t mode)
+{
+  FAR struct inode *inode;
+  struct inode_search_s desc;
+  char fullpath[sizeof(CONFIG_FS_SHMFS_VFS_PATH) + CONFIG_NAME_MAX + 2];
+  int ret;
+
+  /* Make sure that a non-NULL name is supplied */
+
+  if (!shm || !name)
+    {
+      return -EINVAL;
+    }
+
+  /* Remove any number of leading '/' */
+
+  while (*name == '/')

Review Comment:
   don't need inode_nextname already handle this.



##########
fs/shm/shm_unlink.c:
##########
@@ -0,0 +1,167 @@
+/****************************************************************************
+ * fs/shm/shm_unlink.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include "shm/shmfs.h"
+#include "inode/inode.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static int file_shm_unlink(FAR const char *name)
+{
+  FAR struct inode *inode;
+  struct inode_search_s desc;
+  char fullpath[sizeof(CONFIG_FS_SHMFS_VFS_PATH) + CONFIG_NAME_MAX + 2];
+  int ret;
+
+  /* Make sure that a non-NULL name is supplied */
+
+  if (!name)
+    {
+      return -ENOENT;
+    }
+
+  /* Remove any number of leading '/' */
+
+  while (*name == '/')
+    {
+      name++;
+    }
+
+  /* Empty name supplied? */
+
+  if (*name == '\0')
+    {
+      return -ENOENT;
+    }
+
+  /* Name too long? */
+
+  if (strnlen(name, CONFIG_NAME_MAX) > CONFIG_NAME_MAX)
+    {
+      return -ENAMETOOLONG;
+    }
+
+  /* Get the full path to the shm object */
+
+  snprintf(fullpath, sizeof(fullpath),
+           CONFIG_FS_SHMFS_VFS_PATH "/%s", name);
+
+  /* Get the inode for this shm object */
+
+  SETUP_SEARCH(&desc, fullpath, false);
+
+  ret = inode_lock();
+  if (ret < 0)
+    {
+      goto errout_with_search;
+    }
+
+  ret = inode_find(&desc);
+  if (ret < 0)
+    {
+      /* There is no inode that includes in this path */
+
+      goto errout_with_sem;
+    }
+
+  /* Get the search results */
+
+  inode = desc.node;
+  DEBUGASSERT(inode != NULL);
+
+  /* Verify that what we found is, indeed, an shm inode */
+
+  if (!INODE_IS_SHM(inode))
+    {
+      ret = -ENOENT;
+      inode_release(inode);
+      goto errout_with_sem;
+    }
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  if (inode->u.i_ops->unlink)
+    {
+      /* Notify the shmfs driver that it has been unlinked */
+
+      ret = inode->u.i_ops->unlink(inode);
+      if (ret < 0)
+        {
+          goto errout_with_inode;
+        }
+    }
+#endif
+
+  /* Remove the old inode from the tree. If we hold a reference count
+   * on the inode, it will not be deleted now.  This will set the
+   * FSNODEFLAG_DELETED bit in the inode flags.
+   */
+
+  inode_release(inode);

Review Comment:
   double free at line 142



##########
fs/shm/shmfs_alloc.c:
##########
@@ -0,0 +1,143 @@
+/****************************************************************************
+ * fs/shm/shmfs_alloc.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdlib.h>
+#include <nuttx/pgalloc.h>
+
+#include "inode/inode.h"
+#include "shm/shmfs.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+FAR struct shmfs_object_s *shmfs_alloc_object(size_t length)
+{
+  FAR struct shmfs_object_s *object;
+  bool allocated = false;
+
+#if defined(CONFIG_BUILD_FLAT)
+  /* in FLAT build, allocate the object metadata and the data in the same
+   * chunk in kernel heap
+   */
+
+  object = kmm_zalloc(sizeof(struct shmfs_object_s) + length);
+  if (object)
+    {
+      object->paddr = (&object->paddr) + 1;
+      allocated = true;
+    }
+
+#elif defined(CONFIG_BUILD_PROTECTED)
+  /* in PROTECTED build, allocate the shm object in kernel heap, and shared
+   * memory in user heap
+   */
+
+  object = kmm_zalloc(sizeof(struct shmfs_object_s));
+  if (object)
+    {
+      object->paddr = kumm_zalloc(length);
+
+      if (object->paddr)
+        {
+           allocated = true;
+        }
+    }
+
+#elif defined(CONFIG_BUILD_KERNEL)
+  /* in KERNEL build, allocate the shared memory from page pool and store the
+   * physical address
+   */
+
+  size_t i = 0;
+  void **pages;
+  size_t n_pages = MM_NPAGES(length);
+
+  object = kmm_zalloc(sizeof(struct shmfs_object_s) +
+                      (n_pages - 1) * sizeof(object->paddr));
+
+  if (object)
+    {
+      pages = &object->paddr;
+      for (; i < n_pages; i++)
+        {
+          pages[i] = (void *)mm_pgalloc(1);

Review Comment:
   FAR



##########
fs/shm/shm_unlink.c:
##########
@@ -0,0 +1,167 @@
+/****************************************************************************
+ * fs/shm/shm_unlink.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include "shm/shmfs.h"
+#include "inode/inode.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static int file_shm_unlink(FAR const char *name)
+{
+  FAR struct inode *inode;
+  struct inode_search_s desc;
+  char fullpath[sizeof(CONFIG_FS_SHMFS_VFS_PATH) + CONFIG_NAME_MAX + 2];
+  int ret;
+
+  /* Make sure that a non-NULL name is supplied */
+
+  if (!name)
+    {
+      return -ENOENT;
+    }
+
+  /* Remove any number of leading '/' */
+
+  while (*name == '/')
+    {
+      name++;
+    }
+
+  /* Empty name supplied? */
+
+  if (*name == '\0')
+    {
+      return -ENOENT;
+    }
+
+  /* Name too long? */
+
+  if (strnlen(name, CONFIG_NAME_MAX) > CONFIG_NAME_MAX)

Review Comment:
   ditto



##########
fs/shm/shmfs.c:
##########
@@ -0,0 +1,380 @@
+/****************************************************************************
+ * fs/shm/shmfs.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <assert.h>
+
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/mm/map.h>
+
+#if defined (CONFIG_BUILD_KERNEL)
+#include <nuttx/arch.h>
+#include <nuttx/pgalloc.h>
+#include <nuttx/sched.h>
+#endif
+
+#include "shm/shmfs.h"
+#include "inode/inode.h"
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static int shmfs_close(FAR struct file *filep);
+static ssize_t shmfs_read(FAR struct file *filep, FAR char *buffer,
+                          size_t buflen);
+static ssize_t shmfs_write(FAR struct file *filep, FAR const char *buffer,
+                           size_t buflen);
+static int shmfs_truncate(FAR struct file *filep, off_t length);
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+static int shmfs_unlink(FAR struct inode *inode);
+#endif
+
+static int shmfs_mmap(FAR struct file *filep,
+                      FAR struct mm_map_entry_s *entry);
+static int shmfs_munmap(FAR struct task_group_s *group,
+                        FAR struct mm_map_entry_s *entry,
+                        FAR void *start,
+                        size_t length);
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+const struct file_operations shmfs_operations =
+{
+  NULL,             /* open */
+  shmfs_close,      /* close */
+  shmfs_read,       /* read */
+  shmfs_write,      /* write */
+  NULL,             /* seek */
+  NULL,             /* ioctl */
+  shmfs_mmap,       /* mmap */
+  shmfs_truncate,   /* truncate */
+  NULL,             /* poll */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  shmfs_unlink      /* unlink */
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: shmfs_read
+ ****************************************************************************/
+
+static ssize_t shmfs_read(FAR struct file *filep, FAR char *buffer,
+                          size_t buflen)
+{
+  return -ENOSYS;
+}
+
+/****************************************************************************
+ * Name: shmfs_write
+ ****************************************************************************/
+
+static ssize_t shmfs_write(FAR struct file *filep, FAR const char *buffer,
+                           size_t buflen)
+{
+  return -ENOSYS;
+}
+
+/****************************************************************************
+ * Name: shmfs_release
+ ****************************************************************************/
+
+static int shmfs_release(FAR struct inode *inode)
+{
+  /* If the file has been unlinked previously, delete the contents.
+   * The inode is released after this call, hence checking if i_crefs <= 1.
+   */
+
+  int ret = inode_lock();
+  if (ret >= 0)
+    {
+      if (inode->i_parent == NULL &&
+          inode->i_crefs <= 1)
+        {
+          shmfs_free_object(inode->i_private);
+          inode->i_private = NULL;
+          ret = OK;
+        }
+
+      inode_unlock();
+    }
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: shmfs_close
+ ****************************************************************************/
+
+static int shmfs_close(FAR struct file *filep)
+{
+  /* Release the shmfs object. The object gets deleted if no-one has
+   * reference to it (either mmap or open file) and the object has been
+   * unlinked
+   */
+
+  return shmfs_release(filep->f_inode);
+}
+
+/****************************************************************************
+ * Name: shmfs_truncate
+ ****************************************************************************/
+
+static int shmfs_truncate(FAR struct file *filep, off_t length)
+{
+  FAR struct shmfs_object_s *object;
+  int ret;
+
+  if (length == 0)
+    {
+      return -EINVAL;
+    }
+
+  ret = inode_lock();
+  if (ret >= 0)
+    {
+      object = filep->f_inode->i_private;
+      if (!object)
+        {
+          filep->f_inode->i_private = shmfs_alloc_object(length);

Review Comment:
   need update ret if fail



##########
fs/shm/shmfs_alloc.c:
##########
@@ -0,0 +1,143 @@
+/****************************************************************************
+ * fs/shm/shmfs_alloc.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdlib.h>
+#include <nuttx/pgalloc.h>
+
+#include "inode/inode.h"
+#include "shm/shmfs.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+FAR struct shmfs_object_s *shmfs_alloc_object(size_t length)
+{
+  FAR struct shmfs_object_s *object;
+  bool allocated = false;
+
+#if defined(CONFIG_BUILD_FLAT)
+  /* in FLAT build, allocate the object metadata and the data in the same
+   * chunk in kernel heap
+   */
+
+  object = kmm_zalloc(sizeof(struct shmfs_object_s) + length);
+  if (object)
+    {
+      object->paddr = (&object->paddr) + 1;
+      allocated = true;
+    }
+
+#elif defined(CONFIG_BUILD_PROTECTED)
+  /* in PROTECTED build, allocate the shm object in kernel heap, and shared
+   * memory in user heap
+   */
+
+  object = kmm_zalloc(sizeof(struct shmfs_object_s));
+  if (object)
+    {
+      object->paddr = kumm_zalloc(length);
+
+      if (object->paddr)
+        {
+           allocated = true;
+        }
+    }
+
+#elif defined(CONFIG_BUILD_KERNEL)
+  /* in KERNEL build, allocate the shared memory from page pool and store the
+   * physical address
+   */
+
+  size_t i = 0;
+  void **pages;
+  size_t n_pages = MM_NPAGES(length);
+
+  object = kmm_zalloc(sizeof(struct shmfs_object_s) +
+                      (n_pages - 1) * sizeof(object->paddr));
+
+  if (object)
+    {
+      pages = &object->paddr;
+      for (; i < n_pages; i++)
+        {
+          pages[i] = (void *)mm_pgalloc(1);
+          if (!pages[i])
+            {
+              break;
+            }
+        }
+    }
+
+  if (i == n_pages)
+    {
+      allocated = true;
+    }
+#endif
+
+  if (allocated)
+    {
+      object->length = length;
+    }
+  else
+    {
+      /* delete any partial allocation */
+
+      shmfs_free_object(object);
+      object = NULL;
+    }
+
+  return object;
+}
+
+void shmfs_free_object(FAR struct shmfs_object_s *object)
+{
+#if defined(CONFIG_BUILD_KERNEL)
+  size_t i;
+  size_t n_pages = MM_NPAGES(object->length);
+  void **pages;

Review Comment:
   FAR



##########
fs/shm/shmfs_alloc.c:
##########
@@ -0,0 +1,143 @@
+/****************************************************************************
+ * fs/shm/shmfs_alloc.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdlib.h>
+#include <nuttx/pgalloc.h>
+
+#include "inode/inode.h"
+#include "shm/shmfs.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+FAR struct shmfs_object_s *shmfs_alloc_object(size_t length)
+{
+  FAR struct shmfs_object_s *object;
+  bool allocated = false;
+
+#if defined(CONFIG_BUILD_FLAT)
+  /* in FLAT build, allocate the object metadata and the data in the same
+   * chunk in kernel heap
+   */
+
+  object = kmm_zalloc(sizeof(struct shmfs_object_s) + length);
+  if (object)
+    {
+      object->paddr = (&object->paddr) + 1;
+      allocated = true;
+    }
+
+#elif defined(CONFIG_BUILD_PROTECTED)
+  /* in PROTECTED build, allocate the shm object in kernel heap, and shared
+   * memory in user heap
+   */
+
+  object = kmm_zalloc(sizeof(struct shmfs_object_s));
+  if (object)
+    {
+      object->paddr = kumm_zalloc(length);
+
+      if (object->paddr)
+        {
+           allocated = true;
+        }
+    }
+
+#elif defined(CONFIG_BUILD_KERNEL)
+  /* in KERNEL build, allocate the shared memory from page pool and store the
+   * physical address
+   */
+
+  size_t i = 0;
+  void **pages;

Review Comment:
   FAR



##########
fs/shm/shmfs.c:
##########
@@ -0,0 +1,380 @@
+/****************************************************************************
+ * fs/shm/shmfs.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <assert.h>
+
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/mm/map.h>
+
+#if defined (CONFIG_BUILD_KERNEL)
+#include <nuttx/arch.h>
+#include <nuttx/pgalloc.h>
+#include <nuttx/sched.h>
+#endif
+
+#include "shm/shmfs.h"
+#include "inode/inode.h"
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static int shmfs_close(FAR struct file *filep);
+static ssize_t shmfs_read(FAR struct file *filep, FAR char *buffer,
+                          size_t buflen);
+static ssize_t shmfs_write(FAR struct file *filep, FAR const char *buffer,
+                           size_t buflen);
+static int shmfs_truncate(FAR struct file *filep, off_t length);
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+static int shmfs_unlink(FAR struct inode *inode);
+#endif
+
+static int shmfs_mmap(FAR struct file *filep,
+                      FAR struct mm_map_entry_s *entry);
+static int shmfs_munmap(FAR struct task_group_s *group,
+                        FAR struct mm_map_entry_s *entry,
+                        FAR void *start,
+                        size_t length);
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+const struct file_operations shmfs_operations =
+{
+  NULL,             /* open */
+  shmfs_close,      /* close */
+  shmfs_read,       /* read */
+  shmfs_write,      /* write */
+  NULL,             /* seek */
+  NULL,             /* ioctl */
+  shmfs_mmap,       /* mmap */
+  shmfs_truncate,   /* truncate */
+  NULL,             /* poll */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  shmfs_unlink      /* unlink */
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: shmfs_read
+ ****************************************************************************/
+
+static ssize_t shmfs_read(FAR struct file *filep, FAR char *buffer,
+                          size_t buflen)
+{
+  return -ENOSYS;
+}
+
+/****************************************************************************
+ * Name: shmfs_write
+ ****************************************************************************/
+
+static ssize_t shmfs_write(FAR struct file *filep, FAR const char *buffer,
+                           size_t buflen)
+{
+  return -ENOSYS;
+}
+
+/****************************************************************************
+ * Name: shmfs_release
+ ****************************************************************************/
+
+static int shmfs_release(FAR struct inode *inode)
+{
+  /* If the file has been unlinked previously, delete the contents.
+   * The inode is released after this call, hence checking if i_crefs <= 1.
+   */
+
+  int ret = inode_lock();
+  if (ret >= 0)
+    {
+      if (inode->i_parent == NULL &&

Review Comment:
   why check i_parent 



##########
fs/shm/shmfs.c:
##########
@@ -0,0 +1,384 @@
+/****************************************************************************
+ * fs/shm/shmfs.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <assert.h>
+
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/mm/map.h>
+
+#if defined (CONFIG_BUILD_KERNEL)
+#include <nuttx/arch.h>
+#include <nuttx/pgalloc.h>
+#include <nuttx/sched.h>
+#endif
+
+#include "shm/shmfs.h"
+#include "inode/inode.h"
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static int shmfs_close(FAR struct file *filep);
+static ssize_t shmfs_read(FAR struct file *filep, FAR char *buffer,
+                          size_t buflen);
+static ssize_t shmfs_write(FAR struct file *filep, FAR const char *buffer,
+                           size_t buflen);
+static int shmfs_truncate(FAR struct file *filep, off_t length);
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+static int shmfs_unlink(FAR struct inode *inode);
+#endif
+
+static int shmfs_mmap(FAR struct file *filep,
+                      FAR struct mm_map_entry_s *entry);
+static int shmfs_munmap(FAR struct task_group_s *group,
+                        FAR struct mm_map_entry_s *entry,
+                        FAR void *start,
+                        size_t length);
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+const struct file_operations shmfs_operations =
+{
+  NULL,             /* open */
+  shmfs_close,      /* close */
+  shmfs_read,       /* read */
+  shmfs_write,      /* write */
+  NULL,             /* seek */
+  NULL,             /* ioctl */
+  shmfs_mmap,       /* mmap */
+  shmfs_truncate,   /* truncate */
+  NULL,             /* poll */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  shmfs_unlink      /* unlink */
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: shmfs_read
+ ****************************************************************************/
+
+static ssize_t shmfs_read(FAR struct file *filep, FAR char *buffer,
+                          size_t buflen)
+{
+  return -ENOSYS;
+}
+
+/****************************************************************************
+ * Name: shmfs_write
+ ****************************************************************************/
+
+static ssize_t shmfs_write(FAR struct file *filep, FAR const char *buffer,
+                           size_t buflen)
+{
+  return -ENOSYS;
+}
+
+/****************************************************************************
+ * Name: shmfs_release
+ ****************************************************************************/
+
+static int shmfs_release(FAR struct inode *inode)
+{
+  /* If the file has been unlinked previously, delete the contents.
+   * The inode is released after this call, hence checking if i_crefs <= 1.
+   */
+
+  int ret = inode_lock();
+  if (ret >= 0)
+    {
+      if (inode->i_parent == NULL &&
+          inode->i_crefs <= 1)
+        {
+          delete_shm_object(inode->i_private);
+          inode->i_private = NULL;
+        }
+
+      inode_unlock();
+    }
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: shmfs_close
+ ****************************************************************************/
+
+static int shmfs_close(FAR struct file *filep)
+{
+  /* Release the shmfs object. The object gets deleted if no-one has
+   * reference to it (either mmap or open file) and the object has been
+   * unlinked
+   */
+
+  return shmfs_release(filep->f_inode);
+}
+
+/****************************************************************************
+ * Name: shmfs_truncate
+ ****************************************************************************/
+
+static int shmfs_truncate(FAR struct file *filep, off_t length)
+{
+  FAR struct shmfs_object_s *object;
+  int ret;
+
+  if (length == 0)
+    {
+      return -EINVAL;
+    }
+
+  ret = inode_lock();
+  if (ret >= 0)
+    {
+      object = filep->f_inode->i_private;
+      if (!object)
+        {
+          filep->f_inode->i_private = alloc_shm_object(length);
+        }
+      else if (object->length != length)
+        {
+          /* This doesn't support resize */
+
+          ret = -EINVAL;
+        }
+
+      inode_unlock();
+    }
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: shmfs_unlink
+ ****************************************************************************/
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+static int shmfs_unlink(FAR struct inode *inode)
+{
+  if (inode->i_crefs <= 1)
+    {
+      delete_shm_object(inode->i_private);
+      inode->i_private = NULL;
+    }
+
+  return OK;
+}
+#endif
+
+/****************************************************************************
+ * Name: shmfs_map_object
+ ****************************************************************************/
+
+static int shmfs_map_object(FAR struct shmfs_object_s *object,
+                            FAR void **vaddr)
+{
+  int ret = OK;
+
+#ifdef CONFIG_BUILD_KERNEL
+  /* Map the physical pages of the shm object with MMU. */
+
+  FAR struct tcb_s        *tcb   = nxsched_self();
+  FAR struct task_group_s *group = tcb->group;
+  FAR uintptr_t *pages =
+    (FAR uintptr_t *)&object->paddr;
+  uintptr_t                mapaddr;
+  unsigned int             npages;
+
+  /* Find a free vaddr space that satisfies length */
+
+  mapaddr = (uintptr_t)vm_alloc_region(get_group_mm(group), 0,
+                                       object->length);
+  if (mapaddr == 0)
+    {
+      return -ENOMEM;
+    }
+
+  /* Convert the region size to pages */
+
+  npages = MM_NPAGES(object->length);
+
+  /* Map the memory to user virtual address space */
+
+  ret = up_shmat(pages, npages, mapaddr);
+  if (ret < 0)
+    {
+      vm_release_region(get_group_mm(group), (FAR void *)mapaddr,
+                        object->length);
+    }
+  else
+    {
+      *vaddr = (FAR void *)mapaddr;
+    }
+#else
+  /* Use the physical address directly */
+
+  *vaddr = object->paddr;
+#endif
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: shmfs_mmap
+ ****************************************************************************/
+
+static int shmfs_mmap(FAR struct file *filep,
+                      FAR struct mm_map_entry_s *entry)
+{
+  FAR struct shmfs_object_s *object;
+  int ret = -EINVAL;
+
+  DEBUGASSERT(filep->f_inode != NULL);
+
+  /* We don't support offset at the moment, just mapping the whole object */
+
+  if (entry->offset != 0)
+    {
+      return ret;
+    }
+
+  /* Keep the inode when mmapped, increase refcount */
+
+  ret = inode_addref(filep->f_inode);
+  if (ret >= 0)
+    {
+      object = (FAR struct shmfs_object_s *)filep->f_inode->i_private;
+
+      if (object)

Review Comment:
   why? filep already hold one reference for you



##########
fs/shm/shm_unlink.c:
##########
@@ -0,0 +1,167 @@
+/****************************************************************************
+ * fs/shm/shm_unlink.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include "shm/shmfs.h"
+#include "inode/inode.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static int file_shm_unlink(FAR const char *name)
+{
+  FAR struct inode *inode;
+  struct inode_search_s desc;
+  char fullpath[sizeof(CONFIG_FS_SHMFS_VFS_PATH) + CONFIG_NAME_MAX + 2];
+  int ret;
+
+  /* Make sure that a non-NULL name is supplied */
+
+  if (!name)
+    {
+      return -ENOENT;
+    }
+
+  /* Remove any number of leading '/' */
+
+  while (*name == '/')
+    {
+      name++;
+    }
+
+  /* Empty name supplied? */
+
+  if (*name == '\0')
+    {
+      return -ENOENT;
+    }
+
+  /* Name too long? */
+
+  if (strnlen(name, CONFIG_NAME_MAX) > CONFIG_NAME_MAX)
+    {
+      return -ENAMETOOLONG;
+    }
+
+  /* Get the full path to the shm object */
+
+  snprintf(fullpath, sizeof(fullpath),
+           CONFIG_FS_SHMFS_VFS_PATH "/%s", name);
+
+  /* Get the inode for this shm object */
+
+  SETUP_SEARCH(&desc, fullpath, false);
+
+  ret = inode_lock();
+  if (ret < 0)
+    {
+      goto errout_with_search;
+    }
+
+  ret = inode_find(&desc);
+  if (ret < 0)
+    {
+      /* There is no inode that includes in this path */
+
+      goto errout_with_sem;
+    }
+
+  /* Get the search results */
+
+  inode = desc.node;
+  DEBUGASSERT(inode != NULL);
+
+  /* Verify that what we found is, indeed, an shm inode */
+
+  if (!INODE_IS_SHM(inode))
+    {
+      ret = -ENOENT;
+      inode_release(inode);

Review Comment:
   remove and goto errout_with_inode



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to