xiaoxiang781216 commented on code in PR #10602:
URL: https://github.com/apache/nuttx/pull/10602#discussion_r1334930318
##########
include/nuttx/fs/fs.h:
##########
@@ -425,6 +425,55 @@ 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.
+ */
+
+typedef ssize_t cookie_read_function_t(void *cookie, char *buf, size_t size);
+typedef ssize_t cookie_write_function_t(void *cookie, const char *buf,
+ size_t size);
+typedef off_t cookie_seek_function_t(void *cookie, off_t *offset,
Review Comment:
```suggestion
typedef CODE int cookie_seek_function_t(FAR void *cookie, FAR off_t *offset,
```
add CODE to ALL function typdef
##########
libs/libc/libc.h:
##########
@@ -245,6 +245,13 @@ bool lib_isbasedigit(int ch, int base, FAR int *value);
int lib_checkbase(int base, FAR const char **pptr);
+/* Defined in lib_stdio_cb */
Review Comment:
```suggestion
/* Defined in lib_stdio_cb.c */
```
##########
include/nuttx/fs/fs.h:
##########
@@ -497,6 +546,8 @@ struct file_struct
FAR struct file_struct *fs_next; /* Pointer to next file stream */
rmutex_t fs_lock; /* Recursive lock */
int fs_fd; /* File descriptor associated with
stream */
+ FAR struct cookie_s fs_cookie; /* Cookie structure for fopencookie */
+ io_callbacks_t fs_callbacks; /* Internal callbacks for IO call */
Review Comment:
duplicated with fs_cookie.cookie_io
##########
include/nuttx/fs/fs.h:
##########
@@ -425,6 +425,55 @@ 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.
+ */
+
+typedef ssize_t cookie_read_function_t(void *cookie, char *buf, size_t size);
+typedef ssize_t cookie_write_function_t(void *cookie, const char *buf,
+ size_t size);
+typedef off_t cookie_seek_function_t(void *cookie, off_t *offset,
+ int whence);
+typedef int cookie_close_function_t(void *cookie);
+
+typedef struct
+{
+ FAR cookie_read_function_t *read;
+ FAR cookie_write_function_t *write;
+ FAR cookie_seek_function_t *seek;
+ FAR cookie_close_function_t *close;
+} cookie_io_functions_t;
+
+struct cookie_s
+{
+ FAR void *cookie; /* Pointer to the caller's cookie struct */
+ cookie_io_functions_t cookie_io; /* Programmer-defined hook functions */
+};
+
+/* Definition of internal callbacks for read/write/seek/close operations.
+ * These are needed to ensure custom callbacks from fopencookie are called
+ * from user address space and not from kernel address space. These callbacks
+ * are either routed to fopencookie related callbacks or to internal
+ * read/write/seek/close operations defined in file system (thus in kernel
+ * address space)
+ */
+
+typedef ssize_t read_internal_cb_t(FAR void *s, char *buf, size_t size);
+typedef ssize_t write_internal_cb_t(FAR void *s, const char *buf,
+ size_t size);
+typedef off_t seek_internal_cb_t(FAR void *s, off_t offset, int whence);
+typedef int close_internal_cb_t(FAR void *s);
+
+typedef struct
+{
+ FAR read_internal_cb_t *read;
+ FAR write_internal_cb_t *write;
+ FAR seek_internal_cb_t *seek;
+ FAR close_internal_cb_t *close;
+} io_callbacks_t;
Review Comment:
why define the duplicated typedef and struct(from line 463 to line 475), but
use cookie_io_functions_t directly?
##########
include/nuttx/fs/fs.h:
##########
@@ -425,6 +425,55 @@ 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.
+ */
+
+typedef ssize_t cookie_read_function_t(void *cookie, char *buf, size_t size);
Review Comment:
add FAR to ALL pointers
##########
include/nuttx/fs/fs.h:
##########
@@ -497,6 +546,8 @@ struct file_struct
FAR struct file_struct *fs_next; /* Pointer to next file stream */
rmutex_t fs_lock; /* Recursive lock */
int fs_fd; /* File descriptor associated with
stream */
Review Comment:
fs_fd should union with fs_cookie.cookie
##########
include/nuttx/fs/fs.h:
##########
@@ -425,6 +425,55 @@ 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.
+ */
+
+typedef ssize_t cookie_read_function_t(void *cookie, char *buf, size_t size);
+typedef ssize_t cookie_write_function_t(void *cookie, const char *buf,
+ size_t size);
+typedef off_t cookie_seek_function_t(void *cookie, off_t *offset,
+ int whence);
+typedef int cookie_close_function_t(void *cookie);
+
+typedef struct
Review Comment:
```suggestion
typedef struct cookie_io_functions_s
```
##########
libs/libc/libc.h:
##########
@@ -245,6 +245,13 @@ bool lib_isbasedigit(int ch, int base, FAR int *value);
int lib_checkbase(int base, FAR const char **pptr);
+/* Defined in lib_stdio_cb */
+
+ssize_t lib_fread_cb(void *stream, char *buf, size_t size);
Review Comment:
add FAR for ALL pointer
##########
libs/libc/stdio/lib_fclose.c:
##########
@@ -129,7 +129,7 @@ int fclose(FAR FILE *stream)
(uintptr_t)stream);
status = android_fdsan_close_with_tag(stream->fs_fd, tag);
Review Comment:
need move fdscan related code to stdio callback
##########
libs/libc/libc.h:
##########
@@ -245,6 +245,13 @@ bool lib_isbasedigit(int ch, int base, FAR int *value);
int lib_checkbase(int base, FAR const char **pptr);
+/* Defined in lib_stdio_cb */
+
+ssize_t lib_fread_cb(void *stream, char *buf, size_t size);
+ssize_t lib_fwrite_cb(void *stream, const char *buf, size_t size);
+off_t lib_fseek_cb(void *stream, off_t offset, int whence);
Review Comment:
```suggestion
int lib_fseek_cb(void *stream, off_t offset, int whence);
```
##########
libs/libc/stdio/lib_fclose.c:
##########
@@ -129,7 +129,7 @@ int fclose(FAR FILE *stream)
(uintptr_t)stream);
status = android_fdsan_close_with_tag(stream->fs_fd, tag);
#else
- status = close(stream->fs_fd);
+ status = stream->fs_callbacks.close(stream);
Review Comment:
need pass cookie instead stream here
##########
libs/libc/stdio/lib_stdio_cb.c:
##########
@@ -0,0 +1,76 @@
+/****************************************************************************
+ * libs/libc/stdio/lib_stdio_cb.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 <stdio.h>
+#include <unistd.h>
+#include <errno.h>
+
+#include "libc.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: lib_fseek_cb
+ ****************************************************************************/
+
+off_t lib_fseek_cb(FAR void *s, off_t offset, int whence)
+{
+ FAR FILE *stream = (FAR FILE *)s;
+ return lseek(stream->fs_fd, offset, whence);
+}
+
+/****************************************************************************
+ * Name: lib_fwrite_cb
+ ****************************************************************************/
+
+ssize_t lib_fwrite_cb(FAR void *s, const char *buf, size_t size)
+{
+ FAR FILE *stream = (FAR FILE *)s;
+ return _NX_WRITE(stream->fs_fd, buf, size);
Review Comment:
change _NX_WRITE to write
##########
libs/libc/stdio/lib_stdio_cb.c:
##########
@@ -0,0 +1,76 @@
+/****************************************************************************
+ * libs/libc/stdio/lib_stdio_cb.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 <stdio.h>
+#include <unistd.h>
+#include <errno.h>
+
+#include "libc.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: lib_fseek_cb
+ ****************************************************************************/
+
+off_t lib_fseek_cb(FAR void *s, off_t offset, int whence)
+{
+ FAR FILE *stream = (FAR FILE *)s;
+ return lseek(stream->fs_fd, offset, whence);
+}
+
+/****************************************************************************
+ * Name: lib_fwrite_cb
+ ****************************************************************************/
+
+ssize_t lib_fwrite_cb(FAR void *s, const char *buf, size_t size)
+{
+ FAR FILE *stream = (FAR FILE *)s;
+ return _NX_WRITE(stream->fs_fd, buf, size);
+}
+
+/****************************************************************************
+ * Name: lib_fread_cb
+ ****************************************************************************/
+
+ssize_t lib_fread_cb(FAR void *s, char *buf, size_t size)
+{
+ FAR FILE *stream = (FAR FILE *)s;
+ return _NX_READ(stream->fs_fd, buf, size);
Review Comment:
change _NX_READ to read
##########
libs/libc/stdio/lib_fopencookie.c:
##########
@@ -0,0 +1,181 @@
+/****************************************************************************
+ * libs/libc/stdio/lib_fopencookie.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 <stdlib.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <string.h>
+#include <assert.h>
+#include <errno.h>
+#include <nuttx/tls.h>
+
+#include "libc.h"
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+static ssize_t freadcookie(FAR void *s, FAR char *buf, size_t nbytes);
+static ssize_t fwritecookie(FAR void *s, FAR const char *buf, size_t nbytes);
+static off_t fseekcookie(FAR void *s, off_t offset, int whence);
+static int fclosecookie(FAR void *s);
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static FAR FILE *fopencookie_init_stream(int oflags)
+{
+ FAR FILE *stream;
+
+ stream = lib_zalloc(sizeof(FILE));
+ if (stream == NULL)
+ {
+ set_errno(-ENOMEM);
+ return NULL;
+ }
+
+ nxrmutex_init(&stream->fs_lock);
+
+#ifndef CONFIG_STDIO_DISABLE_BUFFERING
+#if CONFIG_STDIO_BUFFER_SIZE > 0
+ /* Set up pointers */
+
+ stream->fs_bufstart = stream->fs_buffer;
+ stream->fs_bufend = &stream->fs_bufstart[CONFIG_STDIO_BUFFER_SIZE];
+ stream->fs_bufpos = stream->fs_bufstart;
+ stream->fs_bufread = stream->fs_bufstart;
+ stream->fs_flags = __FS_FLAG_UBF; /* Fake setvbuf and fclose */
+
+#ifdef CONFIG_STDIO_LINEBUFFER
+ /* Setup buffer flags */
+
+ stream->fs_flags |= __FS_FLAG_LBF; /* Line buffering */
+
+#endif /* CONFIG_STDIO_LINEBUFFER */
+#endif /* CONFIG_STDIO_BUFFER_SIZE > 0 */
+#endif /* CONFIG_STDIO_DISABLE_BUFFERING */
+
+ /* Save the file description and open flags. Setting the
+ * file descriptor locks this stream.
+ */
+
+ stream->fs_fd = -1;
+ stream->fs_oflags = oflags;
+
+ return stream;
+}
+
+static ssize_t freadcookie(FAR void *s, FAR char *buf, size_t nbytes)
+{
+ FAR FILE *stream = (FAR FILE *)s;
+ FAR struct cookie_s *cookie = (FAR struct cookie_s *)&stream->fs_cookie;
+ int ret;
+
+ DEBUGASSERT(cookie->cookie != NULL && cookie->cookie_io.read != NULL);
+
+ ret = cookie->cookie_io.read(cookie->cookie, buf, nbytes);
+ return ret;
+}
+
+static ssize_t fwritecookie(FAR void *s, FAR const char *buf, size_t nbytes)
+{
+ FAR FILE *stream = (FAR FILE *)s;
+ FAR struct cookie_s *cookie = (FAR struct cookie_s *)&stream->fs_cookie;
+ int ret;
+
+ DEBUGASSERT(cookie->cookie != NULL && cookie->cookie_io.write != NULL);
+
+ ret = cookie->cookie_io.write(cookie->cookie, buf, nbytes);
+ return ret;
+}
+
+static off_t fseekcookie(FAR void *s, off_t offset, int whence)
+{
+ FAR FILE *stream = (FAR FILE *)s;
+ FAR struct cookie_s *cookie = (FAR struct cookie_s *)&stream->fs_cookie;
+ int ret;
+
+ DEBUGASSERT(cookie->cookie != NULL && cookie->cookie_io.seek != NULL);
+
+ ret = cookie->cookie_io.seek(cookie->cookie, &offset, whence);
+ return ret;
+}
+
+static int fclosecookie(FAR void *s)
Review Comment:
let's remove f*cookie and forward to cookie_io_functions_t directly
--
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
