Index: /trunk/src/VBox/Devices/Parallel/DrvHostParallel.cpp
===================================================================
--- /trunk/src/VBox/Devices/Parallel/DrvHostParallel.cpp	(revision 40657)
+++ /trunk/src/VBox/Devices/Parallel/DrvHostParallel.cpp	(revision 40658)
@@ -107,4 +107,5 @@
     /** Device Handle */
     RTFILE                        hFileDevice;
+#ifndef VBOX_WITH_WIN_PARPORT_SUP       /** @todo r=bird: Eliminate thing not needed, it rules out things you might have missed. */
     /** Thread waiting for interrupts. */
     PPDMTHREAD                    pMonitorThread;
@@ -113,4 +114,5 @@
     /** Wakeup pipe write end. */
     RTPIPE                        hWakeupPipeW;
+#endif
     /** Current mode the parallel port is in. */
     PDMPARALLELPORTMODE           enmModeCur;
@@ -130,5 +132,5 @@
     /** Whether the parallel port is available or not. */
     bool                          fParportAvail;
-# ifdef IN_RING0
+# ifdef IN_RING0 /** @todo r=bird: This isn't needed and will not work as you always need to declare all structure members in all contexts. (Size will differer between ring-3 and ring-0 now, meaning you'll corrupt heap because ring-3 does the alloc.) */
     typedef struct DEVICE_EXTENSION
     {
@@ -141,5 +143,5 @@
     PDEVICE_EXTENSION             pDevExtn;
 # endif
-#endif
+#endif /* VBOX_WITH_WIN_PARPORT_SUP */
 } DRVHOSTPARALLEL, *PDRVHOSTPARALLEL;
 
@@ -180,5 +182,5 @@
  *
  */
-PDMBOTHCBDECL(int) drvR0HostParallelReqWrite(PPDMDRVINS pDrvIns, uint64_t u64Arg)
+static int drvR0HostParallelReqWrite(PPDMDRVINS pDrvIns, uint64_t u64Arg) /** @todo r=bird: PDMBOTHCBDECL is only required for entry points. Everything which doesn't need to be globally visible shall be static! */
 {
     PDRVHOSTPARALLEL pThis = PDMINS_2_DATA(pDrvIns, PDRVHOSTPARALLEL);
@@ -195,5 +197,5 @@
  * @param   u64Arg      Data to be written to control register.
  */
-PDMBOTHCBDECL(int) drvR0HostParallelReqWriteControl(PPDMDRVINS pDrvIns, uint64_t u64Arg)
+static int drvR0HostParallelReqWriteControl(PPDMDRVINS pDrvIns, uint64_t u64Arg)
 {
     PDRVHOSTPARALLEL pThis = PDMINS_2_DATA(pDrvIns, PDRVHOSTPARALLEL);
@@ -210,5 +212,5 @@
  * @param   u64Arg     Not used.
  */
-PDMBOTHCBDECL(int) drvR0HostParallelReqRead(PPDMDRVINS pDrvIns, uint64_t u64Arg)
+static int drvR0HostParallelReqRead(PPDMDRVINS pDrvIns, uint64_t u64Arg)
 {
     uint8_t u8Data;
@@ -227,5 +229,5 @@
  * @param   u64Arg     Not used.
  */
-PDMBOTHCBDECL(int) drvR0HostParallelReqReadControl(PPDMDRVINS pDrvIns, uint64_t u64Arg)
+static int drvR0HostParallelReqReadControl(PPDMDRVINS pDrvIns, uint64_t u64Arg)
 {
     uint8_t u8Data;
@@ -244,5 +246,5 @@
  * @param   u64Arg     Not used.
  */
-PDMBOTHCBDECL(int) drvR0HostParallelReqReadStatus(PPDMDRVINS pDrvIns, uint64_t u64Arg)
+static int drvR0HostParallelReqReadStatus(PPDMDRVINS pDrvIns, uint64_t u64Arg)
 {
     uint8_t u8Data;
@@ -261,9 +263,9 @@
  * @param   u64Arg     Mode.
  */
-PDMBOTHCBDECL(int) drvR0HostParallelReqSetPortDir(PPDMDRVINS pDrvIns, uint64_t u64Arg)
-{
+static int drvR0HostParallelReqSetPortDir(PPDMDRVINS pDrvIns, uint64_t u64Arg)
+{
+    PDRVHOSTPARALLEL pThis = PDMINS_2_DATA(pDrvIns, PDRVHOSTPARALLEL);
     uint8_t u8ReadControlVal;
     uint8_t u8WriteControlVal;
-    PDRVHOSTPARALLEL pThis = PDMINS_2_DATA(pDrvIns, PDRVHOSTPARALLEL);
 
     if (u64Arg)
@@ -316,16 +318,16 @@
     return rc;
 }
-#endif /**IN_RING0*/
-#endif /**VBOX_WITH_WIN_PARPORT_SUP*/
+#endif /* IN_RING0 */
+#endif /* VBOX_WITH_WIN_PARPORT_SUP */
 
 #ifdef IN_RING3
-#ifdef VBOX_WITH_WIN_PARPORT_SUP
-/**
- * Find IO port range for the parallel port and return the lower
- * address.
+# ifdef VBOX_WITH_WIN_PARPORT_SUP
+/**
+ * Find IO port range for the parallel port and return the lower address.
+ *
  * @returns parallel port IO address.
  * @param   DevInst    Device Instance for parallel port.
  */
-static uint32_t FindIORangeResource(const DEVINST DevInst)
+static uint32_t drvHostWinFindIORangeResource(const DEVINST DevInst) /** @todo r=bird: Prefix methods like in the rest of the file, since windows specific, throw in a 'Win'. */
 {
     uint8_t  *pBuf  = NULL;
@@ -356,14 +358,14 @@
     {
         u32Size = 0;
-        cmRet = CM_Get_Res_Des_Data_Size((PULONG)(&u32Size), nextLogConf, 0L);
+        cmRet = CM_Get_Res_Des_Data_Size((PULONG)(&u32Size), nextLogConf, 0L); /** @todo r=bird: (PULONG)(&u32Size) - generally a bad idea, but not really a problem here though... Why don't you use ULONG for the size variable? Like 'ULONG cbBuf;'? */
         if (cmRet != CR_SUCCESS)
         {
-            CM_Free_Res_Des_Handle(nextLogConf);
+            CM_Free_Res_Des_Handle(nextLogConf); /** @todo r=bird: Why are you doing this twice in this code path? */
             break;
         }
-        pBuf = (uint8_t *)((char*)RTMemAlloc(u32Size + 1));
+        pBuf = (uint8_t *)RTMemAlloc(u32Size + 1);
         if (!pBuf)
         {
-            CM_Free_Res_Des_Handle(nextLogConf);
+            CM_Free_Res_Des_Handle(nextLogConf); /** @todo r=bird: Ditto above. */
             break;
         }
@@ -372,9 +374,9 @@
         {
             CM_Free_Res_Des_Handle(nextLogConf);
-            LocalFree(pBuf);
+            RTMemFree(pBuf);            /** @todo r=bird: LocalFree(pBuf) was the wrong free function! */
             break;
         }
         LogFlowFunc(("call GetIOResource\n"));
-        u32ParportAddr = ((IO_DES*)(pBuf))->IOD_Alloc_Base;
+        u32ParportAddr = ((IO_DES *)pBuf)->IOD_Alloc_Base;
         LogFlowFunc(("called GetIOResource, ret=%#x\n", u32ParportAddr));
         rdPrevResDes = 0;
@@ -387,9 +389,8 @@
         if (cmRet != CR_SUCCESS)
            break;
-        else
-        {
-            CM_Free_Res_Des_Handle(nextLogConf);
-            nextLogConf = rdPrevResDes;
-        }
+        /** @todo r=bird: No need to else soemthing when the 'then' clause
+         *        returned or broke ouf of the loop. */
+        CM_Free_Res_Des_Handle(nextLogConf);
+        nextLogConf = rdPrevResDes;
     }
     CM_Free_Res_Des_Handle(nextLogConf);
@@ -432,5 +433,6 @@
                      RTMemFree(pBuf);
                 /* Max size will never be more than 2048 bytes */
-                pBuf = (uint8_t *)((char*)RTMemAlloc(u32BufSize * 2));
+                /** @todo r=bird: Too many parentheses and casts: (uint8_t *)((char*)RTMemAlloc(u32BufSize * 2)) */
+                pBuf = (uint8_t *)RTMemAlloc(u32BufSize * 2);
             }
             else
@@ -438,5 +440,5 @@
         }
 
-        if (pBuf)
+        if (pBuf) /** @todo r=bird: You're not checking errors here. */
         {
              LogFlowFunc(("device name=%s\n", pBuf));
@@ -444,5 +446,5 @@
              {
                  LogFlowFunc(("found parallel port\n"));
-                 u32ParportAddr = FindIORangeResource(DeviceInfoData.DevInst);
+                 u32ParportAddr = drvHostWinFindIORangeResource(DeviceInfoData.DevInst);
                  if (u32ParportAddr)
                  {
@@ -472,5 +474,5 @@
 
 }
-#endif /**VBOX_WITH_WIN_PARPORT_SUP*/
+# endif /* VBOX_WITH_WIN_PARPORT_SUP */
 
 /**
@@ -487,5 +489,5 @@
     LogFlowFunc(("mode=%d\n", enmMode));
 
-#ifndef VBOX_WITH_WIN_PARPORT_SUP
+# ifndef VBOX_WITH_WIN_PARPORT_SUP
     int rcLnx;
     if (pThis->enmModeCur != enmMode)
@@ -516,7 +518,7 @@
 
     return rc;
-#else /* VBOX_WITH_WIN_PARPORT_SUP */
+# else  /* VBOX_WITH_WIN_PARPORT_SUP */
     return VINF_SUCCESS;
-#endif /* VBOX_WITH_WIN_PARPORT_SUP */
+# endif /* VBOX_WITH_WIN_PARPORT_SUP */
 }
 
@@ -553,5 +555,5 @@
     if (RT_FAILURE(rc))
         return rc;
-#ifndef VBOX_WITH_WIN_PARPORT_SUP
+# ifndef VBOX_WITH_WIN_PARPORT_SUP
     if (enmMode == PDM_PARALLEL_PORT_MODE_SPP)
     {
@@ -566,5 +568,5 @@
     if (RT_UNLIKELY(rcLnx < 0))
         rc = RTErrConvertFromErrno(errno);
-#else /*VBOX_WITH_WIN_PARPORT_SUP*/
+# else /* VBOX_WITH_WIN_PARPORT_SUP */
     /** @todo r=klaus this code assumes cbWrite==1, which may not be guaranteed forever */
     uint64_t u64Data;
@@ -576,5 +578,5 @@
         AssertRC(rc);
     }
-#endif /* VBOX_WITH_WIN_PARPORT_SUP */
+# endif /* VBOX_WITH_WIN_PARPORT_SUP */
     return rc;
 }
@@ -588,5 +590,5 @@
     int rc = VINF_SUCCESS;
 
-#ifndef VBOX_WITH_WIN_PARPORT_SUP
+# ifndef VBOX_WITH_WIN_PARPORT_SUP
     int rcLnx = 0;
     LogFlowFunc(("pvBuf=%#p cbRead=%d\n", pvBuf, cbRead));
@@ -608,5 +610,5 @@
     if (RT_UNLIKELY(rcLnx < 0))
         rc = RTErrConvertFromErrno(errno);
-#else /* VBOX_WITH_WIN_PARPORT_SUP */
+# else  /* VBOX_WITH_WIN_PARPORT_SUP */
     /** @todo r=klaus this code assumes cbRead==1, which may not be guaranteed forever */
     *((uint8_t*)(pvBuf)) = 0; /* Initialize the buffer. */
@@ -616,7 +618,7 @@
         int rc = PDMDrvHlpCallR0(pThis->CTX_SUFF(pDrvIns), DRVHOSTPARALLELR0OP_READ, 0);
         AssertRC(rc);
-        *((uint8_t*)(pvBuf)) = (uint8_t)pThis->u8ReadIn;
-    }
-#endif
+        *(uint8_t *)pvBuf = (uint8_t)pThis->u8ReadIn; /** r=bird: *((uint8_t *)(pvBuf)) is too many parentheses. Heaping them up make the code harder to read. Please check C++ operator precedence rules. */
+    }
+# endif /* VBOX_WITH_WIN_PARPORT_SUP */
     return rc;
 }
@@ -626,14 +628,14 @@
     PDRVHOSTPARALLEL    pThis   = RT_FROM_MEMBER(pInterface, DRVHOSTPARALLEL, CTX_SUFF(IHostParallelConnector));
     int rc = VINF_SUCCESS;
-    int rcLnx = 0;
-    int iMode = 0;
 
     if (!fForward)
         iMode = 1;
-#ifndef VBOX_WITH_WIN_PARPORT_SUP
-    rcLnx = ioctl(RTFileToNative(pThis->hFileDevice), PPDATADIR, &iMode);
+# ifndef VBOX_WITH_WIN_PARPORT_SUP
+    int iMode = 0;                      /** @todo r=bird: unused . */
+    int rcLnx = ioctl(RTFileToNative(pThis->hFileDevice), PPDATADIR, &iMode);
     if (RT_UNLIKELY(rcLnx < 0))
         rc = RTErrConvertFromErrno(errno);
-#else /* VBOX_WITH_WIN_PARPORT_SUP */
+
+# else /* VBOX_WITH_WIN_PARPORT_SUP */
     uint64_t u64Data;
     u64Data = (uint8_t)iMode;
@@ -644,5 +646,5 @@
         AssertRC(rc);
     }
-#endif /* VBOX_WITH_WIN_PARPORT_SUP */
+# endif /* VBOX_WITH_WIN_PARPORT_SUP */
     return rc;
 }
@@ -662,5 +664,5 @@
     if (RT_UNLIKELY(rcLnx < 0))
         rc = RTErrConvertFromErrno(errno);
-#else /* VBOX_WITH_WIN_PARPORT_SUP */
+# else /* VBOX_WITH_WIN_PARPORT_SUP */
     uint64_t u64Data;
     u64Data = (uint8_t)fReg;
@@ -671,5 +673,5 @@
         AssertRC(rc);
     }
-#endif /* VBOX_WITH_WIN_PARPORT_SUP */
+# endif /* VBOX_WITH_WIN_PARPORT_SUP */
     return rc;
 }
@@ -686,5 +688,5 @@
     uint8_t fReg = 0;
 
-#ifndef VBOX_WITH_WIN_PARPORT_SUP
+# ifndef VBOX_WITH_WIN_PARPORT_SUP
     rcLnx = ioctl(RTFileToNative(pThis->hFileDevice), PPRCONTROL, &fReg);
     if (RT_UNLIKELY(rcLnx < 0))
@@ -695,5 +697,5 @@
         *pfReg = fReg;
     }
-#else /* VBOX_WITH_WIN_PARPORT_SUP */
+# else /* VBOX_WITH_WIN_PARPORT_SUP */
     *pfReg = 0; /* Initialize the buffer*/
     if (pThis->fParportAvail)
@@ -704,5 +706,5 @@
         *pfReg = pThis->u8ReadInControl;
     }
-#endif /* VBOX_WITH_WIN_PARPORT_SUP */
+# endif /* VBOX_WITH_WIN_PARPORT_SUP */
     return rc;
 }
@@ -717,5 +719,5 @@
     int rcLnx = 0;
     uint8_t fReg = 0;
-#ifndef  VBOX_WITH_WIN_PARPORT_SUP
+# ifndef  VBOX_WITH_WIN_PARPORT_SUP
     rcLnx = ioctl(RTFileToNative(pThis->hFileDevice), PPRSTATUS, &fReg);
     if (RT_UNLIKELY(rcLnx < 0))
@@ -726,5 +728,5 @@
         *pfReg = fReg;
     }
-#else /* VBOX_WITH_WIN_PARPORT_SUP */
+# else /* VBOX_WITH_WIN_PARPORT_SUP */
     *pfReg = 0; /* Intialize the buffer. */
     if (pThis->fParportAvail)
@@ -735,13 +737,13 @@
         *pfReg = pThis->u8ReadInStatus;
     }
-#endif /* VBOX_WITH_WIN_PARPORT_SUP */
+# endif /* VBOX_WITH_WIN_PARPORT_SUP */
     return rc;
 }
 
+# ifndef VBOX_WITH_WIN_PARPORT_SUP /** @todo r=bird: This whole function is unused in the direct access path. */
+
 static DECLCALLBACK(int) drvHostParallelMonitorThread(PPDMDRVINS pDrvIns, PPDMTHREAD pThread)
 {
     PDRVHOSTPARALLEL pThis = PDMINS_2_DATA(pDrvIns, PDRVHOSTPARALLEL);
-
-#ifndef VBOX_WITH_WIN_PARPORT_SUP
     struct pollfd aFDs[2];
 
@@ -783,5 +785,4 @@
         AssertRC(rc);
     }
-#endif /* VBOX_WITH_WIN_PARPORT_SUP */
 
     return VINF_SUCCESS;
@@ -801,4 +802,6 @@
     return RTPipeWrite(pThis->hWakeupPipeW, "", 1, &cbIgnored);
 }
+
+# endif /* VBOX_WITH_WIN_PARPORT_SUP */
 
 /**
@@ -829,5 +832,5 @@
     pThis->hWakeupPipeR = NIL_RTPIPE;
 
-    rc = RTFileClose(pThis->hFileDevice); AssertRC(rc);
+    rc = RTFileClose(pThis->hFileDevice); AssertRC(rc); /** @todo r=bird: Why aren't this closed on Windows? */
     pThis->hFileDevice = NIL_RTFILE;
 
@@ -858,6 +861,8 @@
      */
     pThis->hFileDevice  = NIL_RTFILE;
+#ifndef VBOX_WITH_WIN_PARPORT_SUP
     pThis->hWakeupPipeR = NIL_RTPIPE;
     pThis->hWakeupPipeW = NIL_RTPIPE;
+#endif
 
     pThis->pDrvInsR3                                = pDrvIns;
@@ -960,9 +965,10 @@
     rc = drvHostParallelGetParportAddr(pThis);
     return rc;
+    /**@todo r=bird: Just remove or #if 0 this code.*/
     HANDLE hPort;
     /** @todo r=klaus convert to IPRT */
     hPort = CreateFile("LPT1", GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ,
-		NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
-    if (INVALID_HANDLE_VALUE == hPort)
+                       NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); /** @todo r=bird: Continuation indent under start parentheses like the rest of the file. */
+    if (hPort == INVALID_HANDLE_VALUE) /** @todo r=bird: Please, variable == constant, not the other way around. Just learn to not make accidental assignments in conditionals! */
     {
         /** @todo r=klaus probably worth a nicely formatted release log entry,