Re: [PATCH 0/2] qemu/queue.h: clear linked list pointers on remove

[Philippe Mathieu-Daudé]

On 2/24/20 12:39 PM, Stefan Hajnoczi wrote:
> On Mon, Feb 24, 2020 at 02:55:33AM -0800, address@hidden wrote:
> > === OUTPUT BEGIN ===
> > 1/2 Checking commit f913b2430ad3 (qemu/queue.h: clear linked list pointers on 
> > remove)
> > ERROR: do not use assignment in if condition
> > #65: FILE: include/qemu/queue.h:314:
> > +    if (((head)->sqh_first = elm->field.sqe_next) == NULL)              \
> >
> > total: 1 errors, 0 warnings, 59 lines checked
>
> The same pattern is used elsewhere in this file.  This code comes from
> BSD and doesn't comply with QEMU's coding style.

Checkpatch is right, assigning out of the if statement makes the review 
easier, and we can avoid the 'elm' null deref:

#define QSIMPLEQ_REMOVE_HEAD(head, field) do { \
-    if (((head)->sqh_first = (head)->sqh_first->field.sqe_next) == NULL)\
+    typeof((head)->sqh_first) elm = (head)->sqh_first; \
+    (head)->sqh_first = elm->field.sqe_next; \
+    if (elm == NULL) { \
         (head)->sqh_last = &(head)->sqh_first; \
+    } else { \
+        elm->field.sqe_next = NULL; \
+    } \
 } while (/*CONSTCOND*/0)