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 [email protected]

[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. 94if (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. 96if (buffer.data != nullptr){97 rc = uv_pipe_getsockname(&handle->pipe, buffer.data, &buffer_size); 98 } 99 } 4. Condition rc == 0, taking false branch. 100if (rc == 0 && buffer_size != 0 && buffer.data != nullptr){101 writer->json_keyvalue("localEndpoint", buffer.data); 102 } else{103 writer->json_keyvalue("localEndpoint", null); 104 } 105106// 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); 108if (rc == UV_ENOBUFS){109 buffer = MallocedBuffer<char>(buffer_size); 110if (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