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


##########
include/nuttx/event.h:
##########
@@ -0,0 +1,258 @@
+/****************************************************************************
+ * include/nuttx/event.h
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_EVENT_H
+#define __INCLUDE_NUTTX_EVENT_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/list.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Initializers */
+
+#define NXEVENT_INITIALIZER(e, v) {LIST_INITIAL_VALUE((e).list), (v)}
+
+/* Event Flags */
+
+#define NXEVENT_WAIT_ALL     (1 << 0) /* Bit 0: Wait ALL */
+#define NXEVENT_WAIT_RESET   (1 << 1) /* Bit 1: Reset events before wait */
+#define NXEVENT_WAIT_NOCLEAR (1 << 2) /* Bit 2: Do not clear events after wait 
*/
+#define NXEVENT_POST_ALL     (1 << 3) /* Bit 3: Post ALL */
+#define NXEVENT_POST_SET     (1 << 4) /* Bit 4: Set event after post */

Review Comment:
   ```suggestion
   #define NXEVENT_POST_SET     (1 << 1) /* Bit 4: Set event after post */
   ```



##########
include/nuttx/event.h:
##########
@@ -0,0 +1,258 @@
+/****************************************************************************
+ * include/nuttx/event.h
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_EVENT_H
+#define __INCLUDE_NUTTX_EVENT_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/list.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Initializers */
+
+#define NXEVENT_INITIALIZER(e, v) {LIST_INITIAL_VALUE((e).list), (v)}
+
+/* Event Flags */
+
+#define NXEVENT_WAIT_ALL     (1 << 0) /* Bit 0: Wait ALL */
+#define NXEVENT_WAIT_RESET   (1 << 1) /* Bit 1: Reset events before wait */
+#define NXEVENT_WAIT_NOCLEAR (1 << 2) /* Bit 2: Do not clear events after wait 
*/
+#define NXEVENT_POST_ALL     (1 << 3) /* Bit 3: Post ALL */

Review Comment:
   ```suggestion
   #define NXEVENT_POST_ALL     (1 << 0) /* Bit 3: Post ALL */
   ```



##########
sched/event/event_destroy.c:
##########
@@ -0,0 +1,50 @@
+/****************************************************************************
+ * sched/event/event_destroy.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 "event.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nxevent_destroy
+ *
+ * Description:
+ *   This function is used to destroy the event.
+ *
+ * Input Parameters:
+ *   event  - Address of the event object
+ *
+ * Returned Value:
+ *   This is an internal OS interface and should not be used by applications.
+ *   It follows the NuttX internal error return policy:  Zero (OK) is
+ *   returned on success.  A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+int nxevent_destroy(FAR nxevent_t *event)
+{
+  return nxevent_post(event, 0, NXEVENT_POST_ALL);

Review Comment:
   ```suggestion
     return nxevent_post(event, 0, NXEVENT_POST_ALL | NXEVENT_POST_SET);
   ```



##########
sched/event/event_post.c:
##########
@@ -0,0 +1,162 @@
+/****************************************************************************
+ * sched/event/event_post.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/sched.h>
+
+#include "event.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nxevent_sem_post
+ ****************************************************************************/
+
+static inline_function int nxevent_sem_post(FAR sem_t *sem)
+{
+  int semcount;
+
+  nxsem_get_value(sem, &semcount);
+  if (semcount < 1)
+    {
+      return nxsem_post(sem);
+    }
+
+  return 0;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nxevent_post
+ *
+ * Description:
+ *   Post one or more events to an event object.
+ *
+ *   This routine posts one or more events to an event object. All tasks
+ *   waiting on the event object event whose waiting conditions become
+ *   met by this posting immediately unpend.
+ *
+ *   Posting differs from setting in that posted events are merged together
+ *   with the current set of events tracked by the event object.
+ *
+ * Input Parameters:
+ *   event  - Address of the event object
+ *   events - Set of events to post to event
+ *          - Set events to 0 will be considered as any,
+ *            waking up the waiting thread immediately.
+ *
+ * Returned Value:
+ *   This is an internal OS interface and should not be used by applications.
+ *   It follows the NuttX internal error return policy:  Zero (OK) is
+ *   returned on success.  A negated errno value is returned on failure.
+ *
+ * Assumptions:
+ *   This function may be called from an interrupt handler.
+ *
+ ****************************************************************************/
+
+int nxevent_post(FAR nxevent_t *event, nxevent_mask_t events,
+                 nxevent_flags_t eflags)
+{
+  nxevent_mask_t clear = 0;
+  FAR nxevent_wait_t *wait;
+  FAR nxevent_wait_t *tmp;
+  irqstate_t flags;
+  bool waitall;
+  bool postall;
+  int ret = 0;
+
+  if (event == NULL)
+    {
+      return -EINVAL;
+    }
+
+  if (events == 0)
+    {
+      events = ~0;
+    }
+
+  flags = enter_critical_section();
+
+  event->events |= events;
+
+  if (!list_is_empty(&event->list))
+    {
+      postall = ((eflags & NXEVENT_POST_ALL) != 0);
+
+      /* Hold schedule lock here to avoid context switch if post high
+       * priority task.
+       */
+
+      sched_lock();
+
+      list_for_every_entry_safe(&event->list, wait, tmp,
+                                nxevent_wait_t, node)
+        {
+          waitall = ((wait->eflags & NXEVENT_WAIT_ALL) != 0);
+
+          if ((!waitall && ((wait->expect & event->events) != 0)) ||
+              (waitall && ((wait->expect & event->events) == wait->expect)))
+            {
+              list_delete(&wait->node);
+
+              if (!waitall)
+                {
+                  wait->expect &= event->events;
+                }
+
+              if (!postall && (wait->eflags & NXEVENT_WAIT_NOCLEAR) == 0)

Review Comment:
   why need check postall?



##########
sched/event/event_post.c:
##########
@@ -0,0 +1,162 @@
+/****************************************************************************
+ * sched/event/event_post.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/sched.h>
+
+#include "event.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nxevent_sem_post
+ ****************************************************************************/
+
+static inline_function int nxevent_sem_post(FAR sem_t *sem)
+{
+  int semcount;
+
+  nxsem_get_value(sem, &semcount);
+  if (semcount < 1)
+    {
+      return nxsem_post(sem);
+    }
+
+  return 0;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nxevent_post
+ *
+ * Description:
+ *   Post one or more events to an event object.
+ *
+ *   This routine posts one or more events to an event object. All tasks
+ *   waiting on the event object event whose waiting conditions become
+ *   met by this posting immediately unpend.
+ *
+ *   Posting differs from setting in that posted events are merged together
+ *   with the current set of events tracked by the event object.
+ *
+ * Input Parameters:
+ *   event  - Address of the event object
+ *   events - Set of events to post to event
+ *          - Set events to 0 will be considered as any,
+ *            waking up the waiting thread immediately.
+ *
+ * Returned Value:
+ *   This is an internal OS interface and should not be used by applications.
+ *   It follows the NuttX internal error return policy:  Zero (OK) is
+ *   returned on success.  A negated errno value is returned on failure.
+ *
+ * Assumptions:
+ *   This function may be called from an interrupt handler.
+ *
+ ****************************************************************************/
+
+int nxevent_post(FAR nxevent_t *event, nxevent_mask_t events,
+                 nxevent_flags_t eflags)
+{
+  nxevent_mask_t clear = 0;
+  FAR nxevent_wait_t *wait;
+  FAR nxevent_wait_t *tmp;
+  irqstate_t flags;
+  bool waitall;
+  bool postall;
+  int ret = 0;
+
+  if (event == NULL)
+    {
+      return -EINVAL;
+    }
+
+  if (events == 0)
+    {
+      events = ~0;
+    }
+
+  flags = enter_critical_section();
+
+  event->events |= events;
+
+  if (!list_is_empty(&event->list))
+    {
+      postall = ((eflags & NXEVENT_POST_ALL) != 0);
+
+      /* Hold schedule lock here to avoid context switch if post high
+       * priority task.
+       */
+
+      sched_lock();
+
+      list_for_every_entry_safe(&event->list, wait, tmp,
+                                nxevent_wait_t, node)
+        {
+          waitall = ((wait->eflags & NXEVENT_WAIT_ALL) != 0);
+
+          if ((!waitall && ((wait->expect & event->events) != 0)) ||
+              (waitall && ((wait->expect & event->events) == wait->expect)))
+            {
+              list_delete(&wait->node);
+
+              if (!waitall)
+                {
+                  wait->expect &= event->events;
+                }
+
+              if (!postall && (wait->eflags & NXEVENT_WAIT_NOCLEAR) == 0)
+                {
+                  clear |= wait->expect;
+                }
+
+              ret = nxevent_sem_post(&wait->sem);

Review Comment:
   move before line 128 to avoid changing clear in fail case.



##########
include/nuttx/event.h:
##########
@@ -0,0 +1,258 @@
+/****************************************************************************
+ * include/nuttx/event.h
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_EVENT_H
+#define __INCLUDE_NUTTX_EVENT_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <nuttx/list.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Initializers */
+
+#define NXEVENT_INITIALIZER(e, v) {LIST_INITIAL_VALUE((e).list), (v)}
+
+/* Event Flags */
+
+#define NXEVENT_WAIT_ALL     (1 << 0) /* Bit 0: Wait ALL */
+#define NXEVENT_WAIT_RESET   (1 << 1) /* Bit 1: Reset events before wait */
+#define NXEVENT_WAIT_NOCLEAR (1 << 2) /* Bit 2: Do not clear events after wait 
*/

Review Comment:
   add blank line



##########
sched/event/event_post.c:
##########
@@ -0,0 +1,162 @@
+/****************************************************************************
+ * sched/event/event_post.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/sched.h>
+
+#include "event.h"
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nxevent_sem_post
+ ****************************************************************************/
+
+static inline_function int nxevent_sem_post(FAR sem_t *sem)
+{
+  int semcount;
+
+  nxsem_get_value(sem, &semcount);
+  if (semcount < 1)
+    {
+      return nxsem_post(sem);
+    }
+
+  return 0;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nxevent_post
+ *
+ * Description:
+ *   Post one or more events to an event object.
+ *
+ *   This routine posts one or more events to an event object. All tasks
+ *   waiting on the event object event whose waiting conditions become
+ *   met by this posting immediately unpend.
+ *
+ *   Posting differs from setting in that posted events are merged together
+ *   with the current set of events tracked by the event object.
+ *
+ * Input Parameters:
+ *   event  - Address of the event object
+ *   events - Set of events to post to event
+ *          - Set events to 0 will be considered as any,
+ *            waking up the waiting thread immediately.
+ *
+ * Returned Value:
+ *   This is an internal OS interface and should not be used by applications.
+ *   It follows the NuttX internal error return policy:  Zero (OK) is
+ *   returned on success.  A negated errno value is returned on failure.
+ *
+ * Assumptions:
+ *   This function may be called from an interrupt handler.
+ *
+ ****************************************************************************/
+
+int nxevent_post(FAR nxevent_t *event, nxevent_mask_t events,
+                 nxevent_flags_t eflags)
+{
+  nxevent_mask_t clear = 0;
+  FAR nxevent_wait_t *wait;
+  FAR nxevent_wait_t *tmp;
+  irqstate_t flags;
+  bool waitall;
+  bool postall;
+  int ret = 0;
+
+  if (event == NULL)
+    {
+      return -EINVAL;
+    }
+
+  if (events == 0)
+    {
+      events = ~0;
+    }
+
+  flags = enter_critical_section();
+
+  event->events |= events;
+
+  if (!list_is_empty(&event->list))
+    {
+      postall = ((eflags & NXEVENT_POST_ALL) != 0);
+
+      /* Hold schedule lock here to avoid context switch if post high
+       * priority task.
+       */
+
+      sched_lock();
+
+      list_for_every_entry_safe(&event->list, wait, tmp,
+                                nxevent_wait_t, node)
+        {
+          waitall = ((wait->eflags & NXEVENT_WAIT_ALL) != 0);
+
+          if ((!waitall && ((wait->expect & event->events) != 0)) ||
+              (waitall && ((wait->expect & event->events) == wait->expect)))
+            {
+              list_delete(&wait->node);
+
+              if (!waitall)
+                {
+                  wait->expect &= event->events;
+                }
+
+              if (!postall && (wait->eflags & NXEVENT_WAIT_NOCLEAR) == 0)
+                {
+                  clear |= wait->expect;
+                }
+
+              ret = nxevent_sem_post(&wait->sem);
+              if (ret >= 0 && (event->events & ~clear) == 0)
+                {
+                  break;
+                }
+            }
+        }
+
+      sched_unlock();
+
+      if (clear)
+        {
+          event->events &= ~clear;
+        }
+    }
+
+  if ((eflags & NXEVENT_POST_SET) != 0)

Review Comment:
   merge to line 106



##########
sched/event/event_init.c:
##########
@@ -0,0 +1,55 @@
+/****************************************************************************
+ * sched/event/event_init.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 "event.h"
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: nxevent_init
+ *
+ * Description:
+ *   This routine initializes an event object, Set of default events to post
+ *   to event.
+ *
+ * Input Parameters:
+ *   event  - Address of the event object
+ *   events - Set of events to post to event
+ *
+ * Returned Value:
+ *   This is an internal OS interface and should not be used by applications.
+ *   It follows the NuttX internal error return policy:  Zero (OK) is
+ *   returned on success.  A negated errno value is returned on failure.
+ *
+ ****************************************************************************/
+
+int nxevent_init(FAR nxevent_t *event, nxevent_mask_t events)

Review Comment:
   ```suggestion
   void nxevent_init(FAR nxevent_t *event, nxevent_mask_t events)
   ```



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