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.

diff --git a/open-vm-tools/lib/misc/hostinfoPosix.c b/open-vm-tools/lib/misc/hostinfoPosix.c
index 472b207714610b6bb622681a26d873b75d1f63da..6c7bf358f91a0351e5e8fa5de3f4613fd60b2f50 100644 (file)
--- 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 3c7cabf8832b86b7919c74c1a9a3fef073edc1be..b210f9c340ae6f49a5c0c0b8d5c9de9b585cbf42 100644 (file)
--- 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 0b12cca9fc9d6aeae63bf88882504dd2fe3d8b37..3cccd295b6da75ebe45f80ed1f1b1de2c6c84b2e 100644 (file)
--- 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;