1405565 - Coverity issue in ssl_keylog_unittest

Status: RESOLVED FIXED

(NSS :: Test, defect)

Description

[Franziskus Kiefer [:franziskus]]

Error handling issues  (CHECKED_RETURN)
/gtests/ssl_gtest/ssl_keylog_unittest.cc: 22 in nss_test::KeyLogFileTest::SetUp()()
16     static const std::string keylog_file_path = "keylog.txt";
17
18     class KeyLogFileTest : public TlsConnectGeneric {
19      public:
20       void SetUp() {
21         TlsConnectTestBase::SetUp();
>>>     CID 1418912:  Error handling issues  (CHECKED_RETURN)
>>>     Calling "remove(_ZN44_INTERNAL_22_ssl_keylog_unittest_cc_d9dbda3a8nss_test16keylog_file_pathB5cxx11E.c_str())" without checking return value. This library function may fail and return an error code.
22         remove(keylog_file_path.c_str());
23         std::ostringstream sstr;
24         sstr << "SSLKEYLOGFILE=" << keylog_file_path;
25         PR_SetEnv(sstr.str().c_str());
26       }
27

Comment 1

[Martin Thomson [:mt:]]

I looked into this and found that remove always reports failure.  The file doesn't seem to be left lying around though, or at least I can't find the file.

Comment 2

[Peter Wu]

The file is only created during the test (and not cleaned up afterwards). It is removed to ensure that previous runs do not interfere with it.

I couldn't find a very portable function to create tmp files. Does Windows support mkstemp?

Comment 3

[Peter Wu]

Attachment: 1-Bug_1405565___ssl_keylog_unittest__ignore_remove_failure.patch

This patch just ignores the return value. I could have added a check for errno == ENOENT, but that seems a bit superfluous for a rare edge case. Even if the remove failed for some other reason, the unit test would have caught that (and error out).

Comment 4

[Martin Thomson [:mt:]]

Comment on attachment 8918666 [details] [diff] [review]
1-Bug_1405565___ssl_keylog_unittest__ignore_remove_failure.patch

Review of attachment 8918666 [details] [diff] [review]:
-----------------------------------------------------------------

https://hg.mozilla.org/projects/nss/rev/24695a55c095