doc: fix coverity report

[mhdawson]

Fix coverity report about possibly dereferencing
a null. If the the buffer.data != nullptr
check indicates that the buffer was null, then
relying on the value in buffer_size is no longer
safe. The later call to uv_pipe_getpeername
depends on the buffer_size being correct to
avoid deferencing buffer.data if it is not
big enough.

Signed-off-by: Michael Dawson mdawson@devrus.com

[mhdawson]

Report from Coverity

 // First call to get required buffer size.
 93  rc = uv_pipe_getsockname(&handle->pipe, buffer.data, &buffer_size);
   	1. Condition rc == UV_ENOBUFS, taking true branch.
 94  if (rc == UV_ENOBUFS) {
 95    buffer = MallocedBuffer<char>(buffer_size);
   	2. Condition buffer.data != NULL, taking false branch.
   	3. var_compare_op: Comparing buffer.data to null implies that buffer.data might be null.
 96    if (buffer.data != nullptr) {
 97      rc = uv_pipe_getsockname(&handle->pipe, buffer.data, &buffer_size);
 98    }
 99  }
   	4. Condition rc == 0, taking false branch.
100  if (rc == 0 && buffer_size != 0 && buffer.data != nullptr) {
101    writer->json_keyvalue("localEndpoint", buffer.data);
102  } else {
103    writer->json_keyvalue("localEndpoint", null);
104  }
105
106  // First call to get required buffer size.
   	
CID 239713 (#1 of 1): Dereference after null check (FORWARD_NULL)
5. var_deref_model: Passing null pointer buffer.data to uv_pipe_getpeername, which dereferences it.
107  rc = uv_pipe_getpeername(&handle->pipe, buffer.data, &buffer_size);
108  if (rc == UV_ENOBUFS) {
109    buffer = MallocedBuffer<char>(buffer_size);
110    if (buffer.data != nullptr) {
111      rc = uv_pipe_getpeername(&handle->pipe, buffer.data, &buffer_size);
112    }
113  }

[RaisinTen]

This doesn't look like the correct fix because buffer.data can't be null here. Also, should we use src as the subsystem instead of doc?

[RaisinTen]

LGTM

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/43464/

[RaisinTen]

@mhdawson wdyt about?

Also, should we use src as the subsystem instead of doc?

Are you planning to change it while landing this?

[mhdawson]

Also, should we use src as the subsystem instead of doc?

Good point, I must have had doc on my mind, will change while landing.

[mhdawson]

CI run looks to be complete (https://ci.nodejs.org/job/node-test-pull-request/43464/) even though what's shown on the PR shows a job still running. Will land.

[mhdawson]

Landed in 3026ca0