Changeset 31025 in vbox

Timestamp:

Jul 22, 2010 11:43:25 PM (15 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

63945

Message:

Quick VbglR3SharedFolder* api review.

Location:

trunk

Files:

• include/VBox/VBoxGuestLib.h (modified) (2 diffs)
• src/VBox/Additions/common/VBoxControl/VBoxControl.cpp (modified) (3 diffs)
• src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3LibSharedFolders.cpp (modified) (6 diffs)

trunk/include/VBox/VBoxGuestLib.h

@@ -514 +514 @@
  * Structure containing mapping information for a shared folder.
  */
-struct VBGLR3SHAREDFOLDERMAPPING
+typedef struct VBGLR3SHAREDFOLDERMAPPING
 {
     /** Mapping status. */
@@ -520 +520 @@
     /** Root handle. */
     uint32_t u32Root;
-};
+} VBGLR3SHAREDFOLDERMAPPING;
+/** Pointer to a shared folder mapping information struct. */
 typedef VBGLR3SHAREDFOLDERMAPPING *PVBGLR3SHAREDFOLDERMAPPING;
-/** @todo Docs. */
+
 VBGLR3DECL(int)     VbglR3SharedFolderConnect(uint32_t *pu32ClientId);
 VBGLR3DECL(int)     VbglR3SharedFolderDisconnect(uint32_t u32ClientId);
-VBGLR3DECL(int)     VbglR3SharedFolderGetMappings(uint32_t                   u32ClientId,  bool      bAutoMountOnly,
-                                                  VBGLR3SHAREDFOLDERMAPPING  paMappings[], uint32_t  cbMappings,
-                                                  uint32_t                  *pcMapCount);
-VBGLR3DECL(int)     VbglR3SharedFolderGetName(uint32_t   u32ClientId,uint32_t  u32Root,
-                                              char     **ppszName,   uint32_t *pcbLen);
+VBGLR3DECL(int)     VbglR3SharedFolderGetMappings(uint32_t u32ClientId, bool fAutoMountOnly,
+                                                  PVBGLR3SHAREDFOLDERMAPPING paMappings, uint32_t cbMappings,
+                                                  uint32_t *pcMappings);
+VBGLR3DECL(int)     VbglR3SharedFolderGetName(uint32_t  u32ClientId,uint32_t u32Root, char **ppszName);
 /** @}  */
 # endif /* VBOX_WITH_SHARED_FOLDERS defined */

trunk/src/VBox/Additions/common/VBoxControl/VBoxControl.cpp

@@ -1298 +1298 @@
         uint32_t cMappings = 64; /* See shflsvc.h for define; should be used later. */
         uint32_t cbMappings = cMappings * sizeof(VBGLR3SHAREDFOLDERMAPPING);
-        VBGLR3SHAREDFOLDERMAPPING *pMappings = (VBGLR3SHAREDFOLDERMAPPING*)RTMemAlloc(cbMappings);
-
-        if (pMappings)
+        VBGLR3SHAREDFOLDERMAPPING *paMappings = (PVBGLR3SHAREDFOLDERMAPPING)RTMemAlloc(cbMappings);
+
+        if (paMappings)
         {
             rc = VbglR3SharedFolderGetMappings(u32ClientId, fOnlyShowAutoMount,
-                                               pMappings, cbMappings,
+                                               paMappings, cbMappings,
                                                &cMappings);
             if (RT_SUCCESS(rc))
@@ -1311 +1311 @@
                 for (uint32_t i = 0; i < cMappings; i++)
                 {
-                    char *ppszName = NULL;
-                    uint32_t pcbLen = 0;
-                    rc = VbglR3SharedFolderGetName(u32ClientId, pMappings[i].u32Root,
-                                                   &ppszName, &pcbLen);
+                    char *pszName;
+                    rc = VbglR3SharedFolderGetName(u32ClientId, paMappings[i].u32Root, &pszName);
                     if (RT_SUCCESS(rc))
                     {
-                        RTPrintf("%02u - %s\n", i + 1, ppszName);
-                        RTStrFree(ppszName);
+                        RTPrintf("%02u - %s\n", i + 1, pszName);
+                        RTStrFree(pszName);
                     }
                     else
                         VBoxControlError("Error while getting the shared folder name for root node = %u, rc = %Rrc\n",
-                                         pMappings[i].u32Root, rc);
+                                         paMappings[i].u32Root, rc);
                 }
                 if (cMappings == 0)
@@ -1329 +1327 @@
             else
                 VBoxControlError("Error while getting the shared folder mappings, rc = %Rrc\n", rc);
-            RTMemFree(pMappings);
+            RTMemFree(paMappings);
         }
         else

trunk/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3LibSharedFolders.cpp

@@ -38 +38 @@
 
 #include "VBGLR3Internal.h"
+
 
 /**
@@ -90 +91 @@
  * @returns VBox status code.
  * @param   u32ClientId     The client id returned by VbglR3InvsSvcConnect().
- * @param   bAutoMountOnly  Flag whether only auto-mounted shared folders should be reported.
+ * @param   fAutoMountOnly  Flag whether only auto-mounted shared folders
+ *                          should be reported.
  * @param   paMappings      Pointer to a preallocated array which will retrieve the mapping info.
  * @param   cbMappings      Size (in bytes) of the provided array.
- * @param   pcMapCount      Number of mappings returned.
+ * @param   pcMappings      On input, the size of @a paMappings gives as an
+ *                          item count.  On output, the number of mappings
+ *                          returned in @a paMappings.
+ *
+ * @todo    r=bird: cbMappings and @a *pcMappings overlap.  The better API
+ *          would be to change cbMappings to cMappings (the entries are fixed
+ *          sized) and move the move the input aspect of @a *pcMappings to it.
+ *
+ *          However, it would be better if this function would do the array
+ *          allocation.  This way you could deal with too-much-data conditions
+ *          here (or hide the max-number-of-shared-folders-per-vm-define).
+ *          Then paMappings would become ppaMappings and cbMappings could be
+ *          removed altogether. *pcMappings would only be output.  A
+ *          corresponding VbglR3SharedFolderFreeMappings would be required for
+ *          a 100% clean API (this is an (/going to be) offical API for C/C++
+ *          programs).
  */
-VBGLR3DECL(int) VbglR3SharedFolderGetMappings(uint32_t                   u32ClientId,  bool      bAutoMountOnly,
-                                              VBGLR3SHAREDFOLDERMAPPING  paMappings[], uint32_t  cbMappings,
-                                              uint32_t                  *pcMapCount)
+VBGLR3DECL(int) VbglR3SharedFolderGetMappings(uint32_t u32ClientId, bool fAutoMountOnly,
+                                              PVBGLR3SHAREDFOLDERMAPPING paMappings, uint32_t cbMappings,
+                                              uint32_t *pcMappings)
 {
-    int rc;
-
-    AssertPtr(pcMapCount);
+    AssertPtr(pcMappings);
 
     VBoxSFQueryMappings Msg;
@@ -111 +126 @@
 
     /* Set the mapping flags. */
-    uint32_t u32Flags = 0; /* @todo SHFL_MF_UTF8 is not implemented yet. */
-    if (bAutoMountOnly) /* We only want the mappings which get auto-mounted. */
+    uint32_t u32Flags = 0; /** @todo SHFL_MF_UTF8 is not implemented yet. */
+    if (fAutoMountOnly) /* We only want the mappings which get auto-mounted. */
         u32Flags |= SHFL_MF_AUTOMOUNT;
     VbglHGCMParmUInt32Set(&Msg.flags, u32Flags);
 
     /* Init the rest of the message. */
-    VbglHGCMParmUInt32Set(&Msg.numberOfMappings, *pcMapCount);
+    VbglHGCMParmUInt32Set(&Msg.numberOfMappings, *pcMappings);
     VbglHGCMParmPtrSet(&Msg.mappings, &paMappings[0], cbMappings);
 
-    rc = vbglR3DoIOCtl(VBOXGUEST_IOCTL_HGCM_CALL(sizeof(Msg)), &Msg, sizeof(Msg));
+    int rc = vbglR3DoIOCtl(VBOXGUEST_IOCTL_HGCM_CALL(sizeof(Msg)), &Msg, sizeof(Msg));
     if (RT_SUCCESS(rc))
     {
-        VbglHGCMParmUInt32Get(&Msg.numberOfMappings, pcMapCount);
+        VbglHGCMParmUInt32Get(&Msg.numberOfMappings, pcMappings);
         rc = Msg.callInfo.result;
     }
@@ -136 +151 @@
  * @param   u32ClientId     The client id returned by VbglR3InvsSvcConnect().
  * @param   u32Root         Root ID of shared folder to get the name for.
- * @param   ppszName        Name of the shared folder.
- * @param   pcbLen          Length (in bytes) of shared folder name.
+ * @param   ppszName        Where to return the name string.  This shall be
+ *                          freed by calling RTStrFree.
  */
-VBGLR3DECL(int) VbglR3SharedFolderGetName(uint32_t   u32ClientId, uint32_t  u32Root,
-                                          char     **ppszName,    uint32_t *pcbLen)
+VBGLR3DECL(int) VbglR3SharedFolderGetName(uint32_t u32ClientId, uint32_t u32Root, char **ppszName)
 {
-    int rc;
-
     AssertPtr(ppszName);
-    AssertPtr(pcbLen);
 
     VBoxSFQueryMapName Msg;
@@ -154 +165 @@
     Msg.callInfo.cParms = 2;
 
-    uint32_t cbString = sizeof(SHFLSTRING) + SHFL_MAX_LEN;
+    int         rc;
+    uint32_t    cbString = sizeof(SHFLSTRING) + SHFL_MAX_LEN;
     PSHFLSTRING pString = (PSHFLSTRING)RTMemAlloc(cbString);
     if (pString)
@@ -168 +180 @@
         {
             *ppszName = NULL;
-            rc = RTUtf16ToUtf8Ex((PCRTUTF16)&pString->String.ucs2, RTSTR_MAX,
-                                 ppszName, (size_t)pcbLen, NULL);
+            rc = RTUtf16ToUtf8(&pString->String.ucs2[0], ppszName);
             if (RT_SUCCESS(rc))
-                rc = Msg.callInfo.result;
+                rc = Msg.callInfo.result; /** @todo r=bird: Shouldn't you check this *before* doing the conversion? */
         }
         RTMemFree(pString);