You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by ag...@apache.org on 2021/01/11 22:48:07 UTC

[incubator-nuttx] branch master updated (7a953bb -> 1604fe0)

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

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


    from 7a953bb  xtensa/esp32: Fix ESP32 SPI3 slave ops data error
     new 0032ddb  fs: Reimplement file_open to not depend on nx_open
     new 1604fe0  fs: Remove file_detach since it is unefficient to call open and file_detach

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 fs/driver/driver.h        |   2 +-
 fs/driver/fs_blockproxy.c |  14 ++---
 fs/inode/Make.defs        |   1 -
 fs/inode/fs_filedetach.c  | 147 ----------------------------------------------
 fs/inode/fs_fileopen.c    | 110 ----------------------------------
 fs/vfs/fs_close.c         |   5 +-
 fs/vfs/fs_open.c          | 132 +++++++++++++++++++++++++++++------------
 include/nuttx/fs/fs.h     |  33 ++---------
 8 files changed, 106 insertions(+), 338 deletions(-)
 delete mode 100644 fs/inode/fs_filedetach.c
 delete mode 100644 fs/inode/fs_fileopen.c


[incubator-nuttx] 01/02: fs: Reimplement file_open to not depend on nx_open

Posted by ag...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 0032ddb8bf59a64005d2cb216e1d6f8949a1c035
Author: Xiang Xiao <xi...@xiaomi.com>
AuthorDate: Thu Oct 1 19:21:36 2020 +0800

    fs: Reimplement file_open to not depend on nx_open
    
    on the other hand, open/nx_open call file_open instead
    
    Signed-off-by: Xiang Xiao <xi...@xiaomi.com>
    Change-Id: I66990a77cdeb6ff18f7bf48a65bbc7b701dad552
---
 fs/driver/driver.h        |   2 +-
 fs/driver/fs_blockproxy.c |  14 +++--
 fs/inode/Make.defs        |   2 +-
 fs/inode/fs_fileopen.c    | 110 --------------------------------------
 fs/vfs/fs_open.c          | 132 ++++++++++++++++++++++++++++++++--------------
 include/nuttx/fs/fs.h     |   2 +
 6 files changed, 103 insertions(+), 159 deletions(-)

diff --git a/fs/driver/driver.h b/fs/driver/driver.h
index c7202bc..04a015f 100644
--- a/fs/driver/driver.h
+++ b/fs/driver/driver.h
@@ -119,7 +119,7 @@ int find_blockdriver(FAR const char *pathname, int mountflags,
  ****************************************************************************/
 
 #if !defined(CONFIG_DISABLE_MOUNTPOINT)
-int block_proxy(FAR const char *blkdev, int oflags);
+int block_proxy(FAR struct file *filep, FAR const char *blkdev, int oflags);
 #endif
 
 /****************************************************************************
diff --git a/fs/driver/fs_blockproxy.c b/fs/driver/fs_blockproxy.c
index edfb8a0..a17c7a2 100644
--- a/fs/driver/fs_blockproxy.c
+++ b/fs/driver/fs_blockproxy.c
@@ -105,10 +105,10 @@ static FAR char *unique_chardev(void)
 
       /* Make sure that file name is not in use */
 
-      ret = stat(devbuf, &statbuf);
+      ret = nx_stat(devbuf, &statbuf, 1);
       if (ret < 0)
         {
-          DEBUGASSERT(errno == ENOENT);
+          DEBUGASSERT(ret == -ENOENT);
           return strdup(devbuf);
         }
 
@@ -147,12 +147,11 @@ static FAR char *unique_chardev(void)
  *
  ****************************************************************************/
 
-int block_proxy(FAR const char *blkdev, int oflags)
+int block_proxy(FAR struct file *filep, FAR const char *blkdev, int oflags)
 {
   FAR char *chardev;
   bool readonly;
   int ret;
-  int fd;
 
   DEBUGASSERT(blkdev);
 
@@ -183,10 +182,9 @@ int block_proxy(FAR const char *blkdev, int oflags)
   /* Open the newly created character driver */
 
   oflags &= ~(O_CREAT | O_EXCL | O_APPEND | O_TRUNC);
-  fd = nx_open(chardev, oflags);
-  if (fd < 0)
+  ret = file_open(filep, chardev, oflags);
+  if (ret < 0)
     {
-      ret = fd;
       ferr("ERROR: Failed to open %s: %d\n", chardev, ret);
       goto errout_with_bchdev;
     }
@@ -209,7 +207,7 @@ int block_proxy(FAR const char *blkdev, int oflags)
    */
 
   kmm_free(chardev);
-  return fd;
+  return OK;
 
 errout_with_bchdev:
   unlink(chardev);
diff --git a/fs/inode/Make.defs b/fs/inode/Make.defs
index 7f00fd3..d019c5c 100644
--- a/fs/inode/Make.defs
+++ b/fs/inode/Make.defs
@@ -36,7 +36,7 @@
 CSRCS += fs_files.c fs_foreachinode.c fs_inode.c fs_inodeaddref.c
 CSRCS += fs_inodebasename.c fs_inodefind.c fs_inodefree.c fs_inoderelease.c
 CSRCS += fs_inoderemove.c fs_inodereserve.c fs_inodesearch.c
-CSRCS += fs_fileopen.c fs_filedetach.c
+CSRCS += fs_filedetach.c
 
 # Include inode/utils build support
 
diff --git a/fs/inode/fs_fileopen.c b/fs/inode/fs_fileopen.c
deleted file mode 100644
index 90a13c1..0000000
--- a/fs/inode/fs_fileopen.c
+++ /dev/null
@@ -1,110 +0,0 @@
-/****************************************************************************
- * fs/inode/fs_fileopen.c
- *
- *   Copyright (C) 2018-2019 Gregory Nutt. All rights reserved.
- *   Author: Gregory Nutt <gn...@nuttx.org>
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- *
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in
- *    the documentation and/or other materials provided with the
- *    distribution.
- * 3. Neither the name NuttX nor the names of its contributors may be
- *    used to endorse or promote products derived from this software
- *    without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
- * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
- * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
- * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
- * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
- * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
- * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
- * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- * POSSIBILITY OF SUCH DAMAGE.
- *
- ****************************************************************************/
-
-/****************************************************************************
- * Included Files
- ****************************************************************************/
-
-#include <nuttx/config.h>
-
-#include <sys/types.h>
-#include <fcntl.h>
-#include <stdarg.h>
-
-#include <nuttx/fs/fs.h>
-
-#include "inode/inode.h"
-
-/****************************************************************************
- * Public Functions
- ****************************************************************************/
-
-/****************************************************************************
- * Name: file_open
- *
- * Description:
- *   file_open() is similar to the standard 'open' interface except that it
- *   returns an instance of 'struct file' rather than a file descriptor.  It
- *   also is not a cancellation point and does not modify the errno variable.
- *
- * Input Parameters:
- *   filep  - The caller provided location in which to return the 'struct
- *            file' instance.
- *   path   - The full path to the file to be open.
- *   oflags - open flags
- *   ...    - Variable number of arguments, may include 'mode_t mode'
- *
- * Returned Value:
- *   Zero (OK) is returned on success.  On failure, a negated errno value is
- *   returned.
- *
- ****************************************************************************/
-
-int file_open(FAR struct file *filep, FAR const char *path, int oflags, ...)
-{
-  va_list ap;
-  int ret;
-  int fd;
-
-  DEBUGASSERT(filep != NULL && path != NULL);
-
-  /* At present, this is just a placeholder.  It is just a wrapper around
-   * nx_open() followed by a called to file_detach().  Ideally, this should
-   * a native open function that opens the VFS node directly without using
-   * any file descriptors.
-   */
-
-  va_start(ap, oflags);
-  fd = nx_vopen(path, oflags, ap);
-  va_end(ap);
-
-  if (fd < 0)
-    {
-      return fd;
-    }
-
-  /* Detach the file structure from the file descriptor so that it can be
-   * used on any thread.
-   */
-
-  ret = file_detach(fd, filep);
-  if (ret < 0)
-    {
-      nx_close(fd);
-      return ret;
-    }
-
-  return OK;
-}
diff --git a/fs/vfs/fs_open.c b/fs/vfs/fs_open.c
index 1801177..6f599c4 100644
--- a/fs/vfs/fs_open.c
+++ b/fs/vfs/fs_open.c
@@ -80,31 +80,30 @@ int inode_checkflags(FAR struct inode *inode, int oflags)
 }
 
 /****************************************************************************
- * Name: nx_vopen
+ * Name: file_vopen
  *
  * Description:
- *   nx_vopen() is identical to 'nx_open' except that it accepts a va_list
+ *   file_vopen() is identical to 'file_open' except that it accepts va_list
  *   as an argument versus taking a variable length list of arguments.
  *
- *   nx_vopen() is an internal NuttX interface and should not be called from
- *   applications.
+ *   file_vopen() is an internal NuttX interface and should not be called
+ *   from applications.
  *
  * Returned Value:
- *   The new file descriptor is returned on success; a negated errno value is
- *   returned on any failure.
+ *   Zero (OK) is returned on success.  On failure, a negated errno value is
+ *   returned.
  *
  ****************************************************************************/
 
-int nx_vopen(FAR const char *path, int oflags, va_list ap)
+int file_vopen(FAR struct file *filep,
+               FAR const char *path, int oflags, va_list ap)
 {
   struct inode_search_s desc;
-  FAR struct file *filep;
   FAR struct inode *inode;
 #if defined(CONFIG_FILE_MODE) || !defined(CONFIG_DISABLE_MOUNTPOINT)
   mode_t mode = 0666;
 #endif
   int ret;
-  int fd;
 
   if (path == NULL)
     {
@@ -160,20 +159,11 @@ int nx_vopen(FAR const char *path, int oflags, va_list ap)
       /* Release the inode reference */
 
       inode_release(inode);
+      RELEASE_SEARCH(&desc);
 
       /* Get the file descriptor of the opened character driver proxy */
 
-      fd = block_proxy(path, oflags);
-      if (fd < 0)
-        {
-          ret = fd;
-          goto errout_with_search;
-        }
-
-      /* Return the file descriptor */
-
-      RELEASE_SEARCH(&desc);
-      return fd;
+      return block_proxy(filep, path, oflags);
     }
   else
 #endif
@@ -204,20 +194,10 @@ int nx_vopen(FAR const char *path, int oflags, va_list ap)
 
   /* Associate the inode with a file structure */
 
-  fd = files_allocate(inode, oflags, 0, NULL, 0);
-  if (fd < 0)
-    {
-      ret = fd;
-      goto errout_with_inode;
-    }
-
-  /* Get the file structure corresponding to the file descriptor. */
-
-  ret = fs_getfilep(fd, &filep);
-  if (ret < 0)
-    {
-      goto errout_with_inode;
-    }
+  filep->f_oflags = oflags;
+  filep->f_pos    = 0;
+  filep->f_inode  = inode;
+  filep->f_priv   = NULL;
 
   /* Perform the driver open operation.  NOTE that the open method may be
    * called many times.  The driver/mountpoint logic should handled this
@@ -241,16 +221,14 @@ int nx_vopen(FAR const char *path, int oflags, va_list ap)
 
   if (ret < 0)
     {
-      goto errout_with_fd;
+      goto errout_with_inode;
     }
 
   RELEASE_SEARCH(&desc);
-  return fd;
-
-errout_with_fd:
-  files_release(fd);
+  return OK;
 
 errout_with_inode:
+  filep->f_inode = NULL;
   inode_release(inode);
 
 errout_with_search:
@@ -259,6 +237,82 @@ errout_with_search:
 }
 
 /****************************************************************************
+ * Name: file_open
+ *
+ * Description:
+ *   file_open() is similar to the standard 'open' interface except that it
+ *   returns an instance of 'struct file' rather than a file descriptor.  It
+ *   also is not a cancellation point and does not modify the errno variable.
+ *
+ * Input Parameters:
+ *   filep  - The caller provided location in which to return the 'struct
+ *            file' instance.
+ *   path   - The full path to the file to be open.
+ *   oflags - open flags
+ *   ...    - Variable number of arguments, may include 'mode_t mode'
+ *
+ * Returned Value:
+ *   Zero (OK) is returned on success.  On failure, a negated errno value is
+ *   returned.
+ *
+ ****************************************************************************/
+
+int file_open(FAR struct file *filep, FAR const char *path, int oflags, ...)
+{
+  va_list ap;
+  int ret;
+
+  va_start(ap, oflags);
+  ret = file_vopen(filep, path, oflags, ap);
+  va_end(ap);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: nx_vopen
+ *
+ * Description:
+ *   nx_vopen() is identical to 'nx_open' except that it accepts a va_list
+ *   as an argument versus taking a variable length list of arguments.
+ *
+ *   nx_vopen() is an internal NuttX interface and should not be called from
+ *   applications.
+ *
+ * Returned Value:
+ *   The new file descriptor is returned on success; a negated errno value is
+ *   returned on any failure.
+ *
+ ****************************************************************************/
+
+int nx_vopen(FAR const char *path, int oflags, va_list ap)
+{
+  struct file filep;
+  int ret;
+  int fd;
+
+  /* Let file_vopen() do all of the work */
+
+  ret = file_vopen(&filep, path, oflags, ap);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  /* Allocate a new file descriptor for the inode */
+
+  fd = files_allocate(filep.f_inode, filep.f_oflags,
+                      filep.f_pos, filep.f_priv, 0);
+  if (fd < 0)
+    {
+      file_close(&filep);
+      return fd;
+    }
+
+  return fd;
+}
+
+/****************************************************************************
  * Name: nx_open
  *
  * Description:
diff --git a/include/nuttx/fs/fs.h b/include/nuttx/fs/fs.h
index 1a11bd0..e69c8e2 100644
--- a/include/nuttx/fs/fs.h
+++ b/include/nuttx/fs/fs.h
@@ -835,6 +835,8 @@ int nx_dup2(int fd1, int fd2);
  *
  ****************************************************************************/
 
+int file_vopen(FAR struct file *filep,
+               FAR const char *path, int oflags, va_list ap);
 int file_open(FAR struct file *filep, FAR const char *path, int oflags, ...);
 
 /****************************************************************************


[incubator-nuttx] 02/02: fs: Remove file_detach since it is unefficient to call open and file_detach

Posted by ag...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 1604fe0b4b52e823fd1514e44e9086209facb5f5
Author: Xiang Xiao <xi...@xiaomi.com>
AuthorDate: Sun Jan 3 02:35:06 2021 +0800

    fs: Remove file_detach since it is unefficient to call open and file_detach
    
    the kernel user should call file_open directly instead
    
    Signed-off-by: Xiang Xiao <xi...@xiaomi.com>
    Change-Id: I5bf7f661006f5d43739bc8618abfb4b983fde78d
---
 fs/inode/Make.defs       |   1 -
 fs/inode/fs_filedetach.c | 147 -----------------------------------------------
 fs/vfs/fs_close.c        |   5 +-
 include/nuttx/fs/fs.h    |  31 +---------
 4 files changed, 4 insertions(+), 180 deletions(-)

diff --git a/fs/inode/Make.defs b/fs/inode/Make.defs
index d019c5c..67c4549 100644
--- a/fs/inode/Make.defs
+++ b/fs/inode/Make.defs
@@ -36,7 +36,6 @@
 CSRCS += fs_files.c fs_foreachinode.c fs_inode.c fs_inodeaddref.c
 CSRCS += fs_inodebasename.c fs_inodefind.c fs_inodefree.c fs_inoderelease.c
 CSRCS += fs_inoderemove.c fs_inodereserve.c fs_inodesearch.c
-CSRCS += fs_filedetach.c
 
 # Include inode/utils build support
 
diff --git a/fs/inode/fs_filedetach.c b/fs/inode/fs_filedetach.c
deleted file mode 100644
index 227f97b..0000000
--- a/fs/inode/fs_filedetach.c
+++ /dev/null
@@ -1,147 +0,0 @@
-/****************************************************************************
- * fs/inode/fs_filedetach.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 <assert.h>
-#include <errno.h>
-
-#include <nuttx/sched.h>
-#include <nuttx/fs/fs.h>
-#include <nuttx/semaphore.h>
-
-#include "inode/inode.h"
-
-/****************************************************************************
- * Private Functions
- ****************************************************************************/
-
-/****************************************************************************
- * Name: _files_semtake
- ****************************************************************************/
-
-static inline int _files_semtake(FAR struct filelist *list)
-{
-  return nxsem_wait_uninterruptible(&list->fl_sem);
-}
-
-/****************************************************************************
- * Name: _files_semgive
- ****************************************************************************/
-
-#define _files_semgive(list) nxsem_post(&list->fl_sem)
-
-/****************************************************************************
- * Public Functions
- ****************************************************************************/
-
-/****************************************************************************
- * Name: file_detach
- *
- * Description:
- *   This function is used in device drivers to create a task-independent
- *   handle to an entity in the file system.  file_detach() duplicates the
- *   'struct file' that underlies the file descriptor, then closes the file
- *   descriptor.
- *
- *   This function will fail if fd is not a valid file descriptor.  In
- *   particular, it will fail if fd is a socket descriptor.
- *
- * Input Parameters:
- *   fd    - The file descriptor to be detached.  This descriptor will be
- *           closed and invalid if the file was successfully detached.
- *   filep - A pointer to a user provided memory location in which to
- *           received the duplicated, detached file structure.
- *
- * Returned Value:
- *   Zero (OK) is returned on success; A negated errno value is returned on
- *   any failure to indicate the nature of the failure.
- *
- ****************************************************************************/
-
-int file_detach(int fd, FAR struct file *filep)
-{
-  FAR struct filelist *list;
-  FAR struct file *parent;
-  int ret;
-
-  DEBUGASSERT(filep != NULL);
-
-  /* Verify the file descriptor range */
-
-  if (fd < 0 || fd >= CONFIG_NFILE_DESCRIPTORS)
-    {
-      /* Not a file descriptor (might be a socket descriptor) */
-
-      return -EBADF;
-    }
-
-  /* Get the thread-specific file list.  It should never be NULL in this
-   * context.
-   */
-
-  list = nxsched_get_files();
-  DEBUGASSERT(list != NULL);
-
-  /* If the file was properly opened, there should be an inode assigned */
-
-  ret = _files_semtake(list);
-  if (ret < 0)
-    {
-      /* Probably canceled */
-
-      return ret;
-    }
-
-  parent = &list->fl_files[fd];
-  if (parent->f_inode == NULL)
-    {
-      /* File is not open */
-
-      _files_semgive(list);
-      return -EBADF;
-    }
-
-  /* Duplicate the 'struct file' content into the user-provided file
-   * structure.
-   */
-
-  filep->f_oflags  = parent->f_oflags;
-  filep->f_pos     = parent->f_pos;
-  filep->f_inode   = parent->f_inode;
-  filep->f_priv    = parent->f_priv;
-
-  /* Release the file descriptor *without* calling the driver close method
-   * and without decrementing the inode reference count.  That will be done
-   * in file_close().
-   */
-
-  parent->f_oflags = 0;
-  parent->f_pos    = 0;
-  parent->f_inode  = NULL;
-  parent->f_priv   = NULL;
-
-  _files_semgive(list);
-  return OK;
-}
diff --git a/fs/vfs/fs_close.c b/fs/vfs/fs_close.c
index 89c4fd8..4f7eec8 100644
--- a/fs/vfs/fs_close.c
+++ b/fs/vfs/fs_close.c
@@ -60,12 +60,11 @@
  * Name: file_close
  *
  * Description:
- *   Close a file that was previously opend with file_open() (or detached
- *   with file_detach()).
+ *   Close a file that was previously opened with file_open().
  *
  * Input Parameters:
  *   filep - A pointer to a user provided memory location containing the
- *           open file data returned by file_detach().
+ *           open file data returned by file_open().
  *
  * Returned Value:
  *   Zero (OK) is returned on success; A negated errno value is returned on
diff --git a/include/nuttx/fs/fs.h b/include/nuttx/fs/fs.h
index e69c8e2..cc8b3ca 100644
--- a/include/nuttx/fs/fs.h
+++ b/include/nuttx/fs/fs.h
@@ -882,41 +882,14 @@ int nx_open(FAR const char *path, int oflags, ...);
 int fs_getfilep(int fd, FAR struct file **filep);
 
 /****************************************************************************
- * Name: file_detach
- *
- * Description:
- *   This function is used to device drivers to create a task-independent
- *   handle to an entity in the file system.  file_detach() duplicates the
- *   'struct file' that underlies the file descriptor, then closes the file
- *   descriptor.
- *
- *   This function will fail if fd is not a valid file descriptor.  In
- *   particular, it will fail if fd is a socket descriptor.
- *
- * Input Parameters:
- *   fd    - The file descriptor to be detached.  This descriptor will be
- *           closed and invalid if the file was successfully detached.
- *   filep - A pointer to a user provided memory location in which to
- *           received the duplicated, detached file structure.
- *
- * Returned Value:
- *   Zero (OK) is returned on success; A negated errno value is returned on
- *   any failure to indicate the nature of the failure.
- *
- ****************************************************************************/
-
-int file_detach(int fd, FAR struct file *filep);
-
-/****************************************************************************
  * Name: file_close
  *
  * Description:
- *   Close a file that was previously opened with file_open() (or detached
- *   with file_detach()).
+ *   Close a file that was previously opened with file_open().
  *
  * Input Parameters:
  *   filep - A pointer to a user provided memory location containing the
- *           open file data returned by file_detach().
+ *           open file data returned by file_open().
  *
  * Returned Value:
  *   Zero (OK) is returned on success; A negated errno value is returned on