[GitHub] [incubator-nuttx] hartmannathan commented on a change in pull request #5432: sched/wdog: fix delay calculation in wd_start()

[GitBox <gi...@apache.org>]

hartmannathan commented on a change in pull request #5432:
URL: https://github.com/apache/incubator-nuttx/pull/5432#discussion_r800762549



##########
File path: sched/wdog/wd_start.c
##########
@@ -74,6 +74,12 @@
 #  define CALL_FUNC(func, arg) func(arg)
 #endif
 
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static bool g_wdexpiration_context;

Review comment:
       Shouldn't it be volatile, also?

##########
File path: sched/wdog/wd_start.c
##########
@@ -204,15 +214,31 @@ int wd_start(FAR struct wdog_s *wdog, sclock_t delay,
   up_getpicbase(&wdog->picbase);
   wdog->arg = arg;
 
-  /* Calculate delay+1, forcing the delay into a range that we can handle */
+  /* Delay for at least one tick should be performed */
 
-  if (delay <= 0)
+  if (delay == 0)

Review comment:
       sclock_t is a signed value. Do we really want to use `==` here?

##########
File path: sched/wdog/wd_start.c
##########
@@ -204,15 +214,31 @@ int wd_start(FAR struct wdog_s *wdog, sclock_t delay,
   up_getpicbase(&wdog->picbase);
   wdog->arg = arg;
 
-  /* Calculate delay+1, forcing the delay into a range that we can handle */
+  /* Delay for at least one tick should be performed */
 
-  if (delay <= 0)
+  if (delay == 0)

Review comment:
       Maybe we need tri-state:
   
   ```
   if (delay < 0)
     {
       ...
     }
   else if (delay == 0)
     {
       ...
     }
   else
     {
       ...
     }
   
   ```




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