You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "xiaoxiang781216 (via GitHub)" <gi...@apache.org> on 2023/04/01 14:15:39 UTC

[GitHub] [nuttx] xiaoxiang781216 opened a new pull request, #8939: fs: Flush the file system cache in BOARDIOC_POWEROFF too

xiaoxiang781216 opened a new pull request, #8939:
URL: https://github.com/apache/nuttx/pull/8939

   ## Summary
   
   and remove the check of SYS_DOWN since it defines to SYS_RESTART in reboot_notifer.h:
   ```
     #define SYS_DOWN        0x0001     /* Notify of system down */
     #define SYS_RESTART     SYS_DOWN
     #define SYS_HALT        0x0002     /* Notify of system halt */
     #define SYS_POWER_OFF   0x0003     /* Notify of system power off */
   ```
   
   ## Impact
   
   flush the cache in poweroff path
   
   ## Testing
   
   internal


-- 
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] [nuttx] pkarashchenko merged pull request #8939: fs: Flush the file system cache in BOARDIOC_POWEROFF too

Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko merged PR #8939:
URL: https://github.com/apache/nuttx/pull/8939


-- 
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] [nuttx] xiaoxiang781216 commented on pull request #8939: fs: Flush the file system cache in BOARDIOC_POWEROFF too

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #8939:
URL: https://github.com/apache/nuttx/pull/8939#issuecomment-1518139867

   ping @pkarashchenko 


-- 
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] [nuttx] fjpanag commented on pull request #8939: fs: Flush the file system cache in BOARDIOC_POWEROFF too

Posted by "fjpanag (via GitHub)" <gi...@apache.org>.
fjpanag commented on PR #8939:
URL: https://github.com/apache/nuttx/pull/8939#issuecomment-1496195094

   > `SYS_DOWN` does not need `sync()`?
   
   `SYS_DOWN` will call `sync()` since it uses the same value as `SYS_RESTART`:
   ```c
   #define SYS_RESTART     SYS_DOWN
   ```
   
   But IMO, this hurts the maintenability of the project.  
   What if someone changes this `define` in the future?
   
   Maybe it is better to change it to:
   ```c
   if (action == SYS_POWER_OFF || action == SYS_DOWN || action == SYS_RESTART)
   ```
   so we don't depend on these two macros having the same value?


-- 
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] [nuttx] hartmannathan commented on pull request #8939: fs: Flush the file system cache in BOARDIOC_POWEROFF too

Posted by "hartmannathan (via GitHub)" <gi...@apache.org>.
hartmannathan commented on PR #8939:
URL: https://github.com/apache/nuttx/pull/8939#issuecomment-1496209742

   > > `SYS_DOWN` does not need `sync()`?
   > 
   > `SYS_DOWN` will call `sync()` since it uses the same value as `SYS_RESTART`:
   > 
   > ```c
   > #define SYS_RESTART     SYS_DOWN
   > ```
   > 
   > But IMO, this hurts the maintenability of the project. What if someone changes this `define` in the future?
   > 
   > Maybe it is better to change it to:
   > 
   > ```c
   > if (action == SYS_POWER_OFF || action == SYS_DOWN || action == SYS_RESTART)
   > ```
   > 
   > so we don't depend on these two macros having the same value?
   
   +1


-- 
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] [nuttx] xiaoxiang781216 commented on pull request #8939: fs: Flush the file system cache in BOARDIOC_POWEROFF too

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #8939:
URL: https://github.com/apache/nuttx/pull/8939#issuecomment-1496214814

   OK, let's remove SYS_DOWN to avoid any potential issue.


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