From 9367d3e3cf8819b198dd52cd4e1f19872efcfefe Mon Sep 17 00:00:00 2001
From: Oliver Kurth <okurth@vmware.com>
Date: Fri, 15 Sep 2017 11:23:36 -0700
Subject: [PATCH] Fix HostinfoGetCmdOutput to return NULL on failure

HostinfoGetCmdOutput unconditionally calls DynBuf_DetachString
along its success path.  DynBuf_DetachString never returns NULL,
so that broke callers expecting a NULL return value if the command
had no output (which can happen if HostinfoGetCmdOutput attempts to
run a non-existent executable; note that this does not trigger any
of the failure paths).

Restore the old code that checked if the DynBuf is non-empty
before retrieving its contents.

Bonus:
* Fix incorrect documentation to Posix_Popen.
* Adjust the StdIO_ReadNextLine documentation to clarify its behavior.
---
 open-vm-tools/lib/misc/hostinfoPosix.c |  5 ++++-
 open-vm-tools/lib/misc/posixPosix.c    |  3 +--
 open-vm-tools/lib/misc/vmstdio.c       | 29 ++++++++++++++------------
 3 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/open-vm-tools/lib/misc/hostinfoPosix.c b/open-vm-tools/lib/misc/hostinfoPosix.c
index 472b20771..6c7bf358f 100644
--- a/open-vm-tools/lib/misc/hostinfoPosix.c
+++ b/open-vm-tools/lib/misc/hostinfoPosix.c
@@ -846,7 +846,10 @@ HostinfoGetCmdOutput(const char *cmd)  // IN:
       free(line);
    }
 
-   out = DynBuf_DetachString(&db);
+   /* Return NULL instead of an empty string if there's no output. */
+   if (DynBuf_Get(&db) != NULL) {
+      out = DynBuf_DetachString(&db);
+   }
 
  closeIt:
    pclose(stream);
diff --git a/open-vm-tools/lib/misc/posixPosix.c b/open-vm-tools/lib/misc/posixPosix.c
index 3c7cabf88..b210f9c34 100644
--- a/open-vm-tools/lib/misc/posixPosix.c
+++ b/open-vm-tools/lib/misc/posixPosix.c
@@ -621,8 +621,7 @@ Posix_Pathconf(const char *pathName,  // IN:
  *      Open a file using POSIX popen().
  *
  * Results:
- *      -1	error
- *      >= 0	success (file descriptor)
+ *      Returns a non-NULL FILE* on success, NULL on error.
  *
  * Side effects:
  *      errno is set on error
diff --git a/open-vm-tools/lib/misc/vmstdio.c b/open-vm-tools/lib/misc/vmstdio.c
index 0b12cca9f..3cccd295b 100644
--- a/open-vm-tools/lib/misc/vmstdio.c
+++ b/open-vm-tools/lib/misc/vmstdio.c
@@ -68,7 +68,7 @@ SuperFgets(FILE *stream,   // IN:
            size_t *count,  // IN/OUT:
            void *bufIn)    // OUT:
 {
-   char *buf = (char *)bufIn;
+   char *buf = bufIn;
    size_t size = 0;
 
    ASSERT(stream);
@@ -146,11 +146,11 @@ SuperFgets(FILE *stream,   // IN:
  *
  *      Read the next line from a stream.
  *
- *      A line is defined as an arbitrary long sequence of arbitrary bytes,
+ *      A line is defined as an arbitrarily long sequence of arbitrary bytes,
  *      that ends with the first occurrence of one of these line terminators:
- *      . \r\n          (the ANSI way, in text mode)
+ *      . \r\n          (the Windows/DOS way, in text mode)
  *      . \n            (the UNIX way)
- *      . \r            (the Legacy Mac (pre-OS X) way)
+ *      . \r            (the legacy Mac (pre-OS X) way)
  *      . end-of-stream
  *
  *      Note that on Windows, getc() returns one '\n' for a \r\n two-byte
@@ -162,13 +162,16 @@ SuperFgets(FILE *stream,   // IN:
  *      allocated.
  *
  * Results:
- *      StdIO_Success on success: '*buf' is an allocated, NUL terminated buffer
- *                                that contains the line (excluding the line
- *                                terminator). If not NULL, '*count' contains
- *                                the size of the buffer (excluding the NUL
- *                                terminator)
- *      StdIO_EOF if there is no next line (end of stream)
- *      StdIO_Error on failure: errno is set accordingly
+ *      StdIO_Success on success:
+ *          '*buf' is an allocated, NUL terminated buffer that contains the
+ *          line (excluding the line terminator).  If not NULL, '*count'
+ *          contains the size of the buffer (excluding the NUL terminator).
+ *
+ *      StdIO_EOF if there is no next line (end of stream):
+ *          '*buf' is left untouched.
+ *
+ *      StdIO_Error on failure:
+ *          errno is set accordingly and '*buf' is left untouched.
  *
  * Side effects:
  *      If the line read is terminated by a standalone '\r' (legacy Mac), the
@@ -183,8 +186,8 @@ SuperFgets(FILE *stream,   // IN:
 StdIO_Status
 StdIO_ReadNextLine(FILE *stream,         // IN:
                    char **buf,           // OUT:
-                   size_t maxBufLength,  // IN:
-                   size_t *count)        // OUT:
+                   size_t maxBufLength,  // IN: May be 0.
+                   size_t *count)        // OUT/OPT:
 {
    DynBuf b;
 
-- 
2.47.3