Skip to content

(master) Xephyr: consistently use scrpriv->mynum for debug logging - #3716

Open
metux wants to merge 1 commit into
masterfrom
pr/master-xephyr-consistently-use-scrpriv-mynum-for-debug-logging-_2026-09-22_14-39-49
Open

metux wants to merge 1 commit into
masterfrom
pr/master-xephyr-consistently-use-scrpriv-mynum-for-debug-logging-_2026-09-22_14-39-49

Conversation

@metux

@metux metux commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

The debug logging is inconsistent with which screen number to print out in
(usually disabled) debug log messages. For now both pScreen->myNum and
scrnpriv->mynum are the same - but in the future they might become different.

Signed-off-by: Enrico Weigelt, metux IT consult info@metux.net

@metux metux self-assigned this Sep 22, 2026
@metux
metux requested a review from a team September 22, 2026 12:41
@metux

metux commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Automated review — generated by Starfleet ship Enterprise (model: heavy-model) on behalf of @metux. Not a human review.

Verdict: changes-requested — blocking compile error: ScreenRec has no member driver.

The problem

The PR replaces pScreen->myNum with scrpriv->mynum / pScreen->driver->mynum in the debug
logs. The first hunk is fine (scrpriv->mynum, where EphyrScrPriv.mynum exists at
hw/kdrive/ephyr/ephyr.h:84), but the motion/button hunks dereference a non-existent member:

screen->pScreen->driver->mynum        /* ephyr.c:1047, 1061, 1083, 1095, 1112 */
ephyrCursorScreen->driver->mynum      /* ephyr.c:1054 */
  • screen is KdScreenInfo * (KdScreenInfo.pScreen is ScreenPtr, hw/kdrive/src/kdrive.h:93).
  • struct _Screen (include/scrnintstr.h) has no driver member — only myNum, devPrivate, etc.
  • Verified with offsetof(ScreenRec, driver) → GCC: “struct _Screen has no member named ‘driver’”.
  • ephyrCursorScreen is a ScreenPtr too, so ->driver is equally invalid.

So this does not compile whenever Xephyr is built (-Dxephyr=true). The currently-green CI
lanes (macos/freebsd/openbsd/dragonflybsd/arch/cygwin/…) do not build Xephyr; the ubuntu lane,
which sets -Dxephyr=true, is still pending — the error just hasn't been surfaced yet.

Suggested fix (matches the commit's intent)

  • For screen (KdScreenInfo *): use the ephyr scrpriv the author means —
    ((EphyrScrPriv *) screen->driver)->mynum — or simply screen->mynum (KdScreenInfo.mynum,
    kdrive.h), which is the kdrive screen number.
  • For the ScreenPtr ephyrCursorScreen: go through the kdrive screen priv —
    KdGetScreenPriv(ephyrCursorScreen)->screen->mynum (with the existing ?: -1 null handling) —
    or, if a consistent X-level number is acceptable, keep ephyrCursorScreen->myNum.

Other rules

  • Backport: N/A — debug-log-only refactor, targets master.
  • Driver-ABI: no impact (no struct changes, no _X_EXPORT symbol touched; debug logging only).
  • abi-changes-check passes, as expected.

Happy to re-review once the ->driver dereferences are fixed.

@metux metux added the bot-review-changes-requested Automated bot review requested changes (blocking finding) label Sep 22, 2026
The debug logging is inconsistent with which screen number to print out in
(usually disabled) debug log messages. For now both pScreen->myNum and
scrnpriv->mynum are the same - but in the future they might become different.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
@metux
metux force-pushed the pr/master-xephyr-consistently-use-scrpriv-mynum-for-debug-logging-_2026-09-22_14-39-49 branch from e3b11de to 6f6e95c Compare September 23, 2026 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot-review-changes-requested Automated bot review requested changes (blocking finding)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant