On Fri, Jun 15, 2012 at 07:01:34PM +1000, Christopher James Halse Rogers wrote: > xwayland drivers need access to their screen private data to authenticate. > Now that drivers no longer have direct access to the global screen arrays, > this needs to be passed in as function context. The way it was working > was ugly, anyway.
Yes, sure, but it also didn't need to extend the DRI2 API. Now that Dave's gone and locked up the screen arrays, we have a good excuse to go clean that up. > Signed-off-by: Christopher James Halse Rogers > <[email protected]> > --- > > This came up when I got around to fixing the nouveau xwayland patch review > comments; with things no longer meant to access the global xf86Screens array > the authmagic callback needed some other way to access driver private data. > > Nouveau patch using this follows; I'll send it upstream once this gets > applied somewhere. > > CCd to xorg-devel mainly for sanity review, but could be applied; as Airlied > has pointed out, there's no particular reason for the xwayland branch to > diverge on core infrastructure. Yeah, this is good to go upstream. Just add a new function pointer to the struct and a new typedef though, no need to play games with casting the pointers: typedef int (*DRI2AuthMagicWithScreenProcPtr) (ScreenPtr pScreen, int fd, uint32_t magic); or something. Aside from that and the small nit-pick below, it looks good. Kristian > > hw/xfree86/dri2/dri2.c | 40 ++++++++++++++++++++++++++++++++++------ > hw/xfree86/dri2/dri2.h | 2 +- > 2 files changed, 35 insertions(+), 7 deletions(-) > > diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c > index babf32f..0e79875 100644 > --- a/hw/xfree86/dri2/dri2.c > +++ b/hw/xfree86/dri2/dri2.c > @@ -89,6 +89,8 @@ typedef struct _DRI2Drawable { > Bool needInvalidate; > } DRI2DrawableRec, *DRI2DrawablePtr; > > +typedef int (*DRI2LegacyAuthMagicProcPtr) (int fd, uint32_t magic); > + > typedef struct _DRI2Screen { > ScreenPtr screen; > int refcnt; > @@ -105,6 +107,7 @@ typedef struct _DRI2Screen { > DRI2GetMSCProcPtr GetMSC; > DRI2ScheduleWaitMSCProcPtr ScheduleWaitMSC; > DRI2AuthMagicProcPtr AuthMagic; > + DRI2LegacyAuthMagicProcPtr LegacyAuthMagic; > DRI2ReuseBufferNotifyProcPtr ReuseBufferNotify; > DRI2SwapLimitValidateProcPtr SwapLimitValidate; > DRI2GetParamProcPtr GetParam; > @@ -1110,12 +1113,22 @@ DRI2Connect(ScreenPtr pScreen, unsigned int > driverType, int *fd, > return TRUE; > } > > +static Bool > +DRI2AuthMagic (ScreenPtr pScreen, int fd, uint32_t magic) > +{ > + DRI2ScreenPtr ds = DRI2GetScreen(pScreen); > + if (ds == NULL || (*ds->LegacyAuthMagic) (ds->fd, magic)) > + return FALSE; Don't need to check ds == NULL here, we're only called when ds is non-NULL. > + return TRUE; > +} > + > Bool > DRI2Authenticate(ScreenPtr pScreen, uint32_t magic) > { > DRI2ScreenPtr ds = DRI2GetScreen(pScreen); > > - if (ds == NULL || (*ds->AuthMagic) (ds->fd, magic)) > + if (ds == NULL || (*ds->AuthMagic) (pScreen, ds->fd, magic)) > return FALSE; > > return TRUE; > @@ -1202,9 +1215,17 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info) > cur_minor = 1; > } > > - if (info->version >= 5) { > + if (info->version >= 7) { > ds->AuthMagic = info->AuthMagic; > } > + else if (info->version >= 5) { > + /* > + * This cast is safe; if the driver has provided a V5 or V6 InfoRec > + * then AuthMagic is of type DRI2LegacyAuthMagicProcPtr, and the C > + * standard guarantees that we can typecast it back and call it. > + */ > + ds->LegacyAuthMagic = (DRI2LegacyAuthMagicProcPtr)info->AuthMagic; > + } > > if (info->version >= 6) { > ds->ReuseBufferNotify = info->ReuseBufferNotify; > @@ -1218,14 +1239,21 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info) > > /* > * if the driver doesn't provide an AuthMagic function or the info struct > - * version is too low, it relies on the old method (using libdrm) or fail > + * version is too low, call through LegacyAuthMagic > */ > - if (!ds->AuthMagic) > + if (!ds->AuthMagic) { > + ds->AuthMagic = DRI2AuthMagic; > + /* > + * If the driver doesn't provide an AuthMagic function > + * it relies on the old method (using libdrm) or fails > + */ > + if (!ds->LegacyAuthMagic) > #ifdef WITH_LIBDRM > - ds->AuthMagic = drmAuthMagic; > + ds->LegacyAuthMagic = drmAuthMagic; > #else > - goto err_out; > + goto err_out; > #endif > + } > > /* Initialize minor if needed and set to minimum provied by DDX */ > if (!dri2_minor || dri2_minor > cur_minor) > diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h > index f849be6..90886a9 100644 > --- a/hw/xfree86/dri2/dri2.h > +++ b/hw/xfree86/dri2/dri2.h > @@ -63,7 +63,7 @@ typedef void (*DRI2CopyRegionProcPtr) (DrawablePtr pDraw, > DRI2BufferPtr pDestBuffer, > DRI2BufferPtr pSrcBuffer); > typedef void (*DRI2WaitProcPtr) (WindowPtr pWin, unsigned int sequence); > -typedef int (*DRI2AuthMagicProcPtr) (int fd, uint32_t magic); > +typedef int (*DRI2AuthMagicProcPtr) (ScreenPtr pScreen, int fd, uint32_t > magic); > > /** > * Schedule a buffer swap > -- > 1.7.10.4 > > _______________________________________________ > wayland-devel mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
