xiaoxiang781216 commented on code in PR #19900:
URL: https://github.com/apache/nuttx/pull/19900#discussion_r3888292736
##########
fs/driver/fs_findmtddriver.c:
##########
@@ -70,7 +70,12 @@ int find_mtddriver(FAR const char *pathname, FAR struct
inode **ppinode)
/* Find the inode registered with this pathname */
- SETUP_SEARCH(&desc, pathname, false);
+ ret = inode_search_setup(&desc, pathname, false);
+ if (ret < 0)
+ {
+ inode_search_release(&desc);
Review Comment:
ditto
##########
fs/driver/fs_finddriver.c:
##########
@@ -51,12 +51,17 @@ FAR void *find_driver(FAR const char *pathname)
{
struct inode_search_s desc;
FAR void *drvr = NULL;
+ int ret;
DEBUGASSERT(pathname != NULL);
/* Find the inode registered with this pathname */
- SETUP_SEARCH(&desc, pathname, false);
+ ret = inode_search_setup(&desc, pathname, false);
Review Comment:
`if (inode_search_setup(&desc, pathname, false) < 0)`
##########
fs/vfs/fs_rmdir.c:
##########
@@ -70,7 +70,12 @@ int rmdir(FAR const char *pathname)
* on the inode if one is found.
*/
- SETUP_SEARCH(&desc, pathname, true);
+ ret = inode_search_setup(&desc, pathname, true);
+ if (ret < 0)
+ {
+ errcode = -ret;
+ goto errout_with_search;
Review Comment:
add errout label
##########
fs/fat/fs_fat32attrib.c:
##########
@@ -59,7 +59,12 @@ static int fat_attrib(const char *path, fat_attrib_t
*retattrib,
/* Find the inode for this file */
- SETUP_SEARCH(&desc, path, false);
+ ret = inode_search_setup(&desc, path, false);
+ if (ret < 0)
+ {
+ inode_search_release(&desc);
Review Comment:
ditto
##########
fs/vfs/fs_rename.c:
##########
Review Comment:
errout_with_newsearch
##########
fs/vfs/fs_readlink.c:
##########
@@ -80,7 +80,12 @@ ssize_t readlink(FAR const char *path, FAR char *buf, size_t
bufsize)
* symbolic link node.
*/
- SETUP_SEARCH(&desc, path, true);
+ ret = inode_search_setup(&desc, path, true);
+ if (ret < 0)
+ {
+ errcode = -ret;
+ goto errout_with_search;
Review Comment:
goto errout
##########
fs/event/event_open.c:
##########
@@ -102,7 +102,12 @@ int nxevent_open(FAR nxevent_t **event, FAR const char
*name,
* will have incremented the reference count on the inode.
*/
- SETUP_SEARCH(&desc, fullpath, false);
+ ret = inode_search_setup(&desc, fullpath, false);
+ if (ret < 0)
+ {
+ inode_search_release(&desc);
Review Comment:
ditto
##########
fs/semaphore/sem_unlink.c:
##########
@@ -78,7 +78,12 @@ int nxsem_unlink(FAR const char *name)
/* Get the inode for this semaphore. */
- SETUP_SEARCH(&desc, fullpath, false);
+ ret = inode_search_setup(&desc, fullpath, false);
+ if (ret < 0)
+ {
+ inode_search_release(&desc);
Review Comment:
remove
##########
fs/unionfs/fs_unionfs.c:
##########
@@ -2538,7 +2538,12 @@ static int unionfs_getmount(FAR const char *path, FAR
struct inode **inode)
/* Find the mountpt */
- SETUP_SEARCH(&desc, path, false);
+ ret = inode_search_setup(&desc, path, false);
+ if (ret < 0)
+ {
+ inode_search_release(&desc);
Review Comment:
remove
##########
fs/vfs/fs_readlink.c:
##########
@@ -80,7 +80,12 @@ ssize_t readlink(FAR const char *path, FAR char *buf, size_t
bufsize)
* symbolic link node.
*/
- SETUP_SEARCH(&desc, path, true);
+ ret = inode_search_setup(&desc, path, true);
+ if (ret < 0)
+ {
+ errcode = -ret;
Review Comment:
let's remove errcode and set_errno(-ret) directly
##########
fs/mount/fs_umount2.c:
##########
@@ -72,7 +72,12 @@ int nx_umount2(FAR const char *target, unsigned int flags)
/* Find the mountpt */
- SETUP_SEARCH(&desc, target, false);
+ ret = inode_search_setup(&desc, target, false);
+ if (ret < 0)
+ {
+ inode_search_release(&desc);
Review Comment:
remove
##########
fs/mount/fs_automount.c:
##########
@@ -470,56 +476,56 @@ static void automount_mount(FAR struct
automounter_state_s *priv)
ret = automount_findinode(lower->mountpoint);
switch (ret)
{
- case OK_EXIST:
+ case OK_EXIST:
Review Comment:
let's move the format to the last patch
##########
fs/inode/inode.h:
##########
Review Comment:
remove
##########
fs/vfs/fs_chroot.c:
##########
@@ -0,0 +1,131 @@
+/****************************************************************************
+ * fs/vfs/fs_chroot.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * 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 <sys/stat.h>
+#include <assert.h>
+#include <errno.h>
+#include <limits.h>
+#include <string.h>
+
+#include <nuttx/fs/fs.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/sched.h>
+
+#include "inode/inode.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: chroot
+ *
+ * Description:
+ * Cause the named directory to become the root directory, that is, the
+ * starting point for path names beginning with '/'.
+ *
+ * Input Parameters:
+ * path - Directory to use as the new root
+ *
+ * Returned Value:
+ * 0(OK) on success; -1(ERROR) on failure with errno set appropriately.
+ *
+ ****************************************************************************/
+
+int chroot(FAR const char *path)
+{
+ FAR struct tcb_s *rtcb;
+ FAR struct task_group_s *group;
+ FAR char *newroot;
+ char abspath[PATH_MAX];
+ struct stat buf;
+ int ret;
+
+ if (path == NULL || path[0] == '\0')
+ {
+ set_errno(ENOENT);
+ return ERROR;
+ }
+
+ rtcb = nxsched_self();
+ DEBUGASSERT(rtcb != NULL && rtcb->group != NULL);
+ group = rtcb->group;
+
+#ifdef CONFIG_SCHED_USER_IDENTITY
+ if (group->tg_euid != 0)
+ {
+ set_errno(EPERM);
+ return ERROR;
+ }
+#endif
+
+ ret = nx_stat(path, &buf, 1);
+ if (ret < 0)
+ {
+ set_errno(-ret);
+ return ERROR;
+ }
+
+ if (!S_ISDIR(buf.st_mode))
+ {
+ set_errno(ENOTDIR);
+ return ERROR;
+ }
+
+ /* Resolve to a host absolute path the same way lookups do: make
+ * absolute, prepend the current jail, and normalize. No second
+ * inode walk.
+ */
+
+ ret = inode_chroot_hostpath(path, abspath, sizeof(abspath));
Review Comment:
move after `fs: start absolute lookups at the jail root`
##########
fs/vfs/fs_rmdir.c:
##########
@@ -70,7 +70,12 @@ int rmdir(FAR const char *pathname)
* on the inode if one is found.
*/
- SETUP_SEARCH(&desc, pathname, true);
+ ret = inode_search_setup(&desc, pathname, true);
+ if (ret < 0)
+ {
+ errcode = -ret;
Review Comment:
let's remove errcode and call set_errno(-ret) instead
##########
fs/driver/fs_findblockdriver.c:
##########
@@ -77,7 +77,12 @@ int find_blockdriver(FAR const char *pathname, int
mountflags,
/* Find the inode registered with this pathname */
- SETUP_SEARCH(&desc, pathname, false);
+ ret = inode_search_setup(&desc, pathname, false);
+ if (ret < 0)
+ {
+ inode_search_release(&desc);
Review Comment:
let's clean up in inode_search_setup if fail
##########
fs/inode/fs_inodereserve.c:
##########
@@ -207,7 +208,12 @@ int inode_reserve(FAR const char *path,
/* Find the location to insert the new subtree */
- SETUP_SEARCH(&desc, path, false);
+ ret = inode_search_setup(&desc, path, false);
+ if (ret < 0)
+ {
+ inode_search_release(&desc);
Review Comment:
ditto
##########
fs/inode/fs_inoderemove.c:
##########
@@ -71,7 +71,12 @@ static FAR struct inode *inode_unlink(FAR const char *path)
/* Find the node to unlink */
- SETUP_SEARCH(&desc, path, true);
+ ret = inode_search_setup(&desc, path, true);
+ if (ret < 0)
+ {
+ inode_search_release(&desc);
Review Comment:
ditto
##########
fs/vfs/fs_unlink.c:
##########
@@ -68,7 +68,11 @@ int nx_unlink(FAR const char *pathname)
* which may be a symbolic link)
*/
- SETUP_SEARCH(&desc, pathname, true);
+ ret = inode_search_setup(&desc, pathname, true);
+ if (ret < 0)
+ {
+ goto errout_with_search;
Review Comment:
return ret
##########
fs/vfs/fs_statfs.c:
##########
@@ -93,7 +93,12 @@ int statfs(FAR const char *path, FAR struct statfs *buf)
/* Get an inode for this file */
- SETUP_SEARCH(&desc, path, false);
+ ret = inode_search_setup(&desc, path, false);
+ if (ret < 0)
+ {
+ inode_search_release(&desc);
Review Comment:
remove and goto errout
##########
fs/vfs/fs_stat.c:
##########
@@ -88,7 +88,12 @@ static int stat_recursive(FAR const char *path,
/* Get an inode for this path */
- SETUP_SEARCH(&desc, path, true);
+ ret = inode_search_setup(&desc, path, true);
+ if (ret < 0)
+ {
+ inode_search_release(&desc);
Review Comment:
remove
##########
fs/vfs/fs_rename.c:
##########
@@ -553,7 +568,12 @@ int rename(FAR const char *oldpath, FAR const char
*newpath)
/* Get an inode that includes the oldpath */
- SETUP_SEARCH(&olddesc, oldpath, true);
+ ret = inode_search_setup(&olddesc, oldpath, true);
+ if (ret < 0)
+ {
+ goto errout_with_oldsearch;
Review Comment:
goto errout
##########
fs/vfs/fs_rename.c:
##########
@@ -315,7 +325,12 @@ static int mountptrename(FAR const char *oldpath, FAR
struct inode *oldinode,
* mountpoint
*/
- SETUP_SEARCH(&newdesc, newpath, true);
+ ret = inode_search_setup(&newdesc, newpath, true);
+ if (ret < 0)
+ {
+ goto errout_with_newsearch;
Review Comment:
return ret
##########
fs/vfs/fs_rename.c:
##########
Review Comment:
goto errout_with_newsearch
##########
fs/vfs/fs_rename.c:
##########
Review Comment:
errout_with_newsearch
##########
fs/vfs/fs_rename.c:
##########
Review Comment:
errout_with_newsearch
##########
fs/vfs/fs_rename.c:
##########
Review Comment:
errout_with_newsearch
##########
fs/vfs/fs_rename.c:
##########
Review Comment:
errout_with_newsearch
##########
fs/vfs/fs_rename.c:
##########
@@ -196,9 +200,15 @@ static int pseudorename(FAR const char *oldpath, FAR
struct inode *oldinode,
/* Re-resolve the source under the same lock before unlinking it. */
- SETUP_SEARCH(&olddesc, oldpath, true);
+ ret = inode_search_setup(&olddesc, oldpath, true);
+ if (ret < 0)
+ {
+ inode_search_release(&olddesc);
Review Comment:
remove and goto errout_with_newsearch
##########
fs/vfs/fs_link.c:
##########
@@ -95,7 +95,13 @@ int link(FAR const char *path1, FAR const char *path2)
goto errout;
}
- SETUP_SEARCH(&desc_path1, path1, false);
+ ret = inode_search_setup(&desc_path1, path1, false);
+ if (ret < 0)
+ {
+ errcode = -ret;
Review Comment:
remove errcode too
##########
fs/vfs/fs_mkdir.c:
##########
@@ -71,7 +71,12 @@ int mkdir(const char *pathname, mode_t mode)
/* Find the inode that includes this path */
- SETUP_SEARCH(&desc, pathname, false);
+ ret = inode_search_setup(&desc, pathname, false);
+ if (ret < 0)
+ {
+ errcode = -ret;
Review Comment:
remove errcode too
##########
fs/vfs/fs_open.c:
##########
@@ -100,7 +100,12 @@ static int file_vopen(FAR struct file *filep, FAR const
char *path,
/* Get an inode for this file */
- SETUP_SEARCH(&desc, path, (oflags & O_NOFOLLOW) != 0);
+ ret = inode_search_setup(&desc, path, (oflags & O_NOFOLLOW) != 0);
+ if (ret < 0)
+ {
+ inode_search_release(&desc);
Review Comment:
remove
##########
fs/vfs/fs_mkdir.c:
##########
@@ -71,7 +71,12 @@ int mkdir(const char *pathname, mode_t mode)
/* Find the inode that includes this path */
- SETUP_SEARCH(&desc, pathname, false);
+ ret = inode_search_setup(&desc, pathname, false);
+ if (ret < 0)
+ {
+ errcode = -ret;
+ goto errout_with_search;
Review Comment:
goto errout
##########
fs/vfs/fs_chstat.c:
##########
@@ -54,7 +54,12 @@ static int chstat_recursive(FAR const char *path,
/* Get an inode for this path */
- SETUP_SEARCH(&desc, path, true);
+ ret = inode_search_setup(&desc, path, true);
+ if (ret < 0)
+ {
+ inode_search_release(&desc);
Review Comment:
remove
##########
fs/mqueue/mq_unlink.c:
##########
@@ -113,7 +113,12 @@ int file_mq_unlink(FAR const char *mq_name)
/* Get the inode for this message queue. */
- SETUP_SEARCH(&desc, fullpath, false);
+ ret = inode_search_setup(&desc, fullpath, false);
+ if (ret < 0)
+ {
+ inode_search_release(&desc);
Review Comment:
remove
##########
fs/semaphore/sem_open.c:
##########
@@ -111,7 +111,12 @@ int nxsem_open(FAR sem_t **sem, FAR const char *name, int
oflags, ...)
* will have incremented the reference count on the inode.
*/
- SETUP_SEARCH(&desc, fullpath, false);
+ ret = inode_search_setup(&desc, fullpath, false);
+ if (ret < 0)
+ {
+ inode_search_release(&desc);
Review Comment:
remove
##########
fs/shm/shm_open.c:
##########
@@ -165,7 +169,7 @@ static int file_shm_open(FAR struct file *shm, FAR const
char *name,
errout_with_sem:
Review Comment:
errout_with_lock
##########
fs/shm/shm_unlink.c:
##########
@@ -136,7 +140,7 @@ static int file_shm_unlink(FAR const char *name)
inode_release(inode);
errout_with_sem:
Review Comment:
errout_with_sem->errout_with_lock
##########
fs/mqueue/mq_open.c:
##########
Review Comment:
remove errout and return directly
##########
fs/mount/fs_mount.c:
##########
@@ -380,7 +380,12 @@ int nx_mount(FAR const char *source, FAR const char
*target,
#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
/* Check if the inode already exists */
- SETUP_SEARCH(&desc, target, false);
+ ret = inode_search_setup(&desc, target, false);
+ if (ret < 0)
+ {
+ inode_search_release(&desc);
Review Comment:
remove
##########
fs/mqueue/mq_open.c:
##########
@@ -235,7 +235,13 @@ static int file_mq_vopen(FAR struct file *mq, FAR const
char *mq_name,
* have incremented the reference count on the inode.
*/
- SETUP_SEARCH(&desc, fullpath, false);
+ ret = inode_search_setup(&desc, fullpath, false);
+ if (ret < 0)
+ {
+ inode_search_release(&desc);
Review Comment:
remove
##########
fs/inode/inode.h:
##########
Review Comment:
let's change to:
`int inode_find(FAR struct inode_search_s *desc, FAR struct inode **inode);`
and remove node from inode_search_s
##########
fs/mount/fs_automount.c:
##########
@@ -409,7 +409,13 @@ static int automount_findinode(FAR const char *path)
/* Find the inode */
- SETUP_SEARCH(&desc, path, false);
+ ret = inode_search_setup(&desc, path, false);
+ if (ret < 0)
+ {
+ inode_runlock();
+ inode_search_release(&desc);
Review Comment:
remove
##########
fs/inode/fs_inodesearch.c:
##########
Review Comment:
let's merge _inode_search here and remove _inode_search
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
