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


##########
drivers/timers/watchdog.c:
##########
@@ -153,24 +156,121 @@ static const struct file_operations g_wdogops =
 static ATOMIC_NOTIFIER_HEAD(g_watchdog_notifier_list);
 #endif
 
+#if defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_CAPTURE)

Review Comment:
   ```suggestion
   #ifdef CONFIG_WATCHDOG_AUTOMONITOR_BY_CAPTURE
   ```



##########
drivers/timers/watchdog.c:
##########
@@ -153,24 +156,121 @@ static const struct file_operations g_wdogops =
 static ATOMIC_NOTIFIER_HEAD(g_watchdog_notifier_list);
 #endif
 
+#if defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_CAPTURE)
+/* Active capture automonitor instances.  Lower halves that pass an argument
+ * can identify their upper-half directly or through the lower-half pointer.
+ * A NULL argument is accepted only when this list contains one instance.
+ */
+
+static FAR struct watchdog_upperhalf_s *g_watchdog_capture_list;
+#endif
+
 /****************************************************************************
  * Private Functions
  ****************************************************************************/
 
 #if defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_CAPTURE)
+static void watchdog_capture_add(FAR struct watchdog_upperhalf_s *upper)
+{
+  irqstate_t flags = enter_critical_section();
+
+  upper->capture_next = g_watchdog_capture_list;
+  g_watchdog_capture_list = upper;
+  leave_critical_section(flags);
+}
+
+static void watchdog_capture_remove(FAR struct watchdog_upperhalf_s *upper)
+{
+  FAR struct watchdog_upperhalf_s **cursor;
+  irqstate_t flags = enter_critical_section();

Review Comment:
   change to spinlock



##########
drivers/timers/watchdog.c:
##########
@@ -153,24 +156,121 @@ static const struct file_operations g_wdogops =
 static ATOMIC_NOTIFIER_HEAD(g_watchdog_notifier_list);
 #endif
 
+#if defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_CAPTURE)
+/* Active capture automonitor instances.  Lower halves that pass an argument
+ * can identify their upper-half directly or through the lower-half pointer.
+ * A NULL argument is accepted only when this list contains one instance.
+ */
+
+static FAR struct watchdog_upperhalf_s *g_watchdog_capture_list;
+#endif
+
 /****************************************************************************
  * Private Functions
  ****************************************************************************/
 
 #if defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_CAPTURE)
+static void watchdog_capture_add(FAR struct watchdog_upperhalf_s *upper)
+{
+  irqstate_t flags = enter_critical_section();
+
+  upper->capture_next = g_watchdog_capture_list;
+  g_watchdog_capture_list = upper;
+  leave_critical_section(flags);
+}
+
+static void watchdog_capture_remove(FAR struct watchdog_upperhalf_s *upper)
+{
+  FAR struct watchdog_upperhalf_s **cursor;
+  irqstate_t flags = enter_critical_section();
+
+  cursor = &g_watchdog_capture_list;
+  while (*cursor != NULL)
+    {
+      if (*cursor == upper)
+        {
+          *cursor = upper->capture_next;
+          upper->capture_next = NULL;
+          break;
+        }
+
+      cursor = &(*cursor)->capture_next;
+    }
+
+  leave_critical_section(flags);
+}
+
+static FAR struct watchdog_upperhalf_s *
+watchdog_capture_find(FAR void *arg)
+{
+  FAR struct watchdog_upperhalf_s *upper;
+  FAR struct watchdog_upperhalf_s *match = NULL;
+  irqstate_t flags = enter_critical_section();
+
+  for (upper = g_watchdog_capture_list; upper != NULL;
+       upper = upper->capture_next)
+    {
+      if (arg != NULL && (arg == upper || arg == upper->lower))
+        {
+          match = upper;
+          break;
+        }
+
+      if (arg == NULL)
+        {
+          if (match != NULL)
+            {
+              /* More than one NULL-context instance cannot be identified. */
+
+              match = NULL;
+              break;
+            }
+
+          match = upper;
+        }
+    }
+
+  leave_critical_section(flags);
+  return match;
+}
+
 static int watchdog_automonitor_capture(int irq, FAR void *context,
                                         FAR void *arg)
 {
-  FAR struct watchdog_upperhalf_s *upper = arg;
+  FAR struct watchdog_upperhalf_s *upper = watchdog_capture_find(arg);
+
+  /* A stop operation can race with a pending interrupt.  Do not dereference
+   * a removed association after automonitor has been stopped.
+   */
+
+  if (upper == NULL)
+    {
+      return 0;
+    }
+
   FAR struct watchdog_lowerhalf_s *lower = upper->lower;

Review Comment:
   need in the definition section



##########
drivers/timers/watchdog.c:
##########
@@ -153,24 +156,121 @@ static const struct file_operations g_wdogops =
 static ATOMIC_NOTIFIER_HEAD(g_watchdog_notifier_list);
 #endif
 
+#if defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_CAPTURE)
+/* Active capture automonitor instances.  Lower halves that pass an argument
+ * can identify their upper-half directly or through the lower-half pointer.
+ * A NULL argument is accepted only when this list contains one instance.
+ */
+
+static FAR struct watchdog_upperhalf_s *g_watchdog_capture_list;
+#endif
+
 /****************************************************************************
  * Private Functions
  ****************************************************************************/
 
 #if defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_CAPTURE)
+static void watchdog_capture_add(FAR struct watchdog_upperhalf_s *upper)
+{
+  irqstate_t flags = enter_critical_section();
+
+  upper->capture_next = g_watchdog_capture_list;
+  g_watchdog_capture_list = upper;
+  leave_critical_section(flags);
+}
+
+static void watchdog_capture_remove(FAR struct watchdog_upperhalf_s *upper)
+{
+  FAR struct watchdog_upperhalf_s **cursor;
+  irqstate_t flags = enter_critical_section();
+
+  cursor = &g_watchdog_capture_list;
+  while (*cursor != NULL)
+    {
+      if (*cursor == upper)
+        {
+          *cursor = upper->capture_next;
+          upper->capture_next = NULL;
+          break;
+        }
+
+      cursor = &(*cursor)->capture_next;
+    }
+
+  leave_critical_section(flags);
+}
+
+static FAR struct watchdog_upperhalf_s *
+watchdog_capture_find(FAR void *arg)
+{
+  FAR struct watchdog_upperhalf_s *upper;
+  FAR struct watchdog_upperhalf_s *match = NULL;
+  irqstate_t flags = enter_critical_section();
+
+  for (upper = g_watchdog_capture_list; upper != NULL;
+       upper = upper->capture_next)
+    {
+      if (arg != NULL && (arg == upper || arg == upper->lower))

Review Comment:
   ```
   if (arg == NULL)
     {
        ...
     }
   else if (arg == upper || arg == upper->lower)
     {
       ...
     }
   ```



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to