VirtualBox

Changeset 62845 in vbox for trunk/src/VBox


Ignore:
Timestamp:
Aug 1, 2016 8:18:42 PM (8 years ago)
Author:
vboxsync
Message:

vbglR3DnDHGRecvDataRaw: Added suggestion about dropping buggy pre-v3 protocol code, spotted while fixing complaints about variables (cbFormatRecv & cbDataRecv, only the former being spot on) being used uninitialized. Again, it looks like MSC 2010 isn't very clever wrt determining uninitialized variables, so no do-while-false-break loops of flat if (RT_SUCCESS(rc)) statements - sideways if pyramids only.

File:
1 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3LibDragAndDrop.cpp

    r62470 r62845  
    770770                                  void *pvData, uint32_t cbData, uint32_t *pcbDataRecv)
    771771{
    772     AssertPtrReturn(pCtx,     VERR_INVALID_POINTER);
    773     AssertPtrReturn(pDataHdr, VERR_INVALID_POINTER);
    774     AssertPtrReturn(pvData,   VERR_INVALID_POINTER);
    775     AssertReturn(cbData,      VERR_INVALID_PARAMETER);
    776     /* pcbDataRecv is optional. */
    777 
    778     int rc;
     772    AssertPtrReturn(pCtx,            VERR_INVALID_POINTER);
     773    AssertPtrReturn(pDataHdr,        VERR_INVALID_POINTER);
     774    AssertPtrReturn(pvData,          VERR_INVALID_POINTER);
     775    AssertReturn(cbData,             VERR_INVALID_PARAMETER);
     776    AssertPtrNullReturn(pcbDataRecv, VERR_INVALID_POINTER);
    779777
    780778    LogFlowFunc(("pvDate=%p, cbData=%RU32\n", pvData, cbData));
    … …  
    786784    Msg.hdr.u32Function = HOST_DND_HG_SND_DATA;
    787785
    788     do
    789     {
    790         uint32_t cbDataRecv;
    791 
    792         if (pCtx->uProtocol < 3)
    793         {
    794             Msg.hdr.cParms  = 5;
    795 
    796             Msg.u.v1.uScreenId.SetUInt32(0);
    797             Msg.u.v1.pvFormat.SetPtr(pDataHdr->pvMetaFmt, pDataHdr->cbMetaFmt);
    798             Msg.u.v1.cbFormat.SetUInt32(0);
    799             Msg.u.v1.pvData.SetPtr(pvData, cbData);
    800             Msg.u.v1.cbData.SetUInt32(0);
    801 
    802             rc = vbglR3DoIOCtl(VBOXGUEST_IOCTL_HGCM_CALL(sizeof(Msg)), &Msg, sizeof(Msg));
    803             if (RT_SUCCESS(rc))
    804             {
    805                 rc = Msg.hdr.result;
    806                 if (   RT_SUCCESS(rc)
    807                     || rc == VERR_BUFFER_OVERFLOW)
     786    int rc;
     787    if (pCtx->uProtocol < 3)
     788    {
     789        Msg.hdr.cParms  = 5;
     790
     791        Msg.u.v1.uScreenId.SetUInt32(0);
     792        Msg.u.v1.pvFormat.SetPtr(pDataHdr->pvMetaFmt, pDataHdr->cbMetaFmt);
     793        Msg.u.v1.cbFormat.SetUInt32(0);
     794        Msg.u.v1.pvData.SetPtr(pvData, cbData);
     795        Msg.u.v1.cbData.SetUInt32(0);
     796
     797        rc = vbglR3DoIOCtl(VBOXGUEST_IOCTL_HGCM_CALL(sizeof(Msg)), &Msg, sizeof(Msg));
     798        if (RT_SUCCESS(rc))
     799        {
     800            rc = Msg.hdr.result;
     801            if (   RT_SUCCESS(rc)
     802                || rc == VERR_BUFFER_OVERFLOW)
     803            {
     804                /** @todo r=bird: The VERR_BUFFER_OVERFLOW case is probably broken as the
     805                 *        status isn't returned to the caller (vbglR3DnDHGRecvDataLoop).
     806                 *        This was the case before fixing the uninitalized variable.  As
     807                 *        other V0-2 protocol functions have been marked deprecated, it's
     808                 *        probably a good idea to just remove this code and tell the 1-2 users
     809                 *        to upgrade the host instead.  Unused and untested weird code like this
     810                 *        is just hard+costly to maintain and liability.
     811                 *        (VERR_BUFFER_OVERFLOW == weird, no disrespect intended) */
     812
     813                /* Unmarshal the whole message first. */
     814                rc = Msg.u.v1.uScreenId.GetUInt32(&pDataHdr->uScreenId);
     815                AssertRC(rc);
     816                if (RT_SUCCESS(rc))
    808817                {
    809                     rc = Msg.u.v1.uScreenId.GetUInt32(&pDataHdr->uScreenId);
    810                     AssertRC(rc);
    811 
    812                     /*
    813                      * In case of VERR_BUFFER_OVERFLOW get the data sizes required
    814                      * for the format + data blocks.
    815                      */
    816818                    uint32_t cbFormatRecv;
    817819                    rc = Msg.u.v1.cbFormat.GetUInt32(&cbFormatRecv);
    818820                    AssertRC(rc);
    819                     if (cbFormatRecv >= pDataHdr->cbMetaFmt)
     821                    if (RT_SUCCESS(rc))
    820822                    {
    821                         rc = VERR_TOO_MUCH_DATA;
    822                         break;
     823                        uint32_t cbDataRecv;
     824                        rc = Msg.u.v1.cbData.GetUInt32(&cbDataRecv);
     825                        AssertRC(rc);
     826                        if (RT_SUCCESS(rc))
     827                        {
     828                            /*
     829                             * In case of VERR_BUFFER_OVERFLOW get the data sizes required
     830                             * for the format + data blocks.
     831                             */
     832                            if (   cbFormatRecv >= pDataHdr->cbMetaFmt
     833                                || cbDataRecv   >= pDataHdr->cbMeta)
     834                                rc = VERR_TOO_MUCH_DATA;
     835                            else
     836                            {
     837                                pDataHdr->cbMetaFmt = cbFormatRecv;
     838                                if (pcbDataRecv)
     839                                    *pcbDataRecv = cbDataRecv;
     840                                LogFlowFuncLeaveRC(rc);
     841                                return rc;
     842                            }
     843                        }
    823844                    }
    824 
    825                     rc = Msg.u.v1.cbData.GetUInt32(&cbDataRecv);
    826                     AssertRC(rc);
    827                     if (cbDataRecv >= pDataHdr->cbMeta)
    828                     {
    829                         rc = VERR_TOO_MUCH_DATA;
    830                         break;
    831                     }
    832 
    833                     pDataHdr->cbMetaFmt = cbFormatRecv;
    834845                }
    835846            }
    836 
    837             if (RT_FAILURE(rc))
    838                 break;
    839         }
    840         else /* Protocol v3 and up. */
    841         {
    842             Msg.hdr.cParms = 5;
    843 
    844             Msg.u.v3.uContext.SetUInt32(0);
    845             Msg.u.v3.pvData.SetPtr(pvData, cbData);
    846             Msg.u.v3.cbData.SetUInt32(0);
    847             Msg.u.v3.pvChecksum.SetPtr(NULL, 0);
    848             Msg.u.v3.cbChecksum.SetUInt32(0);
    849 
    850             rc = vbglR3DoIOCtl(VBOXGUEST_IOCTL_HGCM_CALL(sizeof(Msg)), &Msg, sizeof(Msg));
     847        }
     848    }
     849    else /* Protocol v3 and up. */
     850    {
     851        Msg.hdr.cParms = 5;
     852
     853        Msg.u.v3.uContext.SetUInt32(0);
     854        Msg.u.v3.pvData.SetPtr(pvData, cbData);
     855        Msg.u.v3.cbData.SetUInt32(0);
     856        Msg.u.v3.pvChecksum.SetPtr(NULL, 0);
     857        Msg.u.v3.cbChecksum.SetUInt32(0);
     858
     859        rc = vbglR3DoIOCtl(VBOXGUEST_IOCTL_HGCM_CALL(sizeof(Msg)), &Msg, sizeof(Msg));
     860        if (RT_SUCCESS(rc))
     861        {
     862            rc = Msg.hdr.result;
    851863            if (RT_SUCCESS(rc))
    852864            {
    853                 rc = Msg.hdr.result;
     865                uint32_t cbDataRecv;
     866                rc = Msg.u.v3.cbData.GetUInt32(&cbDataRecv);
     867                AssertRC(rc);
    854868                if (RT_SUCCESS(rc))
    855869                {
    856                     rc = Msg.u.v3.cbData.GetUInt32(&cbDataRecv);
    857                     AssertRC(rc);
     870                    /** @todo Use checksum for validating the received data. */
     871                    if (pcbDataRecv)
     872                        *pcbDataRecv = cbDataRecv;
     873                    LogFlowFuncLeaveRC(rc);
     874                    return rc;
    858875                }
    859 
    860                 /** @todo Use checksum for validating the received data. */
    861             }
    862 
    863             if (RT_FAILURE(rc))
    864                 break;
    865         }
    866 
    867         if (pcbDataRecv)
    868             *pcbDataRecv = cbDataRecv;
    869 
    870     } while (0);
    871 
     876            }
     877
     878        }
     879    }
     880
     881    /* failure */
    872882    LogFlowFuncLeaveRC(rc);
    873883    return rc;
    … …  
    925935
    926936/** @todo Deprecated function; will be removed. */
    927 static int vbglR3DnDHGRecvMoreData(PVBGLR3GUESTDNDCMDCTX pCtx,
    928                                    void *pvData, uint32_t cbData, uint32_t *pcbDataRecv)
     937static int vbglR3DnDHGRecvMoreData(PVBGLR3GUESTDNDCMDCTX pCtx, void *pvData, uint32_t cbData, uint32_t *pcbDataRecv)
    929938{
    930939    AssertPtrReturn(pCtx,        VERR_INVALID_POINTER);
    … …  
    978987            return VERR_NO_MEMORY;
    979988
    980         /**
     989        /*
    981990         * Protocols < v3 contain the header information in every HOST_DND_HG_SND_DATA
    982991         * message, so do the actual retrieving immediately.
    … …  
    14691478    AssertPtrReturn(pEvent, VERR_INVALID_POINTER);
    14701479
    1471     uint32_t       uMsg      = 0;
    1472     uint32_t       uNumParms = 0;
    1473 
    1474     const uint32_t cbDataMax   = pCtx->cbMaxChunkSize;
    14751480    const uint32_t cbFormatMax = pCtx->cbMaxChunkSize;
    14761481
    1477     int rc = vbglR3DnDGetNextMsgType(pCtx, &uMsg, &uNumParms, true /* fWait */);
     1482    uint32_t       uMsg   = 0;
     1483    uint32_t       cParms = 0;
     1484    int rc = vbglR3DnDGetNextMsgType(pCtx, &uMsg, &cParms, true /* fWait */);
    14781485    if (RT_SUCCESS(rc))
    14791486    {
Note: See TracChangeset for help on using the changeset viewer.

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