HGFS: Clean up server of the reply packet handling

The reply packet handler was returning a bool when in fact it cannot
fail. Fix this up and make it consistent with other calls for the reply
packet handler when filling the header in the complete request common
function.

This is a lot of repeating the same change for all the opcodes of each
supported request and version.

Signed-off-by: Dmitry Torokhov <dtor@vmware.com>

diff --git a/open-vm-tools/lib/hgfsServer/hgfsServer.c b/open-vm-tools/lib/hgfsServer/hgfsServer.c
index 6dbaf425f191bd7fd5d1f285cac3e2307ecf6984..3ff5b43643b51d35f96a5a6ad8462e7cf3082381 100644 (file)
--- a/open-vm-tools/lib/hgfsServer/hgfsServer.c
+++ b/open-vm-tools/lib/hgfsServer/hgfsServer.c
@@ -5690,10 +5690,11 @@ HgfsCreateAndCacheFileNode(HgfsFileOpenInfo *openInfo, // IN: Open info struct
  *
  * HgfsAllocInitReply --
  *
- *    Allocates hgfs reply packet and calculates pointer to HGFS payload.
+ *    Retrieves the hgfs protocol reply data buffer that follows the reply header.
  *
  * Results:
- *    TRUE on success, FALSE on failure.
+ *    Cannot fail, returns the protocol reply data buffer for the corresponding
+ *    processed protocol request.
  *
  * Side effects:
  *    None
@@ -5701,18 +5702,17 @@ HgfsCreateAndCacheFileNode(HgfsFileOpenInfo *openInfo, // IN: Open info struct
  *-----------------------------------------------------------------------------
  */
 
-Bool
+void *
 HgfsAllocInitReply(HgfsPacket *packet,           // IN/OUT: Hgfs Packet
-                   char const *packetHeader,     // IN: packet header
-                   size_t payloadSize,           // IN: payload size
-                   void **payload,               // OUT: pointer to the reply payload
+                   void const *packetHeader,     // IN: packet header
+                   size_t replyDataSize,         // IN: replyDataSize size
                    HgfsSessionInfo *session)     // IN: Session Info
 {
-   HgfsRequest *request = (HgfsRequest *)packetHeader;
+   const HgfsRequest *request = packetHeader;
    size_t replyPacketSize;
    size_t headerSize = 0; /* Replies prior to V3 do not have a header. */
-   Bool result = FALSE;
-   char *reply;
+   void *replyHeader;
+   void *replyData;
 
    if (HGFS_V4_LEGACY_OPCODE == request->op) {
       headerSize = sizeof(HgfsHeader);
@@ -5720,20 +5720,20 @@ HgfsAllocInitReply(HgfsPacket *packet,           // IN/OUT: Hgfs Packet
               request->op > HGFS_OP_RENAME_V2) {
       headerSize = sizeof(HgfsReply);
    }
-   replyPacketSize = headerSize + payloadSize;
-   reply = HSPU_GetReplyPacket(packet, &replyPacketSize, session->transportSession);
+   replyPacketSize = headerSize + replyDataSize;
+   replyHeader = HSPU_GetReplyPacket(packet, &replyPacketSize, session->transportSession);
 
-   if (reply && (replyPacketSize >= headerSize + payloadSize)) {
-      memset(reply, 0, headerSize + payloadSize);
-      result = TRUE;
-      if (payloadSize > 0) {
-         *payload = reply + headerSize;
-      } else {
-         *payload = NULL;
-      }
+   ASSERT_DEVEL(replyHeader && (replyPacketSize >= headerSize + replyDataSize));
+
+   memset(replyHeader, 0, headerSize + replyDataSize);
+   if (replyDataSize > 0) {
+      replyData = (char *)replyHeader + headerSize;
+   } else {
+      replyData = NULL;
+      ASSERT(FALSE);
    }
 
-   return result;
+   return replyData;
 }
 
 
@@ -5778,50 +5778,41 @@ HgfsServerRead(HgfsInputParam *input)  // IN: Input params
             uint32 inlineDataSize =
                (HGFS_OP_READ_FAST_V4 == input->op) ? 0 : requiredSize;
 
-            if (!HgfsAllocInitReply(input->packet, input->metaPacket,
-                                    sizeof *reply + inlineDataSize, (void **)&reply,
-                                    input->session)) {
-               status = HGFS_ERROR_PROTOCOL;
-               LOG(4, ("%s: V3/V4 Failed to alloc reply -> PROTOCOL_ERROR.\n", __FUNCTION__));
+            reply = HgfsAllocInitReply(input->packet, input->metaPacket,
+                                       sizeof *reply + inlineDataSize, input->session);
+            if (HGFS_OP_READ_V3 == input->op) {
+               payload = &reply->payload[0];
             } else {
-               if (HGFS_OP_READ_V3 == input->op) {
-                  payload = &reply->payload[0];
-               } else {
-                  payload = HSPU_GetDataPacketBuf(input->packet, BUF_WRITEABLE,
-                                                  input->transportSession);
-               }
-               if (payload) {
-                  status = HgfsPlatformReadFile(file, input->session, offset,
-                                                requiredSize, payload,
-                                                &reply->actualSize);
-                  if (HGFS_ERROR_SUCCESS == status) {
-                     reply->reserved = 0;
-                     replyPayloadSize = sizeof *reply +
-                                         ((inlineDataSize > 0) ? reply->actualSize : 0);
-                  }
-               } else {
-                  status = HGFS_ERROR_PROTOCOL;
-                  LOG(4, ("%s: V3/V4 Failed to get payload -> PROTOCOL_ERROR.\n", __FUNCTION__));
+               payload = HSPU_GetDataPacketBuf(input->packet, BUF_WRITEABLE,
+                                               input->transportSession);
+            }
+            if (payload) {
+               status = HgfsPlatformReadFile(file, input->session, offset,
+                                             requiredSize, payload,
+                                             &reply->actualSize);
+               if (HGFS_ERROR_SUCCESS == status) {
+                  reply->reserved = 0;
+                  replyPayloadSize = sizeof *reply +
+                                       ((inlineDataSize > 0) ? reply->actualSize : 0);
                }
+            } else {
+               status = HGFS_ERROR_PROTOCOL;
+               LOG(4, ("%s: V3/V4 Failed to get payload -> PROTOCOL_ERROR.\n", __FUNCTION__));
             }
             break;
          }
       case HGFS_OP_READ: {
             HgfsReplyRead *reply;
 
-            if (HgfsAllocInitReply(input->packet, input->metaPacket,
-                                   sizeof *reply + requiredSize, (void **)&reply,
-                                   input->session)) {
-               status = HgfsPlatformReadFile(file, input->session, offset, requiredSize,
-                                             reply->payload, &reply->actualSize);
-               if (HGFS_ERROR_SUCCESS == status) {
-                  replyPayloadSize = sizeof *reply + reply->actualSize;
-               } else {
-                  LOG(4, ("%s: V1 Failed to read-> %d.\n", __FUNCTION__, status));
-               }
+            reply = HgfsAllocInitReply(input->packet, input->metaPacket,
+                                       sizeof *reply + requiredSize, input->session);
+
+            status = HgfsPlatformReadFile(file, input->session, offset, requiredSize,
+                                          reply->payload, &reply->actualSize);
+            if (HGFS_ERROR_SUCCESS == status) {
+               replyPayloadSize = sizeof *reply + reply->actualSize;
             } else {
-               status = HGFS_ERROR_PROTOCOL;
-               LOG(4, ("%s: V1 Failed to alloc reply -> PROTOCOL_ERROR.\n", __FUNCTION__));
+               LOG(4, ("%s: V1 Failed to read-> %d.\n", __FUNCTION__, status));
             }
             break;
          }
@@ -7938,89 +7929,86 @@ HgfsServerSearchRead(HgfsInputParam *input)  // IN: Input params
       LOG(4, ("%s: read search #%u, offset %u\n", __FUNCTION__,
               hgfsSearchHandle, info.startIndex));
 
-      if (!HgfsAllocInitReply(input->packet, input->metaPacket,
-                              baseReplySize + inlineDataSize, &info.reply,
-                              input->session)) {
-         status = HGFS_ERROR_PROTOCOL;
+      info.reply = HgfsAllocInitReply(input->packet, input->metaPacket,
+                                      baseReplySize + inlineDataSize,
+                                      input->session);
+
+      if (inlineDataSize == 0) {
+         info.replyPayload = HSPU_GetDataPacketBuf(input->packet, BUF_WRITEABLE,
+                                                   input->transportSession);
       } else {
+         info.replyPayload = (char *)info.reply + baseReplySize;
+      }
 
-         if (inlineDataSize == 0) {
-            info.replyPayload = HSPU_GetDataPacketBuf(input->packet, BUF_WRITEABLE,
-                                                      input->transportSession);
-         } else {
-            info.replyPayload = (char *)info.reply + baseReplySize;
-         }
+      if (info.replyPayload == NULL) {
+         LOG(4, ("%s: Op %d reply buffer failure\n", __FUNCTION__, input->op));
+         status = HGFS_ERROR_PROTOCOL;
+      } else {
 
-         if (info.replyPayload == NULL) {
-            LOG(4, ("%s: Op %d reply buffer failure\n", __FUNCTION__, input->op));
-            status = HGFS_ERROR_PROTOCOL;
-         } else {
+         if (HgfsGetSearchCopy(hgfsSearchHandle, input->session, &search)) {
+            /* Get the config options. */
+            if (search.utf8ShareNameLen != 0) {
+               nameStatus = HgfsServerPolicy_GetShareOptions(search.utf8ShareName,
+                                                               search.utf8ShareNameLen,
+                                                               &configOptions);
+               if (nameStatus != HGFS_NAME_STATUS_COMPLETE) {
+                  LOG(4, ("%s: no matching share: %s.\n", __FUNCTION__,
+                           search.utf8ShareName));
+                  status = HGFS_ERROR_FILE_NOT_FOUND;
+               }
+            } else if (0 == info.startIndex) {
+               Bool readAllEntries = FALSE;
 
-            if (HgfsGetSearchCopy(hgfsSearchHandle, input->session, &search)) {
-               /* Get the config options. */
-               if (search.utf8ShareNameLen != 0) {
-                  nameStatus = HgfsServerPolicy_GetShareOptions(search.utf8ShareName,
-                                                                search.utf8ShareNameLen,
-                                                                &configOptions);
-                  if (nameStatus != HGFS_NAME_STATUS_COMPLETE) {
-                     LOG(4, ("%s: no matching share: %s.\n", __FUNCTION__,
-                             search.utf8ShareName));
-                     status = HGFS_ERROR_FILE_NOT_FOUND;
-                  }
-               } else if (0 == info.startIndex) {
-                  Bool readAllEntries = FALSE;
+               /*
+                * Reading the first entry, we check if this is a second scan
+                * of the directory. If so, in some cases we restart the scan
+                * by refreshing the entries first.
+                 */
+               if (!HgfsSearchHasReadAllEntries(hgfsSearchHandle,
+                                                input->session,
+                                                &readAllEntries)) {
+                  status = HGFS_ERROR_INTERNAL;
+               }
 
+               if (readAllEntries) {
                   /*
-                   * Reading the first entry, we check if this is a second scan
-                   * of the directory. If so, in some cases we restart the scan
-                   * by refreshing the entries first.
+                   * XXX - a hack that is now required until Fusion 5.0 end
+                   * of lifes see bug 710697.
+                   * The coder modified the server instead of the OS X client
+                   * for the shares directory refresh needed by OS X clients in
+                   * order to work around handles remaining open by Finder.
+                   * This was fixed CLN 1988575 in the OS X client for 5.0.2.
+                   * However, Fusion 4.0 and Fusion 5.0 tools will rely on this hack.
+                   * At least now it works correctly without breaking everything
+                   * else.
                    */
-                  if (!HgfsSearchHasReadAllEntries(hgfsSearchHandle,
-                                                   input->session,
-                                                   &readAllEntries)) {
-                     status = HGFS_ERROR_INTERNAL;
-                  }
-
-                  if (readAllEntries) {
-                     /*
-                      * XXX - a hack that is now required until Fusion 5.0 end
-                      * of lifes see bug 710697.
-                      * The coder modified the server instead of the OS X client
-                      * for the shares directory refresh needed by OS X clients in
-                      * order to work around handles remaining open by Finder.
-                      * This was fixed CLN 1988575 in the OS X client for 5.0.2.
-                      * However, Fusion 4.0 and Fusion 5.0 tools will rely on this hack.
-                      * At least now it works correctly without breaking everything
-                      * else.
-                      */
-                     status = HgfsPlatformRestartSearchDir(hgfsSearchHandle,
-                                                           input->session,
-                                                           search.type);
-                  }
+                  status = HgfsPlatformRestartSearchDir(hgfsSearchHandle,
+                                                         input->session,
+                                                         search.type);
                }
+            }
 
-               if (HGFS_ERROR_SUCCESS == status) {
-                  status = HgfsDoSearchRead(hgfsSearchHandle,
-                                            &search,
-                                            configOptions,
-                                            input->session,
-                                            &info,
-                                            &replyInfoSize,
-                                            &replyDirentSize);
-               }
+            if (HGFS_ERROR_SUCCESS == status) {
+               status = HgfsDoSearchRead(hgfsSearchHandle,
+                                          &search,
+                                          configOptions,
+                                          input->session,
+                                          &info,
+                                          &replyInfoSize,
+                                          &replyDirentSize);
+            }
 
-               if (HGFS_ERROR_SUCCESS == status) {
-                  replyPayloadSize = replyInfoSize +
-                                     ((inlineDataSize == 0) ? 0 : replyDirentSize);
-               }
+            if (HGFS_ERROR_SUCCESS == status) {
+               replyPayloadSize = replyInfoSize +
+                                    ((inlineDataSize == 0) ? 0 : replyDirentSize);
+            }
 
-               free(search.utf8Dir);
-               free(search.utf8ShareName);
+            free(search.utf8Dir);
+            free(search.utf8ShareName);
 
-            } else {
-               LOG(4, ("%s: handle %u is invalid\n", __FUNCTION__, hgfsSearchHandle));
-               status = HGFS_ERROR_INVALID_HANDLE;
-            }
+         } else {
+            LOG(4, ("%s: handle %u is invalid\n", __FUNCTION__, hgfsSearchHandle));
+            status = HGFS_ERROR_INVALID_HANDLE;
          }
       }
    } else {

diff --git a/open-vm-tools/lib/hgfsServer/hgfsServerInt.h b/open-vm-tools/lib/hgfsServer/hgfsServerInt.h
index ca6ee902557b804a87e8aff59887172c2c4a2fc3..af606c032b795b3f3814af3a74ea34e3d3a2d3d3 100644 (file)
--- a/open-vm-tools/lib/hgfsServer/hgfsServerInt.h
+++ b/open-vm-tools/lib/hgfsServer/hgfsServerInt.h
@@ -591,11 +591,10 @@ void HgfsServerSessionGet(HgfsSessionInfo *session);
 HgfsInternalStatus HgfsServerTransportAddSessionToList(HgfsTransportSessionInfo *transportSession,
                                                        HgfsSessionInfo *sessionInfo);
 
-Bool
+void *
 HgfsAllocInitReply(HgfsPacket *packet,           // IN/OUT: Hgfs Packet
-                   char const *packetHeader,     // IN: incoming packet header
-                   size_t payloadSize,           // IN: payload size
-                   void **payload,               // OUT: size of the allocated packet
+                   void const *packetHeader,     // IN: incoming packet header
+                   size_t replyDataSize,         // IN: payload size
                    HgfsSessionInfo *session);    // IN: Session Info
 
 /* Node cache functions. */

diff --git a/open-vm-tools/lib/hgfsServer/hgfsServerParameters.c b/open-vm-tools/lib/hgfsServer/hgfsServerParameters.c
index e8dc597a5f5ba18c6503976796b5c7faf2a771ce..e794752a881949a5b47d81392b62d2b2bf96b168 100644 (file)
--- a/open-vm-tools/lib/hgfsServer/hgfsServerParameters.c
+++ b/open-vm-tools/lib/hgfsServer/hgfsServerParameters.c
@@ -943,7 +943,7 @@ HgfsPackOpenV1Reply(HgfsFileOpenInfo *openInfo,   // IN: open info struct
  *    Pack hgfs open reply to the HgfsReplyOpen{V2} structure.
  *
  * Results:
- *    Always TRUE.
+ *    Always TRUE, FALSE if bad opcode.
  *
  * Side effects:
  *    None
@@ -958,7 +958,7 @@ HgfsPackOpenReply(HgfsPacket *packet,           // IN/OUT: Hgfs Packet
                   size_t *payloadSize,          // OUT: size of packet
                   HgfsSessionInfo *session)     // IN: Session info
 {
-   Bool result;
+   Bool result = TRUE;
 
    ASSERT(openInfo);
    HGFS_ASSERT_PACK_PARAMS;
@@ -969,34 +969,28 @@ HgfsPackOpenReply(HgfsPacket *packet,           // IN/OUT: Hgfs Packet
    case HGFS_OP_OPEN_V3: {
       HgfsReplyOpenV3 *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         HgfsPackOpenReplyV3(openInfo, reply);
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      HgfsPackOpenReplyV3(openInfo, reply);
+      *payloadSize = sizeof *reply;
       break;
    }
    case HGFS_OP_OPEN_V2: {
       HgfsReplyOpenV2 *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         HgfsPackOpenV2Reply(openInfo, reply);
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      HgfsPackOpenV2Reply(openInfo, reply);
+      *payloadSize = sizeof *reply;
       break;
    }
    case HGFS_OP_OPEN: {
       HgfsReplyOpen *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                 (void **)&reply, session);
-      if (result) {
-         HgfsPackOpenV1Reply(openInfo, reply);
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      HgfsPackOpenV1Reply(openInfo, reply);
+      *payloadSize = sizeof *reply;
       break;
    }
    default:
@@ -1132,7 +1126,7 @@ HgfsUnpackCloseRequest(void const *packet,  // IN: request packet
  *    Pack hgfs close reply to the HgfsReplyClose(V3) structure.
  *
  * Results:
- *    Always TRUE.
+ *    Always TRUE, FALSE if bad opcode.
  *
  * Side effects:
  *    None
@@ -1147,7 +1141,7 @@ HgfsPackCloseReply(HgfsPacket *packet,         // IN/OUT: Hgfs Packet
                    size_t *payloadSize,        // OUT: size of packet excluding header
                    HgfsSessionInfo *session)   // IN: Session info
 {
-   Bool result;
+   Bool result = TRUE;
 
    HGFS_ASSERT_PACK_PARAMS;
 
@@ -1157,23 +1151,19 @@ HgfsPackCloseReply(HgfsPacket *packet,         // IN/OUT: Hgfs Packet
    case HGFS_OP_CLOSE_V3: {
       HgfsReplyCloseV3 *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         /* Reply consists of a reserved field only. */
-         reply->reserved = 0;
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      /* Reply consists of a reserved field only. */
+      reply->reserved = 0;
+      *payloadSize = sizeof *reply;
       break;
    }
    case HGFS_OP_CLOSE: {
       HgfsReplyClose *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                 (void **)&reply, session);
-      if (result) {
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      *payloadSize = sizeof *reply;
       break;
    }
    default:
@@ -1327,7 +1317,7 @@ HgfsPackSearchCloseReply(HgfsPacket *packet,         // IN/OUT: Hgfs Packet
                          size_t *payloadSize,        // OUT: size of packet
                          HgfsSessionInfo *session)   // IN: Session info
 {
-   Bool result;
+   Bool result = TRUE;
 
    HGFS_ASSERT_PACK_PARAMS;
 
@@ -1337,23 +1327,19 @@ HgfsPackSearchCloseReply(HgfsPacket *packet,         // IN/OUT: Hgfs Packet
    case HGFS_OP_SEARCH_CLOSE_V3: {
       HgfsReplyCloseV3 *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         /* Reply consists of only a reserved field. */
-         reply->reserved = 0;
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      /* Reply consists of only a reserved field. */
+      reply->reserved = 0;
+      *payloadSize = sizeof *reply;
       break;
    }
    case HGFS_OP_SEARCH_CLOSE: {
       HgfsReplyClose *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      *payloadSize = sizeof *reply;
       break;
    }
    default:
@@ -1737,7 +1723,7 @@ HgfsPackDeleteReply(HgfsPacket *packet,        // IN/OUT: Hgfs Packet
                     size_t *payloadSize,       // OUT: size of packet
                     HgfsSessionInfo *session)  // IN: Session info
 {
-   Bool result;
+   Bool result = TRUE;
 
    HGFS_ASSERT_PACK_PARAMS;
 
@@ -1749,11 +1735,9 @@ HgfsPackDeleteReply(HgfsPacket *packet,        // IN/OUT: Hgfs Packet
    case HGFS_OP_DELETE_DIR_V3: {
       HgfsReplyDeleteV3 *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      *payloadSize = sizeof *reply;
       break;
    }
    case HGFS_OP_DELETE_FILE_V2:
@@ -1762,11 +1746,9 @@ HgfsPackDeleteReply(HgfsPacket *packet,        // IN/OUT: Hgfs Packet
    case HGFS_OP_DELETE_DIR: {
       HgfsReplyDelete *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                 (void **)&reply, session);
-      if (result) {
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      *payloadSize = sizeof *reply;
       break;
    }
    default:
@@ -2150,7 +2132,7 @@ HgfsPackRenameReply(HgfsPacket *packet,        // IN/OUT: Hgfs Packet
                     size_t *payloadSize,       // OUT: size of packet
                     HgfsSessionInfo *session)  // IN: Session info
 {
-   Bool result;
+   Bool result = TRUE;
 
    HGFS_ASSERT_PACK_PARAMS;
 
@@ -2160,24 +2142,20 @@ HgfsPackRenameReply(HgfsPacket *packet,        // IN/OUT: Hgfs Packet
    case HGFS_OP_RENAME_V3: {
       HgfsReplyRenameV3 *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         /* Reply consists of only a reserved field. */
-         reply->reserved = 0;
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      /* Reply consists of only a reserved field. */
+      reply->reserved = 0;
+      *payloadSize = sizeof *reply;
       break;
    }
    case HGFS_OP_RENAME_V2:
    case HGFS_OP_RENAME: {
       HgfsReplyRename *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      *payloadSize = sizeof *reply;
       break;
    }
    default:
@@ -2682,7 +2660,7 @@ HgfsPackGetattrReply(HgfsPacket *packet,         // IN/OUT: Hgfs Packet
                      size_t *payloadSize,        // OUT: size of packet
                      HgfsSessionInfo *session)   // IN: Session info
 {
-   Bool result;
+   Bool result = TRUE;
 
    HGFS_ASSERT_PACK_PARAMS;
 
@@ -2693,11 +2671,9 @@ HgfsPackGetattrReply(HgfsPacket *packet,         // IN/OUT: Hgfs Packet
       HgfsReplyGetattrV3 *reply;
 
       *payloadSize = sizeof *reply + utf8TargetNameLen;
-      result = HgfsAllocInitReply(packet, packetHeader, *payloadSize,
-                                  (void **)&reply, session);
-      if (result) {
-         HgfsPackGetattrReplyPayloadV3(attr, utf8TargetName, utf8TargetNameLen, reply);
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, *payloadSize,
+                                 session);
+      HgfsPackGetattrReplyPayloadV3(attr, utf8TargetName, utf8TargetNameLen, reply);
       break;
    }
 
@@ -2705,26 +2681,22 @@ HgfsPackGetattrReply(HgfsPacket *packet,         // IN/OUT: Hgfs Packet
       HgfsReplyGetattrV2 *reply;
       *payloadSize = sizeof *reply + utf8TargetNameLen;
 
-      result = HgfsAllocInitReply(packet, packetHeader, *payloadSize,
-                                  (void **)&reply, session);
-      if (result) {
-         HgfsPackGetattrReplyPayloadV2(attr,
-                                       utf8TargetName,
-                                       utf8TargetNameLen,
-                                       reply);
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, *payloadSize,
+                                 session);
+      HgfsPackGetattrReplyPayloadV2(attr,
+                                    utf8TargetName,
+                                    utf8TargetNameLen,
+                                    reply);
       break;
    }
 
    case HGFS_OP_GETATTR: {
       HgfsReplyGetattr *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         HgfsPackGetattrReplyPayloadV1(attr, reply);
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      HgfsPackGetattrReplyPayloadV1(attr, reply);
+      *payloadSize = sizeof *reply;
       break;
    }
 
@@ -3687,7 +3659,7 @@ HgfsPackSetattrReply(HgfsPacket *packet,        // IN/OUT: Hgfs Packet
                      size_t *payloadSize,       // OUT: size of packet
                      HgfsSessionInfo *session)  // IN: Session info
 {
-   Bool result;
+   Bool result = TRUE;
 
    *payloadSize = 0;
 
@@ -3695,24 +3667,20 @@ HgfsPackSetattrReply(HgfsPacket *packet,        // IN/OUT: Hgfs Packet
    case HGFS_OP_SETATTR_V3: {
       HgfsReplySetattrV3 *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         /* Reply consists of only a reserved field. */
-         reply->reserved = 0;
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      /* Reply consists of only a reserved field. */
+      reply->reserved = 0;
+      *payloadSize = sizeof *reply;
       break;
    }
    case HGFS_OP_SETATTR_V2:
    case HGFS_OP_SETATTR: {
       HgfsReplySetattr *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      *payloadSize = sizeof *reply;
       break;
    }
    default:
@@ -3985,7 +3953,7 @@ HgfsPackCreateDirReply(HgfsPacket *packet,        // IN/OUT: Hgfs Packet
                        size_t *payloadSize,        // OUT: size of packet
                        HgfsSessionInfo *session)  // IN: Session info
 {
-   Bool result;
+   Bool result = TRUE;
 
    *payloadSize = 0;
 
@@ -3993,33 +3961,27 @@ HgfsPackCreateDirReply(HgfsPacket *packet,        // IN/OUT: Hgfs Packet
    case HGFS_OP_CREATE_DIR_V3: {
       HgfsReplyCreateDirV3 *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         /* Reply consists of only a reserved field. */
-         reply->reserved = 0;
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      /* Reply consists of only a reserved field. */
+      reply->reserved = 0;
+      *payloadSize = sizeof *reply;
       break;
    }
    case HGFS_OP_CREATE_DIR_V2: {
       HgfsReplyCreateDirV2 *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      *payloadSize = sizeof *reply;
       break;
    }
    case HGFS_OP_CREATE_DIR: {
       HgfsReplyCreateDir *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      *payloadSize = sizeof *reply;
       break;
    }
    default:
@@ -4151,18 +4113,16 @@ HgfsPackWriteWin32StreamReply(HgfsPacket *packet,        // IN/OUT: Hgfs Packet
                               HgfsSessionInfo *session)  // IN: Session info
 {
    HgfsReplyWriteWin32StreamV3 *reply;
-   Bool result;
+   Bool result = TRUE;
 
    *payloadSize = 0;
 
    if (HGFS_OP_WRITE_WIN32_STREAM_V3 == op) {
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                 (void **)&reply, session);
-      if (result) {
-         reply->reserved = 0;
-         reply->actualSize = actualSize;
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      reply->reserved = 0;
+      reply->actualSize = actualSize;
+      *payloadSize = sizeof *reply;
    } else {
       LOG(4, ("%s: Incorrect opcode %d\n", __FUNCTION__, op));
       NOT_REACHED();
@@ -4509,7 +4469,7 @@ HgfsUnpackWriteRequest(HgfsInputParam *input,   // IN: Input params
  *    Pack hgfs write reply to the HgfsReplyWrite structure.
  *
  * Results:
- *    TRUE is there are no bugs in the code.
+ *    Always TRUE, FALSE if bad opcode.
  *
  * Side effects:
  *    None
@@ -4525,7 +4485,7 @@ HgfsPackWriteReply(HgfsPacket *packet,           // IN/OUT: Hgfs Packet
                    size_t *payloadSize,          // OUT: size of packet
                    HgfsSessionInfo *session)     // IN: Session info
 {
-   Bool result;
+   Bool result = TRUE;
 
    *payloadSize = 0;
 
@@ -4534,24 +4494,20 @@ HgfsPackWriteReply(HgfsPacket *packet,           // IN/OUT: Hgfs Packet
    case HGFS_OP_WRITE_V3: {
       HgfsReplyWriteV3 *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         reply->reserved = 0;
-         reply->actualSize = actualSize;
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      reply->reserved = 0;
+      reply->actualSize = actualSize;
+      *payloadSize = sizeof *reply;
       break;
    }
    case HGFS_OP_WRITE: {
       HgfsReplyWrite *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         reply->actualSize = actualSize;
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      reply->actualSize = actualSize;
+      *payloadSize = sizeof *reply;
       break;
    }
    default:
@@ -4729,7 +4685,7 @@ HgfsPackQueryVolumeReply(HgfsPacket *packet,        // IN/OUT: Hgfs Packet
                          size_t *payloadSize,       // OUT: size of packet
                          HgfsSessionInfo *session)  // IN: Session info
 {
-   Bool result;
+   Bool result = TRUE;
 
    *payloadSize = 0;
 
@@ -4737,26 +4693,22 @@ HgfsPackQueryVolumeReply(HgfsPacket *packet,        // IN/OUT: Hgfs Packet
    case HGFS_OP_QUERY_VOLUME_INFO_V3: {
       HgfsReplyQueryVolumeV3 *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         reply->reserved = 0;
-         reply->freeBytes = freeBytes;
-         reply->totalBytes = totalBytes;
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      reply->reserved = 0;
+      reply->freeBytes = freeBytes;
+      reply->totalBytes = totalBytes;
+      *payloadSize = sizeof *reply;
       break;
    }
    case HGFS_OP_QUERY_VOLUME_INFO: {
       HgfsReplyQueryVolume *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         reply->freeBytes = freeBytes;
-         reply->totalBytes = totalBytes;
-          *payloadSize = sizeof *reply;
-     }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      reply->freeBytes = freeBytes;
+      reply->totalBytes = totalBytes;
+         *payloadSize = sizeof *reply;
       break;
    }
    default:
@@ -4977,7 +4929,7 @@ HgfsPackSymlinkCreateReply(HgfsPacket *packet,        // IN/OUT: Hgfs Packet
                            size_t *payloadSize,       // OUT: size of packet
                            HgfsSessionInfo *session)  // IN: Session info
 {
-   Bool result;
+   Bool result = TRUE;
 
    HGFS_ASSERT_PACK_PARAMS;
 
@@ -4987,23 +4939,19 @@ HgfsPackSymlinkCreateReply(HgfsPacket *packet,        // IN/OUT: Hgfs Packet
    case HGFS_OP_CREATE_SYMLINK_V3: {
       HgfsReplySymlinkCreateV3 *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         /* Reply only consists of a reserved field. */
-         reply->reserved = 0;
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      /* Reply only consists of a reserved field. */
+      reply->reserved = 0;
+      *payloadSize = sizeof *reply;
       break;
    }
    case HGFS_OP_CREATE_SYMLINK: {
       HgfsReplySymlinkCreate *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      *payloadSize = sizeof *reply;
       break;
    }
    default:
@@ -5174,7 +5122,7 @@ HgfsPackSearchOpenReply(HgfsPacket *packet,          // IN/OUT: Hgfs Packet
                         size_t *payloadSize,         // OUT: size of packet
                         HgfsSessionInfo *session)    // IN: Session info
 {
-   Bool result;
+   Bool result = TRUE;
 
    HGFS_ASSERT_PACK_PARAMS;
 
@@ -5184,24 +5132,20 @@ HgfsPackSearchOpenReply(HgfsPacket *packet,          // IN/OUT: Hgfs Packet
    case HGFS_OP_SEARCH_OPEN_V3: {
       HgfsReplySearchOpenV3 *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         reply->reserved = 0;
-         reply->search = search;
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      reply->reserved = 0;
+      reply->search = search;
+      *payloadSize = sizeof *reply;
       break;
    }
    case HGFS_OP_SEARCH_OPEN: {
       HgfsReplySearchOpen *reply;
 
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         reply->search = search;
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      reply->search = search;
+      *payloadSize = sizeof *reply;
       break;
    }
    default:
@@ -5318,7 +5262,6 @@ HgfsPackCreateSessionReply(HgfsPacket *packet,        // IN/OUT: Hgfs Packet
                            size_t *payloadSize,       // OUT: size of packet
                            HgfsSessionInfo *session)  // IN: Session info
 {
-   Bool result;
    HgfsReplyCreateSessionV4 *reply;
    uint32 numCapabilities = session->numberOfCapabilities;
    uint32 capabilitiesLen = numCapabilities * sizeof *session->hgfsSessionCapabilities;
@@ -5327,19 +5270,16 @@ HgfsPackCreateSessionReply(HgfsPacket *packet,        // IN/OUT: Hgfs Packet
 
    *payloadSize = offsetof(HgfsReplyCreateSessionV4, capabilities) + capabilitiesLen;
 
-   result = HgfsAllocInitReply(packet, packetHeader, *payloadSize, (void **)&reply,
-                               session);
-   if (result) {
-      reply->sessionId = session->sessionId;
-      reply->numCapabilities = numCapabilities;
-      reply->maxPacketSize = session->maxPacketSize;
-      reply->identityOffset = 0;
-      reply->flags = session->flags;
-      reply->reserved = 0;
-      memcpy(reply->capabilities, session->hgfsSessionCapabilities, capabilitiesLen);
-   }
+   reply = HgfsAllocInitReply(packet, packetHeader, *payloadSize, session);
+   reply->sessionId = session->sessionId;
+   reply->numCapabilities = numCapabilities;
+   reply->maxPacketSize = session->maxPacketSize;
+   reply->identityOffset = 0;
+   reply->flags = session->flags;
+   reply->reserved = 0;
+   memcpy(reply->capabilities, session->hgfsSessionCapabilities, capabilitiesLen);
 
-   return result;
+   return TRUE;
 }
 
 
@@ -5366,21 +5306,18 @@ HgfsPackDestroySessionReply(HgfsPacket *packet,        // IN/OUT: Hgfs Packet
                             HgfsSessionInfo *session)  // IN: Session info
 {
    HgfsReplyDestroySessionV4 *reply;
-   Bool result;
 
    HGFS_ASSERT_PACK_PARAMS;
 
    *payloadSize = 0;
 
-   result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                               (void **)&reply, session);
-   if (result) {
-      /* Reply only consists of a reserved field. */
-      *payloadSize = sizeof *reply;
-      reply->reserved = 0;
-   }
+   reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                              session);
+   /* Reply only consists of a reserved field. */
+   *payloadSize = sizeof *reply;
+   reply->reserved = 0;
 
-   return result;
+   return TRUE;
 }
 
 
@@ -5459,23 +5396,21 @@ HgfsPackSetWatchReply(HgfsPacket *packet,           // IN/OUT: Hgfs Packet
                       size_t *payloadSize,          // OUT: size of packet
                       HgfsSessionInfo *session)     // IN: Session info
 {
-   Bool result;
+   Bool result = TRUE;
    HgfsReplySetWatchV4 *reply;
 
    HGFS_ASSERT_PACK_PARAMS;
 
    *payloadSize = 0;
 
-   if (HGFS_OP_SET_WATCH_V4 != op) {
+   if (HGFS_OP_SET_WATCH_V4 == op) {
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      HgfsPackSetWatchReplyV4(watchId, reply);
+      *payloadSize = sizeof *reply;
+   } else {
       NOT_REACHED();
       result = FALSE;
-   } else {
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                               (void **)&reply, session);
-      if (result) {
-         HgfsPackSetWatchReplyV4(watchId, reply);
-         *payloadSize = sizeof *reply;
-      }
    }
 
    return result;
@@ -5608,7 +5543,7 @@ HgfsPackRemoveWatchReply(HgfsPacket *packet,           // IN/OUT: Hgfs Packet
                          size_t *payloadSize,          // OUT: size of packet
                          HgfsSessionInfo *session)     // IN: Session info
 {
-   Bool result;
+   Bool result = TRUE;
    HgfsReplyRemoveWatchV4 *reply;
 
    HGFS_ASSERT_PACK_PARAMS;
@@ -5619,12 +5554,10 @@ HgfsPackRemoveWatchReply(HgfsPacket *packet,           // IN/OUT: Hgfs Packet
       NOT_REACHED();
       result = FALSE;
    } else {
-      result = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
-                                  (void **)&reply, session);
-      if (result) {
-         reply->reserved = 0;
-         *payloadSize = sizeof *reply;
-      }
+      reply = HgfsAllocInitReply(packet, packetHeader, sizeof *reply,
+                                 session);
+      reply->reserved = 0;
+      *payloadSize = sizeof *reply;
    }
    return result;
 }