You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/06/14 07:17:21 UTC
[GitHub] [incubator-nuttx] DS-LK commented on a change in pull request #3901: nuttx: Initialize chmod\fchmod\chown\fchown\lchown
DS-LK commented on a change in pull request #3901:
URL: https://github.com/apache/incubator-nuttx/pull/3901#discussion_r650022933
##########
File path: fs/vfs/fs_chmod.c
##########
@@ -0,0 +1,103 @@
+/****************************************************************************
+ * fs/vfs/fs_chmod.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 <sys/types.h>
+#include <sys/stat.h>
+
+#include <errno.h>
+#include <assert.h>
+
+#include <nuttx/fs/fs.h>
+
+#include "inode/inode.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: chown
+ *
+ * Description:
+ * changes the mode of the file specified whose pathname is given in
+ * pathname, which is dereferenced if it is a symbolic link.
+ *
+ * NOTE: Due to the characteristics of fs, this function is incomplete.
+ *
+ * Input Parameters:
+ * fd : fd.
+ * mode : Permission value.
+ *
+ * Returned Value:
+ * Zero (OK) is returned on success; -1 is returned on fail, and errno is
+ * set appropriately.
+ *
+ ****************************************************************************/
+
+int chmod(FAR const char *path, mode_t mode)
+{
+ if (path == NULL)
+ {
+ return -EINVAL;
+ }
+
+#ifdef CONFIG_FILE_MODE
+ struct inode_search_s desc;
+ FAR struct inode *inode;
+ int ret;
+
+ /* Get an inode for this file */
+
+ SETUP_SEARCH(&desc, path, false);
+
+ ret = inode_find(&desc);
+ if (ret < 0)
+ {
+ /* "O_CREAT is not set and the named file does not exist. Or, a
+ * directory component in pathname does not exist or is a dangling
+ * symbolic link."
+ */
Review comment:
ok, thanks.
##########
File path: fs/vfs/fs_chmod.c
##########
@@ -0,0 +1,103 @@
+/****************************************************************************
+ * fs/vfs/fs_chmod.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 <sys/types.h>
+#include <sys/stat.h>
+
+#include <errno.h>
+#include <assert.h>
+
+#include <nuttx/fs/fs.h>
+
+#include "inode/inode.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: chown
+ *
+ * Description:
+ * changes the mode of the file specified whose pathname is given in
+ * pathname, which is dereferenced if it is a symbolic link.
+ *
+ * NOTE: Due to the characteristics of fs, this function is incomplete.
+ *
+ * Input Parameters:
+ * fd : fd.
+ * mode : Permission value.
+ *
+ * Returned Value:
+ * Zero (OK) is returned on success; -1 is returned on fail, and errno is
+ * set appropriately.
+ *
+ ****************************************************************************/
+
+int chmod(FAR const char *path, mode_t mode)
+{
+ if (path == NULL)
+ {
+ return -EINVAL;
+ }
+
+#ifdef CONFIG_FILE_MODE
+ struct inode_search_s desc;
+ FAR struct inode *inode;
+ int ret;
+
+ /* Get an inode for this file */
+
+ SETUP_SEARCH(&desc, path, false);
+
+ ret = inode_find(&desc);
+ if (ret < 0)
+ {
+ /* "O_CREAT is not set and the named file does not exist. Or, a
+ * directory component in pathname does not exist or is a dangling
+ * symbolic link."
+ */
+
+ RELEASE_SEARCH(&desc);
+ set_errno(ret);
+ return ERROR;
+ }
+
+ /* Get the search results */
+
+ inode = desc.node;
+ DEBUGASSERT(inode != NULL);
+
+ inode->i_mode = mode;
+
Review comment:
I don't quite understand what you mean. Do you mean that the permission of the file is read-only?
##########
File path: fs/vfs/fs_chown.c
##########
@@ -0,0 +1,105 @@
+/****************************************************************************
+ * fs/vfs/fs_chown.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 <sys/types.h>
+#include <sys/stat.h>
+
+#include <errno.h>
+#include <unistd.h>
+#include <pwd.h>
+#include <grp.h>
+#include <assert.h>
+
+#include <nuttx/fs/fs.h>
+
+#include "inode/inode.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: chown
+ *
+ * Description:
+ * changes the ownership of the file specified by pathname, which is
+ * dereferenced if it is a symbolic link.
+ *
+ * NOTE: Due to the characteristics of fs, this function is incomplete.s
+ *
+ * Input Parameters:
+ * pathname: path of file.
+ * owner : User id.
+ * group : Group id.
+ *
+ * Returned Value:
+ * Zero (OK) is returned on success; -1 is returned on fail, and errno is
+ * set appropriately.
+ *
+ ****************************************************************************/
+
+int chown(FAR const char *pathname, uid_t owner, gid_t group)
+{
+ struct inode_search_s desc;
+ FAR struct inode *inode;
+ int ret;
+
+ if (pathname == NULL || (owner <= -1 && group <= -1))
+ {
+ set_errno(-EINVAL);
+ return ERROR;
+ }
+ /* Get an inode for this file */
+
+ SETUP_SEARCH(&desc, pathname, false);
+
+ ret = inode_find(&desc);
+ if (ret < 0)
+ {
+ /* "O_CREAT is not set and the named file does not exist. Or, a
+ * directory component in pathname does not exist or is a dangling
+ * symbolic link."
+ */
+
+ RELEASE_SEARCH(&desc);
+ set_errno(ret);
+ return ERROR;
+ }
+
+ /* Get the search results */
+
+ inode = desc.node;
+ DEBUGASSERT(inode != NULL);
+
+ /* Set uid and gid */
+
+ inode->i_uid = owner > -1 ? owner : inode->i_uid;
+ inode->i_gid = group > -1 ? group : inode->i_gid;
+
Review comment:
Yes, what I wanted to do at the beginning was to verify the permissions of users and groups as you did, but this would make it easy for this function to be called and fail. As you said, there are still many things that need to be improved, so I only added such a simple setting at present. My idea is that the verification of the permissions of this function can be completed by improving the permissions of the whole system
##########
File path: include/nuttx/fs/fs.h
##########
@@ -352,6 +352,8 @@ struct inode
int16_t i_crefs; /* References to inode */
uint16_t i_flags; /* Flags for inode */
union inode_ops_u u; /* Inode operations */
+ uid_t i_uid; /* The ID of the inode owner */
+ gid_t i_gid; /* The ID of the inode owner group */
#ifdef CONFIG_FILE_MODE
Review comment:
You are right. These two variables are added to the header file. When creating inode, these two values are to be set as the user and group ID of the task. Therefore, more work needs to be done in terms of permissions
##########
File path: fs/vfs/fs_chmod.c
##########
@@ -0,0 +1,103 @@
+/****************************************************************************
+ * fs/vfs/fs_chmod.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 <sys/types.h>
+#include <sys/stat.h>
+
+#include <errno.h>
+#include <assert.h>
+
+#include <nuttx/fs/fs.h>
+
+#include "inode/inode.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: chown
+ *
+ * Description:
+ * changes the mode of the file specified whose pathname is given in
+ * pathname, which is dereferenced if it is a symbolic link.
+ *
+ * NOTE: Due to the characteristics of fs, this function is incomplete.
+ *
+ * Input Parameters:
+ * fd : fd.
+ * mode : Permission value.
+ *
+ * Returned Value:
+ * Zero (OK) is returned on success; -1 is returned on fail, and errno is
+ * set appropriately.
+ *
+ ****************************************************************************/
+
+int chmod(FAR const char *path, mode_t mode)
+{
+ if (path == NULL)
+ {
+ return -EINVAL;
+ }
+
+#ifdef CONFIG_FILE_MODE
+ struct inode_search_s desc;
+ FAR struct inode *inode;
+ int ret;
+
+ /* Get an inode for this file */
+
+ SETUP_SEARCH(&desc, path, false);
+
+ ret = inode_find(&desc);
+ if (ret < 0)
+ {
+ /* "O_CREAT is not set and the named file does not exist. Or, a
+ * directory component in pathname does not exist or is a dangling
+ * symbolic link."
+ */
+
+ RELEASE_SEARCH(&desc);
+ set_errno(ret);
+ return ERROR;
+ }
+
+ /* Get the search results */
+
+ inode = desc.node;
+ DEBUGASSERT(inode != NULL);
+
+ inode->i_mode = mode;
+
Review comment:
Thank you. You're right. I wrote a little too simple, I will seriously think about what you said.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org