Jinoh Kang (@iamahuman) commented about dlls/win32u/winstation.c:
+ } +} + +static NTSTATUS map_shared_session_section( struct shared_session *session, HANDLE handle ) +{ + NTSTATUS status; + + while (!(status = NtMapViewOfSection( handle, GetCurrentProcess(), (void **)&session->shared, 0, 0, + NULL, &session->size, ViewUnmap, 0, PAGE_READONLY ))) + { + BOOL invalid; + + SHARED_READ_BEGIN( session->shared, session_shm_t ) + { + if ((invalid = session->size != shared->size)) break; + session->id = shared->obj.id; `break`-ing out of macro-internal do-while(0) is confusing and unprecedented[^1][^2]. If I see `break`, my eyes go to the nearest ancestor while/for/do/switch, not some odd-looking Wine-specific macro that I don't necessarily have intimate knowledge of.
I don't think people[^3] are supposed to *have* to understand that `SHARED_READ_BEGIN` is a do-while loop in disguise, just to make sense of this control flow. ```suggestion:-1+0 if (!(invalid = session->size != shared->size)) session->id = shared->obj.id; ``` ```suggestion:-1+0 if ((invalid = session->size != shared->size)) goto invalid; session->id = shared->obj.id; ``` ```suggestion:-1+0 if ((invalid = session->size != shared->size)) SHARED_READ_GOTO_BLOCK_EXIT(); // Custom macro session->id = shared->obj.id; ``` [^1]: Excluding perhaps `break` directly *inside* a switch, which still doesn't exit the implicit do-while loop, or `return`, which doesn't change its jump target whether the macro-internal do-while loop exists around it or not. [^2]: I could be wrong; please feel free to correct me if this is not the case. [^3]: Who wants to focus on the bug/feature at hand and doesn't necessarily care how `SHARED_READ_BEGIN` works -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3103#note_62518