Maybe in 1/4 specify from which repository (with url) 850c0516 came?

Sure.

However I would like to have a consistent style. As I can see you
are planning to do a lot of changes on this "dummy" driver so I would
prefer to change all style to a more SPICE one. I can see at least 4
different styles for just the "if".

Well, I think the long term plan is to persuade the Xorg guys to accept this driver in one form or another. (Perhaps as a modification to the dummy driver itself). My thought was to have it start in x11spice, demonstrate a viable purpose for it, and then propose to move it to the xorg tree in some form.

Henri tends to follow the Wine rules on style - don't change what it is already there. It's not a lot of code, so it would be easy to make it follow SPICE style guidelines for now.


Fine with me to keep the 1/4 as an exact copy of the original driver.
Have you also not considered coming out with a proper name instead of
"spicedummy"? I don't know... x11spice for instance could suit.

I don't know that I feel strongly about naming, but x11spice would be a mistake, IMHO. x11spice has a specific meaning right now, and I'd rather not muddy that.

The rationale for the present name is that 'xf86-video-dummy' is the Xorg name for the replacement for Xvfb. So in Xorg parlance, a headless server uses the 'dummy' driver. So sharing that name, in theory, makes the purpose of the driver clear.

Cheers,

Jeremy



I don't feel strongly about it, if you feel that condensing them would
be better, I'm happy to spin them that way.

Cheers,

Jeremy



---
   spice-video-dummy/src/dummy.h        |  2 +-
   spice-video-dummy/src/dummy_cursor.c |  8 ++---
   spice-video-dummy/src/dummy_driver.c | 58
   ++++++++++++++++++------------------
   3 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/spice-video-dummy/src/dummy.h b/spice-video-dummy/src/dummy.h
index 8e7c43b..09cd917 100644
--- a/spice-video-dummy/src/dummy.h
+++ b/spice-video-dummy/src/dummy.h
@@ -36,7 +36,7 @@ typedef struct _color
       int blue;
   } dummy_colors;
-typedef struct dummyRec
+typedef struct dummyRec
   {
       /* options */
       OptionInfoPtr Options;
diff --git a/spice-video-dummy/src/dummy_cursor.c
b/spice-video-dummy/src/dummy_cursor.c
index d7c67c6..9241d24 100644
--- a/spice-video-dummy/src/dummy_cursor.c
+++ b/spice-video-dummy/src/dummy_cursor.c
@@ -17,7 +17,7 @@ dummyShowCursor(ScrnInfoPtr pScrn)
       DUMMYPtr dPtr = DUMMYPTR(pScrn);
/* turn cursor on */
-    dPtr->DummyHWCursorShown = TRUE;
+    dPtr->DummyHWCursorShown = TRUE;
   }
static void
@@ -26,7 +26,7 @@ dummyHideCursor(ScrnInfoPtr pScrn)
       DUMMYPtr dPtr = DUMMYPTR(pScrn);
/*
-     * turn cursor off
+     * turn cursor off
        *
        */
       dPtr->DummyHWCursorShown = FALSE;
@@ -47,7 +47,7 @@ static void
   dummySetCursorColors(ScrnInfoPtr pScrn, int bg, int fg)
   {
       DUMMYPtr dPtr = DUMMYPTR(pScrn);
-
+
       dPtr->cursorFG = fg;
       dPtr->cursorBG = bg;
   }
@@ -94,7 +94,7 @@ DUMMYCursorInit(ScreenPtr pScreen)
       infoPtr->ShowCursor = dummyShowCursor;
       infoPtr->UseHWCursor = dummyUseHWCursor;
   /*     infoPtr->RealizeCursor = dummyRealizeCursor; */
-
+
       return(xf86InitCursor(pScreen, infoPtr));
   }
diff --git a/spice-video-dummy/src/dummy_driver.c
b/spice-video-dummy/src/dummy_driver.c
index b4b42f7..9e29fe7 100644
--- a/spice-video-dummy/src/dummy_driver.c
+++ b/spice-video-dummy/src/dummy_driver.c
@@ -232,7 +232,7 @@ DUMMYProbe(DriverPtr drv, int flags)
for (i = 0; i < numUsed; i++) {
            ScrnInfoPtr pScrn = NULL;
-           int entityIndex =
+           int entityIndex =
                xf86ClaimNoSlot(drv,DUMMY_CHIP,devSections[i],TRUE);
            /* Allocate a ScrnInfoRec and claim the slot */
            if ((pScrn = xf86AllocateScreen(drv,0 ))) {
@@ -253,7 +253,7 @@ DUMMYProbe(DriverPtr drv, int flags)
                    foundScreen = TRUE;
            }
        }
-    }
+    }
free(devSections); @@ -275,21 +275,21 @@ DUMMYPreInit(ScrnInfoPtr pScrn, int flags)
       int maxClock = 300000;
       GDevPtr device = xf86GetEntityInfo(pScrn->entityList[0])->device;
        
-    if (flags & PROBE_DETECT)
+    if (flags & PROBE_DETECT)
        return TRUE;
-
+

Here there are no brackets

       /* Allocate the DummyRec driverPrivate */
       if (!DUMMYGetRec(pScrn)) {
        return FALSE;
       }

Here there are.
Also the spacing is not consistent. Spaces and tabs are mixed.

Which coding style are these files?

-
+
       dPtr = DUMMYPTR(pScrn);
pScrn->chipset = (char *)xf86TokenToString(DUMMYChipsets,
                                               DUMMY_CHIP);
xf86DrvMsg(pScrn->scrnIndex, X_INFO, "Chipset is a DUMMY\n");
-
+
       pScrn->monitor = pScrn->confScreen->monitor;
if (!xf86SetDepthBpp(pScrn, 0, 0, 0, Support24bppFb |
       Support32bppFb))
@@ -335,7 +335,7 @@ DUMMYPreInit(ScrnInfoPtr pScrn, int flags)
        }
       }
- if (!xf86SetDefaultVisual(pScrn, -1))
+    if (!xf86SetDefaultVisual(pScrn, -1))
        return FALSE;
if (pScrn->depth > 1) {
@@ -364,7 +364,7 @@ DUMMYPreInit(ScrnInfoPtr pScrn, int flags)
        xf86DrvMsg(pScrn->scrnIndex, X_PROBED, "VideoRAM: %d kByte\n",
                   pScrn->videoRam);
       }
-
+
       if (device->dacSpeeds[0] != 0) {
        maxClock = device->dacSpeeds[0];
        xf86DrvMsg(pScrn->scrnIndex, X_CONFIG, "Max Clock: %d kHz\n",
@@ -385,7 +385,7 @@ DUMMYPreInit(ScrnInfoPtr pScrn, int flags)
       clockRanges->minClock = 11000;   /* guessed ยงยงยง */
       clockRanges->maxClock = maxClock;
       clockRanges->clockIndex = -1;         /* programmable */
-    clockRanges->interlaceAllowed = TRUE;
+    clockRanges->interlaceAllowed = TRUE;
       clockRanges->doubleScanAllowed = TRUE;
/* Subtract memory for HW cursor */
@@ -421,8 +421,8 @@ DUMMYPreInit(ScrnInfoPtr pScrn, int flags)
        * driver and if the driver doesn't provide code to set them.  They
        * are not pre-initialised at all.
        */
-    xf86SetCrtcForModes(pScrn, 0);
-
+    xf86SetCrtcForModes(pScrn, 0);
+
       /* Set the current mode to the first in the list */
       pScrn->currentMode = pScrn->modes;
@@ -440,7 +440,7 @@ DUMMYPreInit(ScrnInfoPtr pScrn, int flags)
        if (!xf86LoadSubModule(pScrn, "ramdac"))
            RETURN;
       }
-
+
       /* We have no contiguous physical fb in physical memory */
       pScrn->memPhysBase = 0;
       pScrn->fbOffset = 0;
@@ -474,11 +474,11 @@ DUMMYLoadPalette(
      DUMMYPtr dPtr = DUMMYPTR(pScrn);
switch(pScrn->depth) {
-   case 15:
+   case 15:
        shift = Gshift = 1;
        break;
      case 16:
-       shift = 0;
+       shift = 0;
           Gshift = 0;
        break;
      default:
@@ -491,7 +491,7 @@ DUMMYLoadPalette(
          dPtr->colors[index].red = colors[index].red << shift;
          dPtr->colors[index].green = colors[index].green << Gshift;
          dPtr->colors[index].blue = colors[index].blue << shift;
-   }
+   }
} @@ -523,9 +523,9 @@ DUMMYScreenInit(SCREEN_INIT_ARGS_DECL)
        * Reset visual list.
        */
       miClearVisualTypes();
-
+
       /* Setup the visuals we support. */
-
+
       if (!miSetVisualTypes(pScrn->depth,
                              miGetDefaultVisualMask(pScrn->depth),
                      pScrn->rgbBits, pScrn->defaultVisual))
@@ -558,7 +558,7 @@ DUMMYScreenInit(SCREEN_INIT_ARGS_DECL)
            }
        }
       }
-
+
       /* must be after RGB ordering fixed */
       fbPictureInit(pScreen, 0, 0);
@@ -569,7 +569,7 @@ DUMMYScreenInit(SCREEN_INIT_ARGS_DECL) { -
+
        BoxRec AvailFBArea;
        int lines = pScrn->videoRam * 1024 /
            (pScrn->displayWidth * (pScrn->bitsPerPixel >> 3));
@@ -577,16 +577,16 @@ DUMMYScreenInit(SCREEN_INIT_ARGS_DECL)
        AvailFBArea.y1 = 0;
        AvailFBArea.x2 = pScrn->displayWidth;
        AvailFBArea.y2 = lines;
-       xf86InitFBManager(pScreen, &AvailFBArea);
-
-       xf86DrvMsg(pScrn->scrnIndex, X_INFO,
+       xf86InitFBManager(pScreen, &AvailFBArea);
+
+       xf86DrvMsg(pScrn->scrnIndex, X_INFO,
                   "Using %i scanlines of offscreen memory \n"
                   , lines - pScrn->virtualY);
       }
xf86SetBackingStore(pScreen);
       xf86SetSilkenMouse(pScreen);
-
+
       /* Initialise cursor functions */
       miDCInitialize (pScreen, xf86GetPointerScreenFuncs());
@@ -599,20 +599,20 @@ DUMMYScreenInit(SCREEN_INIT_ARGS_DECL)
          return FALSE;
         }
       }
-
+
       /* Initialise default colourmap */
       if(!miCreateDefColormap(pScreen))
        return FALSE;
if (!xf86HandleColormaps(pScreen, 1024, pScrn->rgbBits,
-                         DUMMYLoadPalette, NULL,
-                         CMAP_PALETTED_TRUECOLOR
+                         DUMMYLoadPalette, NULL,
+                         CMAP_PALETTED_TRUECOLOR
                             | CMAP_RELOAD_ON_MODE_SWITCH))
        return FALSE;
pScreen->SaveScreen = DUMMYSaveScreen; -
+
       /* Wrap the current CloseScreen function */
       dPtr->CloseScreen = pScreen->CloseScreen;
       pScreen->CloseScreen = DUMMYCloseScreen;
@@ -698,7 +698,7 @@ DUMMYCreateWindow(WindowPtr pWin)
if(ret != TRUE)

No space after if, this also is not consistent in these files.

        return(ret);

Parenthesis around value while others have no parenthesis.

-
+
       if(dPtr->prop == FALSE) {
   #if GET_ABI_MAJOR(ABI_VIDEODRV_VERSION) < 8
           pWinRoot = WindowTable[DUMMYScrn->pScreen->myNum];
@@ -728,7 +728,7 @@ static Bool
   dummyDriverFunc(ScrnInfoPtr pScrn, xorgDriverFuncOp op, pointer ptr)
   {
       CARD32 *flag;
-
+
       switch (op) {
        case GET_REQUIRED_HW_INTERFACES:
            flag = (CARD32*)ptr;

Frediano




_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Reply via email to