VirtualBox

Changeset 40658 in vbox


Ignore:
Timestamp:
Mar 27, 2012 10:22:07 AM (13 years ago)
Author:
vboxsync
svn:sync-xref-src-repo-rev:
77092
Message:

r=bird: comments. Hope it compiles...

File:
1 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/VBox/Devices/Parallel/DrvHostParallel.cpp

    r40639 r40658  
    107107    /** Device Handle */
    108108    RTFILE                        hFileDevice;
     109#ifndef VBOX_WITH_WIN_PARPORT_SUP       /** @todo r=bird: Eliminate thing not needed, it rules out things you might have missed. */
    109110    /** Thread waiting for interrupts. */
    110111    PPDMTHREAD                    pMonitorThread;
    … …  
    113114    /** Wakeup pipe write end. */
    114115    RTPIPE                        hWakeupPipeW;
     116#endif
    115117    /** Current mode the parallel port is in. */
    116118    PDMPARALLELPORTMODE           enmModeCur;
    … …  
    130132    /** Whether the parallel port is available or not. */
    131133    bool                          fParportAvail;
    132 # ifdef IN_RING0
     134# 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.) */
    133135    typedef struct DEVICE_EXTENSION
    134136    {
    … …  
    141143    PDEVICE_EXTENSION             pDevExtn;
    142144# endif
    143 #endif
     145#endif /* VBOX_WITH_WIN_PARPORT_SUP */
    144146} DRVHOSTPARALLEL, *PDRVHOSTPARALLEL;
    145147
    … …  
    180182 *
    181183 */
    182 PDMBOTHCBDECL(int) drvR0HostParallelReqWrite(PPDMDRVINS pDrvIns, uint64_t u64Arg)
     184static 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! */
    183185{
    184186    PDRVHOSTPARALLEL pThis = PDMINS_2_DATA(pDrvIns, PDRVHOSTPARALLEL);
    … …  
    195197 * @param   u64Arg      Data to be written to control register.
    196198 */
    197 PDMBOTHCBDECL(int) drvR0HostParallelReqWriteControl(PPDMDRVINS pDrvIns, uint64_t u64Arg)
     199static int drvR0HostParallelReqWriteControl(PPDMDRVINS pDrvIns, uint64_t u64Arg)
    198200{
    199201    PDRVHOSTPARALLEL pThis = PDMINS_2_DATA(pDrvIns, PDRVHOSTPARALLEL);
    … …  
    210212 * @param   u64Arg     Not used.
    211213 */
    212 PDMBOTHCBDECL(int) drvR0HostParallelReqRead(PPDMDRVINS pDrvIns, uint64_t u64Arg)
     214static int drvR0HostParallelReqRead(PPDMDRVINS pDrvIns, uint64_t u64Arg)
    213215{
    214216    uint8_t u8Data;
    … …  
    227229 * @param   u64Arg     Not used.
    228230 */
    229 PDMBOTHCBDECL(int) drvR0HostParallelReqReadControl(PPDMDRVINS pDrvIns, uint64_t u64Arg)
     231static int drvR0HostParallelReqReadControl(PPDMDRVINS pDrvIns, uint64_t u64Arg)
    230232{
    231233    uint8_t u8Data;
    … …  
    244246 * @param   u64Arg     Not used.
    245247 */
    246 PDMBOTHCBDECL(int) drvR0HostParallelReqReadStatus(PPDMDRVINS pDrvIns, uint64_t u64Arg)
     248static int drvR0HostParallelReqReadStatus(PPDMDRVINS pDrvIns, uint64_t u64Arg)
    247249{
    248250    uint8_t u8Data;
    … …  
    261263 * @param   u64Arg     Mode.
    262264 */
    263 PDMBOTHCBDECL(int) drvR0HostParallelReqSetPortDir(PPDMDRVINS pDrvIns, uint64_t u64Arg)
    264 {
     265static int drvR0HostParallelReqSetPortDir(PPDMDRVINS pDrvIns, uint64_t u64Arg)
     266{
     267    PDRVHOSTPARALLEL pThis = PDMINS_2_DATA(pDrvIns, PDRVHOSTPARALLEL);
    265268    uint8_t u8ReadControlVal;
    266269    uint8_t u8WriteControlVal;
    267     PDRVHOSTPARALLEL pThis = PDMINS_2_DATA(pDrvIns, PDRVHOSTPARALLEL);
    268270
    269271    if (u64Arg)
    … …  
    316318    return rc;
    317319}
    318 #endif /**IN_RING0*/
    319 #endif /**VBOX_WITH_WIN_PARPORT_SUP*/
     320#endif /* IN_RING0 */
     321#endif /* VBOX_WITH_WIN_PARPORT_SUP */
    320322
    321323#ifdef IN_RING3
    322 #ifdef VBOX_WITH_WIN_PARPORT_SUP
    323 /**
    324  * Find IO port range for the parallel port and return the lower
    325  * address.
     324# ifdef VBOX_WITH_WIN_PARPORT_SUP
     325/**
     326 * Find IO port range for the parallel port and return the lower address.
     327 *
    326328 * @returns parallel port IO address.
    327329 * @param   DevInst    Device Instance for parallel port.
    328330 */
    329 static uint32_t FindIORangeResource(const DEVINST DevInst)
     331static 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'. */
    330332{
    331333    uint8_t  *pBuf  = NULL;
    … …  
    356358    {
    357359        u32Size = 0;
    358         cmRet = CM_Get_Res_Des_Data_Size((PULONG)(&u32Size), nextLogConf, 0L);
     360        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;'? */
    359361        if (cmRet != CR_SUCCESS)
    360362        {
    361             CM_Free_Res_Des_Handle(nextLogConf);
     363            CM_Free_Res_Des_Handle(nextLogConf); /** @todo r=bird: Why are you doing this twice in this code path? */
    362364            break;
    363365        }
    364         pBuf = (uint8_t *)((char*)RTMemAlloc(u32Size + 1));
     366        pBuf = (uint8_t *)RTMemAlloc(u32Size + 1);
    365367        if (!pBuf)
    366368        {
    367             CM_Free_Res_Des_Handle(nextLogConf);
     369            CM_Free_Res_Des_Handle(nextLogConf); /** @todo r=bird: Ditto above. */
    368370            break;
    369371        }
    … …  
    372374        {
    373375            CM_Free_Res_Des_Handle(nextLogConf);
    374             LocalFree(pBuf);
     376            RTMemFree(pBuf);            /** @todo r=bird: LocalFree(pBuf) was the wrong free function! */
    375377            break;
    376378        }
    377379        LogFlowFunc(("call GetIOResource\n"));
    378         u32ParportAddr = ((IO_DES*)(pBuf))->IOD_Alloc_Base;
     380        u32ParportAddr = ((IO_DES *)pBuf)->IOD_Alloc_Base;
    379381        LogFlowFunc(("called GetIOResource, ret=%#x\n", u32ParportAddr));
    380382        rdPrevResDes = 0;
    … …  
    387389        if (cmRet != CR_SUCCESS)
    388390           break;
    389         else
    390         {
    391             CM_Free_Res_Des_Handle(nextLogConf);
    392             nextLogConf = rdPrevResDes;
    393         }
     391        /** @todo r=bird: No need to else soemthing when the 'then' clause
     392         *        returned or broke ouf of the loop. */
     393        CM_Free_Res_Des_Handle(nextLogConf);
     394        nextLogConf = rdPrevResDes;
    394395    }
    395396    CM_Free_Res_Des_Handle(nextLogConf);
    … …  
    432433                     RTMemFree(pBuf);
    433434                /* Max size will never be more than 2048 bytes */
    434                 pBuf = (uint8_t *)((char*)RTMemAlloc(u32BufSize * 2));
     435                /** @todo r=bird: Too many parentheses and casts: (uint8_t *)((char*)RTMemAlloc(u32BufSize * 2)) */
     436                pBuf = (uint8_t *)RTMemAlloc(u32BufSize * 2);
    435437            }
    436438            else
    … …  
    438440        }
    439441
    440         if (pBuf)
     442        if (pBuf) /** @todo r=bird: You're not checking errors here. */
    441443        {
    442444             LogFlowFunc(("device name=%s\n", pBuf));
    … …  
    444446             {
    445447                 LogFlowFunc(("found parallel port\n"));
    446                  u32ParportAddr = FindIORangeResource(DeviceInfoData.DevInst);
     448                 u32ParportAddr = drvHostWinFindIORangeResource(DeviceInfoData.DevInst);
    447449                 if (u32ParportAddr)
    448450                 {
    … …  
    472474
    473475}
    474 #endif /**VBOX_WITH_WIN_PARPORT_SUP*/
     476# endif /* VBOX_WITH_WIN_PARPORT_SUP */
    475477
    476478/**
    … …  
    487489    LogFlowFunc(("mode=%d\n", enmMode));
    488490
    489 #ifndef VBOX_WITH_WIN_PARPORT_SUP
     491# ifndef VBOX_WITH_WIN_PARPORT_SUP
    490492    int rcLnx;
    491493    if (pThis->enmModeCur != enmMode)
    … …  
    516518
    517519    return rc;
    518 #else /* VBOX_WITH_WIN_PARPORT_SUP */
     520# else /* VBOX_WITH_WIN_PARPORT_SUP */
    519521    return VINF_SUCCESS;
    520 #endif /* VBOX_WITH_WIN_PARPORT_SUP */
     522# endif /* VBOX_WITH_WIN_PARPORT_SUP */
    521523}
    522524
    … …  
    553555    if (RT_FAILURE(rc))
    554556        return rc;
    555 #ifndef VBOX_WITH_WIN_PARPORT_SUP
     557# ifndef VBOX_WITH_WIN_PARPORT_SUP
    556558    if (enmMode == PDM_PARALLEL_PORT_MODE_SPP)
    557559    {
    … …  
    566568    if (RT_UNLIKELY(rcLnx < 0))
    567569        rc = RTErrConvertFromErrno(errno);
    568 #else /*VBOX_WITH_WIN_PARPORT_SUP*/
     570# else /* VBOX_WITH_WIN_PARPORT_SUP */
    569571    /** @todo r=klaus this code assumes cbWrite==1, which may not be guaranteed forever */
    570572    uint64_t u64Data;
    … …  
    576578        AssertRC(rc);
    577579    }
    578 #endif /* VBOX_WITH_WIN_PARPORT_SUP */
     580# endif /* VBOX_WITH_WIN_PARPORT_SUP */
    579581    return rc;
    580582}
    … …  
    588590    int rc = VINF_SUCCESS;
    589591
    590 #ifndef VBOX_WITH_WIN_PARPORT_SUP
     592# ifndef VBOX_WITH_WIN_PARPORT_SUP
    591593    int rcLnx = 0;
    592594    LogFlowFunc(("pvBuf=%#p cbRead=%d\n", pvBuf, cbRead));
    … …  
    608610    if (RT_UNLIKELY(rcLnx < 0))
    609611        rc = RTErrConvertFromErrno(errno);
    610 #else /* VBOX_WITH_WIN_PARPORT_SUP */
     612# else /* VBOX_WITH_WIN_PARPORT_SUP */
    611613    /** @todo r=klaus this code assumes cbRead==1, which may not be guaranteed forever */
    612614    *((uint8_t*)(pvBuf)) = 0; /* Initialize the buffer. */
    … …  
    616618        int rc = PDMDrvHlpCallR0(pThis->CTX_SUFF(pDrvIns), DRVHOSTPARALLELR0OP_READ, 0);
    617619        AssertRC(rc);
    618         *((uint8_t*)(pvBuf)) = (uint8_t)pThis->u8ReadIn;
    619     }
    620 #endif
     620        *(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. */
     621    }
     622# endif /* VBOX_WITH_WIN_PARPORT_SUP */
    621623    return rc;
    622624}
    … …  
    626628    PDRVHOSTPARALLEL    pThis   = RT_FROM_MEMBER(pInterface, DRVHOSTPARALLEL, CTX_SUFF(IHostParallelConnector));
    627629    int rc = VINF_SUCCESS;
    628     int rcLnx = 0;
    629     int iMode = 0;
    630630
    631631    if (!fForward)
    632632        iMode = 1;
    633 #ifndef VBOX_WITH_WIN_PARPORT_SUP
    634     rcLnx = ioctl(RTFileToNative(pThis->hFileDevice), PPDATADIR, &iMode);
     633# ifndef VBOX_WITH_WIN_PARPORT_SUP
     634    int iMode = 0;                      /** @todo r=bird: unused . */
     635    int rcLnx = ioctl(RTFileToNative(pThis->hFileDevice), PPDATADIR, &iMode);
    635636    if (RT_UNLIKELY(rcLnx < 0))
    636637        rc = RTErrConvertFromErrno(errno);
    637 #else /* VBOX_WITH_WIN_PARPORT_SUP */
     638
     639# else /* VBOX_WITH_WIN_PARPORT_SUP */
    638640    uint64_t u64Data;
    639641    u64Data = (uint8_t)iMode;
    … …  
    644646        AssertRC(rc);
    645647    }
    646 #endif /* VBOX_WITH_WIN_PARPORT_SUP */
     648# endif /* VBOX_WITH_WIN_PARPORT_SUP */
    647649    return rc;
    648650}
    … …  
    662664    if (RT_UNLIKELY(rcLnx < 0))
    663665        rc = RTErrConvertFromErrno(errno);
    664 #else /* VBOX_WITH_WIN_PARPORT_SUP */
     666# else /* VBOX_WITH_WIN_PARPORT_SUP */
    665667    uint64_t u64Data;
    666668    u64Data = (uint8_t)fReg;
    … …  
    671673        AssertRC(rc);
    672674    }
    673 #endif /* VBOX_WITH_WIN_PARPORT_SUP */
     675# endif /* VBOX_WITH_WIN_PARPORT_SUP */
    674676    return rc;
    675677}
    … …  
    686688    uint8_t fReg = 0;
    687689
    688 #ifndef VBOX_WITH_WIN_PARPORT_SUP
     690# ifndef VBOX_WITH_WIN_PARPORT_SUP
    689691    rcLnx = ioctl(RTFileToNative(pThis->hFileDevice), PPRCONTROL, &fReg);
    690692    if (RT_UNLIKELY(rcLnx < 0))
    … …  
    695697        *pfReg = fReg;
    696698    }
    697 #else /* VBOX_WITH_WIN_PARPORT_SUP */
     699# else /* VBOX_WITH_WIN_PARPORT_SUP */
    698700    *pfReg = 0; /* Initialize the buffer*/
    699701    if (pThis->fParportAvail)
    … …  
    704706        *pfReg = pThis->u8ReadInControl;
    705707    }
    706 #endif /* VBOX_WITH_WIN_PARPORT_SUP */
     708# endif /* VBOX_WITH_WIN_PARPORT_SUP */
    707709    return rc;
    708710}
    … …  
    717719    int rcLnx = 0;
    718720    uint8_t fReg = 0;
    719 #ifndef  VBOX_WITH_WIN_PARPORT_SUP
     721# ifndef  VBOX_WITH_WIN_PARPORT_SUP
    720722    rcLnx = ioctl(RTFileToNative(pThis->hFileDevice), PPRSTATUS, &fReg);
    721723    if (RT_UNLIKELY(rcLnx < 0))
    … …  
    726728        *pfReg = fReg;
    727729    }
    728 #else /* VBOX_WITH_WIN_PARPORT_SUP */
     730# else /* VBOX_WITH_WIN_PARPORT_SUP */
    729731    *pfReg = 0; /* Intialize the buffer. */
    730732    if (pThis->fParportAvail)
    … …  
    735737        *pfReg = pThis->u8ReadInStatus;
    736738    }
    737 #endif /* VBOX_WITH_WIN_PARPORT_SUP */
     739# endif /* VBOX_WITH_WIN_PARPORT_SUP */
    738740    return rc;
    739741}
    740742
     743# ifndef VBOX_WITH_WIN_PARPORT_SUP /** @todo r=bird: This whole function is unused in the direct access path. */
     744
    741745static DECLCALLBACK(int) drvHostParallelMonitorThread(PPDMDRVINS pDrvIns, PPDMTHREAD pThread)
    742746{
    743747    PDRVHOSTPARALLEL pThis = PDMINS_2_DATA(pDrvIns, PDRVHOSTPARALLEL);
    744 
    745 #ifndef VBOX_WITH_WIN_PARPORT_SUP
    746748    struct pollfd aFDs[2];
    747749
    … …  
    783785        AssertRC(rc);
    784786    }
    785 #endif /* VBOX_WITH_WIN_PARPORT_SUP */
    786787
    787788    return VINF_SUCCESS;
    … …  
    801802    return RTPipeWrite(pThis->hWakeupPipeW, "", 1, &cbIgnored);
    802803}
     804
     805# endif /* VBOX_WITH_WIN_PARPORT_SUP */
    803806
    804807/**
    … …  
    829832    pThis->hWakeupPipeR = NIL_RTPIPE;
    830833
    831     rc = RTFileClose(pThis->hFileDevice); AssertRC(rc);
     834    rc = RTFileClose(pThis->hFileDevice); AssertRC(rc); /** @todo r=bird: Why aren't this closed on Windows? */
    832835    pThis->hFileDevice = NIL_RTFILE;
    833836
    … …  
    858861     */
    859862    pThis->hFileDevice  = NIL_RTFILE;
     863#ifndef VBOX_WITH_WIN_PARPORT_SUP
    860864    pThis->hWakeupPipeR = NIL_RTPIPE;
    861865    pThis->hWakeupPipeW = NIL_RTPIPE;
     866#endif
    862867
    863868    pThis->pDrvInsR3                                = pDrvIns;
    … …  
    960965    rc = drvHostParallelGetParportAddr(pThis);
    961966    return rc;
     967    /**@todo r=bird: Just remove or #if 0 this code.*/
    962968    HANDLE hPort;
    963969    /** @todo r=klaus convert to IPRT */
    964970    hPort = CreateFile("LPT1", GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ,
    965                 NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
    966     if (INVALID_HANDLE_VALUE == hPort)
     971                       NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); /** @todo r=bird: Continuation indent under start parentheses like the rest of the file. */
     972    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! */
    967973    {
    968974        /** @todo r=klaus probably worth a nicely formatted release log entry,
Note: See TracChangeset for help on using the changeset viewer.

© 2025 Oracle Support Privacy / Do Not Sell My Info Terms of Use Trademark Policy Automated Access Etiquette