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 2022/10/18 10:26:52 UTC

[GitHub] [incubator-nuttx] fjpanag opened a new pull request, #7345: Added flush of the read buffers when a file is reopened.

fjpanag opened a new pull request, #7345:
URL: https://github.com/apache/incubator-nuttx/pull/7345

   ## Summary
   
   When `CONFIG_STDIO_DISABLE_BUFFERING` is not set (i.e. buffering IS enabled), and when a file is reopened (`freopen()`), then the read buffers where not flushed.
   
   This had as a result that the file position and the buffer became de-synchronized.  
   When reading from this file, wrong data were returned, from a position different to what the file position dictated.  
   
   The addition of the `fseek()` call ensures that the file position is correctly set to what the re-open wants, and internally calls `lib_rdflush()`.
   
   ## Impact
   
   Bug fix.
   
   ## Testing
   
   Tested on simulator with a test scenario that:
   1. Opened an existing file, and read 1 character from it.
   2. Re-opened the file.
   3. Read 1 character from the file again.
   
   Before, step 3 returned the *second* character of the file.  
   Now it correctly returns the *first* character, as it should.
   
   
   
   
   


-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7345: Added flush of the read buffers when a file is reopened.

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7345:
URL: https://github.com/apache/incubator-nuttx/pull/7345#discussion_r998032941


##########
libs/libc/stdio/lib_freopen.c:
##########
@@ -74,6 +75,7 @@ FAR FILE *freopen(FAR const char *path, FAR const char *mode,
   int oflags;
   int ret;
   int fd;
+  off_t f_pos;

Review Comment:
   ```suggestion
     off_t pos;
   ```



##########
libs/libc/stdio/lib_freopen.c:
##########
@@ -100,9 +102,19 @@ FAR FILE *freopen(FAR const char *path, FAR const char *mode,
           return NULL;
         }
 
-      /* Flush the stream and duplicate the new fd to it */
+      /* Get the new file position */
+
+      f_pos = lseek(fd, 0, SEEK_CUR);
+
+      /* Flush the stream and set the file position. The later is needed to
+       * ensure that the read buffers are also flushed.
+       */
 
       fflush(stream);
+      fseek(stream, f_pos, SEEK_SET);

Review Comment:
   should we add a new function to simply discard the buffer?



-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #7345: Added flush of the read buffers when a file is reopened.

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #7345:
URL: https://github.com/apache/incubator-nuttx/pull/7345


-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] fjpanag commented on a diff in pull request #7345: Added flush of the read buffers when a file is reopened.

Posted by GitBox <gi...@apache.org>.
fjpanag commented on code in PR #7345:
URL: https://github.com/apache/incubator-nuttx/pull/7345#discussion_r998091652


##########
libs/libc/stdio/lib_freopen.c:
##########
@@ -100,9 +102,19 @@ FAR FILE *freopen(FAR const char *path, FAR const char *mode,
           return NULL;
         }
 
-      /* Flush the stream and duplicate the new fd to it */
+      /* Get the new file position */
+
+      f_pos = lseek(fd, 0, SEEK_CUR);
+
+      /* Flush the stream and set the file position. The later is needed to
+       * ensure that the read buffers are also flushed.
+       */
 
       fflush(stream);
+      fseek(stream, f_pos, SEEK_SET);

Review Comment:
   I will change it to directly call `lib_rdflush()`.



-- 
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-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org