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