gdalsnes@svn.reactos.com wrote:
*Modified: trunk/reactos/include/reactos/helper.h*
--- trunk/reactos/include/reactos/helper.h 2005-09-26 22:06:31 UTC (rev 18112) +++ trunk/reactos/include/reactos/helper.h 2005-09-26 22:57:48 UTC (rev 18113) @@ -18,17 +18,21 @@
#define EXPORTED __declspec(dllexport) #define IMPORTED __declspec(dllimport)
-/* iterate through the list using a list entry */
+/* iterate through the list using a list entry.
- elem is set to NULL if the list is run thru without breaking out or if list is empty.
- */
#define LIST_FOR_EACH(elem, list, type, field) \ for ((elem) = CONTAINING_RECORD((list)->Flink, type, field); \
&(elem)->field != (list); \
&(elem)->field != (list) || (elem = NULL); \ (elem) = CONTAINING_RECORD((elem)->field.Flink, type, field))
Hi,
IMHO, this is very bad code. You use a macro to hide a 'for' loop, and later in the source you use a 'break', 'continue' or 'goto' statement to leave the loop or to bypass some code.
/* Find entry with SortKey greater than or equal to the passed-in SortKey */ LIST_FOR_EACH(ReturnEntry, &DeviceQueue->DeviceListHead, KDEVICE_QUEUE_ENTRY, DeviceListEntry) { /* Check if keys match */ if (ReturnEntry->SortKey >= SortKey) { /* We found it, so just remove it */ RemoveEntryList(&ReturnEntry->DeviceListEntry); break; } }
Such a source is simply unreadable.
- Hartmut