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


##########
include/nuttx/fs/fs.h:
##########
@@ -426,6 +426,31 @@ struct inode
 
 #define FSNODE_SIZE(n) (sizeof(struct inode) + (n))
 
+/* Definitions for custom stream operations with fopencookie. The
+ * implementation is as defined in Standard C library (libc). The only
+ * difference is that we use off_t instead of off64_t. This means
+ * off_t is int64_t if CONFIG_FS_LARGEFILE is defined and int32_t if not.
+ *
+ * These callbacks can either lead to custom functions if fopencookie is used
+ * or to standard file system functions if not.
+ */
+
+typedef CODE ssize_t cookie_read_function_t(void *cookie, char *buf,
+                                            size_t size);
+typedef CODE ssize_t cookie_write_function_t(void *cookie, const char *buf,
+                                             size_t size);
+typedef CODE off_t cookie_seek_function_t(void *cookie, off_t *offset,
+                                     int whence);

Review Comment:
   align



##########
libs/libc/stdio/lib_libfflush.c:
##########
@@ -107,7 +107,9 @@ ssize_t lib_fflush_unlocked(FAR FILE *stream, bool bforce)
         {
           /* Perform the write */
 
-          bytes_written = _NX_WRITE(stream->fs_fd, src, nbuffer);
+          bytes_written = stream->fs_iofunc.write(stream->fs_cookie,
+                                                  (const char *)src,

Review Comment:
   can we change the type at line 62 to avoid the cast



##########
libs/libc/stdio/lib_ungetc.c:
##########
@@ -52,7 +53,7 @@ int ungetc(int c, FAR FILE *stream)
 
   /* Stream must be open for read access */
 
-  if ((stream->fs_fd < 0) || ((stream->fs_oflags & O_RDOK) == 0))
+  if ((fd < 0) || ((stream->fs_oflags & O_RDOK) == 0))

Review Comment:
   should we allow cookie case? which may pass in the negative value



##########
libs/libc/stdio/lib_libfgets.c:
##########
@@ -97,10 +97,11 @@ FAR char *lib_fgets_unlocked(FAR char *buf, size_t buflen, 
FILE *stream,
                              bool keepnl, bool consume)
 {
   size_t nch = 0;
+  int fd = (int)(intptr_t)stream->fs_cookie;
 
   /* Sanity checks */
 
-  if (!stream || !buf || stream->fs_fd < 0)
+  if (!stream || !buf || fd < 0)

Review Comment:
   should we allow cookie < 0?



##########
include/nuttx/fs/fs.h:
##########
@@ -425,6 +425,31 @@ struct inode
 
 #define FSNODE_SIZE(n) (sizeof(struct inode) + (n))
 
+/* Definitions for custom stream operations with fopencookie. The
+ * implementation is as defined in Standard C library (libc). The only
+ * difference is that we use off_t instead of off64_t. This means
+ * off_t is int64_t if CONFIG_FS_LARGEFILE is defined and int32_t if not.
+ *
+ * These callbacks can either lead to custom functions if fopencookie is used
+ * or to standard file system functions if not.
+ */
+
+typedef CODE ssize_t cookie_read_function_t(void *cookie, char *buf,

Review Comment:
   but, not change



##########
libs/libc/stdio/lib_libfread_unlocked.c:
##########
@@ -213,9 +215,9 @@ ssize_t lib_fread_unlocked(FAR void *ptr, size_t count, FAR 
FILE *stream)
                        * into the buffer.
                        */
 
-                      bytes_read = _NX_READ(stream->fs_fd,
-                                            stream->fs_bufread,
-                                            buffer_available);
+                      bytes_read = stream->fs_iofunc.read(stream->fs_cookie,
+                                              (char *)stream->fs_bufread,

Review Comment:
   could we change fs_bufread type to char * to avoid the cast



##########
libs/libc/stdio/lib_fclose.c:
##########
@@ -115,32 +115,18 @@ int fclose(FAR FILE *stream)
 
       nxmutex_unlock(&slist->sl_lock);
 
-      /* Check that the underlying file descriptor corresponds to an an open
-       * file.
-       */
-
-      if (stream->fs_fd >= 0)
-        {
-          /* Close the file descriptor and save the return status */
+      /* Close the file descriptor and save the return status */
 
-#ifdef CONFIG_FDSAN
-          uint64_t tag;
-          tag = android_fdsan_create_owner_tag(ANDROID_FDSAN_OWNER_TYPE_FILE,
-                                               (uintptr_t)stream);
-          status = android_fdsan_close_with_tag(stream->fs_fd, tag);
-#else
-          status = close(stream->fs_fd);
-#endif
+      status = stream->fs_iofunc.close(stream->fs_cookie);

Review Comment:
   should we check close != NULL before calling, or fill the empty callback 
function in fopencookie



##########
sched/tls/task_initinfo.c:
##########
@@ -28,6 +28,8 @@
 #include <nuttx/kmalloc.h>
 #include <nuttx/mutex.h>
 
+#include <nuttx/lib/lib.h>

Review Comment:
   change to libc.h



##########
libs/libc/stdio/lib_setvbuf.c:
##########
@@ -133,7 +134,7 @@ int setvbuf(FAR FILE *stream, FAR char *buffer, int mode, 
size_t size)
 
   /* Return EBADF if the file is not open */
 
-  if (stream->fs_fd < 0)
+  if (fd < 0)

Review Comment:
   should we allow for cookie case



##########
libs/libc/stdio/lib_libfread_unlocked.c:
##########
@@ -170,7 +170,9 @@ ssize_t lib_fread_unlocked(FAR void *ptr, size_t count, FAR 
FILE *stream)
 
                   if (remaining > buffer_available)
                     {
-                      bytes_read = _NX_READ(stream->fs_fd, dest, remaining);
+                      bytes_read = stream->fs_iofunc.read(stream->fs_cookie,
+                                                          (char *)dest,

Review Comment:
   can we change the type at line 49 to avoid the cast



##########
libs/libc/stdio/lib_libfwrite.c:
##########
@@ -132,7 +132,8 @@ ssize_t lib_fwrite_unlocked(FAR const void *ptr, size_t 
count,
 
   if (count >= CONFIG_STDIO_BUFFER_SIZE)
     {
-      ret = _NX_WRITE(stream->fs_fd, src, count);
+      ret = stream->fs_iofunc.write(stream->fs_cookie, (const char *)src,

Review Comment:
   change src type to avoid the cast



##########
libs/libc/stdio/lib_libfread_unlocked.c:
##########
@@ -258,7 +260,9 @@ ssize_t lib_fread_unlocked(FAR void *ptr, size_t count, FAR 
FILE *stream)
 
           while (remaining > 0)
             {
-              bytes_read = _NX_READ(stream->fs_fd, dest, remaining);
+              bytes_read = stream->fs_iofunc.read(stream->fs_cookie,
+                                                  (char *)dest,

Review Comment:
   ditto



##########
libs/libc/stdio/lib_libfread_unlocked.c:
##########
@@ -213,9 +215,9 @@ ssize_t lib_fread_unlocked(FAR void *ptr, size_t count, FAR 
FILE *stream)
                        * into the buffer.
                        */
 
-                      bytes_read = _NX_READ(stream->fs_fd,
-                                            stream->fs_bufread,
-                                            buffer_available);
+                      bytes_read = stream->fs_iofunc.read(stream->fs_cookie,
+                                              (char *)stream->fs_bufread,

Review Comment:
   but, need add FAR



-- 
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