{"action":"created","comment":{"url":"https://api.github.com/repos/google/UIforETW/pulls/comments/35058886","id":35058886,"diff_hunk":"@@ -108,125 +108,179 @@ void CPowerStatusMonitor::SampleBatteryStat()\n \tfor (int deviceNum = 0; deviceNum < maxBatteries; deviceNum++)\r\n \t{\r\n \t\tSP_DEVICE_INTERFACE_DATA did = { sizeof(did) };\r\n-\r\n-\t\tif (SetupDiEnumDeviceInterfaces(hdev,\r\n+\t\tconst BOOL enumDeviceInterfacesResult =\r\n+\t\t\tSetupDiEnumDeviceInterfaces(hdev,\r\n \t\t\t\t\t\t\t\t\t\t0,\r\n \t\t\t\t\t\t\t\t\t\t&GUID_DEVCLASS_BATTERY,\r\n \t\t\t\t\t\t\t\t\t\tdeviceNum,\r\n-\t\t\t\t\t\t\t\t\t\t&did))\r\n+\t\t\t\t\t\t\t\t\t\t&did);\r\n+\t\tif (!enumDeviceInterfacesResult)\r\n+\t\t{\r\n+\t\t\tconst DWORD lastErr = ::GetLastError( );\r\n+\t\t\tif (lastErr == ERROR_NO_MORE_ITEMS)\r\n+\t\t\t{\r\n+\t\t\t\tbreak; // Enumeration failed - perhaps we're out of items\r\n+\t\t\t}\r\n+\t\t\tcontinue;\r\n+\t\t}\r\n+\r\n+\t\tDWORD bytesNeeded = 0;\r\n+\t\tconst BOOL getDetailResult = SetupDiGetDeviceInterfaceDetail(hdev,\r\n+\t\t\t\t\t\t\t\t\t\t&did,\r\n+\t\t\t\t\t\t\t\t\t\t0,\r\n+\t\t\t\t\t\t\t\t\t\t0,\r\n+\t\t\t\t\t\t\t\t\t\t&bytesNeeded,\r\n+\t\t\t\t\t\t\t\t\t\t0);\r\n+\r\n+\t\tif (getDetailResult)\r\n+\t\t{\r\n+\t\t\tthrow std::logic_error(\"Expected SetupDiGetDeviceInterfaceDetail failure!\");\r\n+\t\t}\r\n+\r\n+\t\tif (GetLastError() != ERROR_INSUFFICIENT_BUFFER)\r\n+\t\t{\r\n+\t\t\tcontinue;\r\n+\t\t}\r\n+\r\n+\r\n+\t\tHLOCAL detailDataAlloc = LocalAlloc(LPTR, bytesNeeded);\r","path":"UIforETW/PowerStatus.cpp","position":42,"original_position":42,"commit_id":"e914aa2576b49f763ff39cc8228f225aa1e0ee6a","original_commit_id":"e914aa2576b49f763ff39cc8228f225aa1e0ee6a","user":{"login":"ariccio","id":2142308,"avatar_url":"https://avatars.githubusercontent.com/u/2142308?v=3","gravatar_id":"","url":"https://api.github.com/users/ariccio","html_url":"https://github.com/ariccio","followers_url":"https://api.github.com/users/ariccio/followers","following_url":"https://api.github.com/users/ariccio/following{/other_user}","gists_url":"https://api.github.com/users/ariccio/gists{/gist_id}","starred_url":"https://api.github.com/users/ariccio/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ariccio/subscriptions","organizations_url":"https://api.github.com/users/ariccio/orgs","repos_url":"https://api.github.com/users/ariccio/repos","events_url":"https://api.github.com/users/ariccio/events{/privacy}","received_events_url":"https://api.github.com/users/ariccio/received_events","type":"User","site_admin":false},"body":"Ah? I figured that there was some reason why you chose `LocalAlloc` over some other memory allocation function....\r\n\r\n","created_at":"2015-07-21T00:12:58Z","updated_at":"2015-07-21T00:12:58Z","html_url":"https://github.com/google/UIforETW/pull/41#discussion_r35058886","pull_request_url":"https://api.github.com/repos/google/UIforETW/pulls/41","_links":{"self":{"href":"https://api.github.com/repos/google/UIforETW/pulls/comments/35058886"},"html":{"href":"https://github.com/google/UIforETW/pull/41#discussion_r35058886"},"pull_request":{"href":"https://api.github.com/repos/google/UIforETW/pulls/41"}}},"pull_request":{"url":"https://api.github.com/repos/google/UIforETW/pulls/41","id":40303829,"html_url":"https://github.com/google/UIforETW/pull/41","diff_url":"https://github.com/google/UIforETW/pull/41.diff","patch_url":"https://github.com/google/UIforETW/pull/41.patch","issue_url":"https://api.github.com/repos/google/UIforETW/issues/41","number":41,"state":"open","locked":false,"title":"Fix `LocalAlloc`/`pdidd` NULL deref warning","user":{"login":"ariccio","id":2142308,"avatar_url":"https://avatars.githubusercontent.com/u/2142308?v=3","gravatar_id":"","url":"https://api.github.com/users/ariccio","html_url":"https://github.com/ariccio","followers_url":"https://api.github.com/users/ariccio/followers","following_url":"https://api.github.com/users/ariccio/following{/other_user}","gists_url":"https://api.github.com/users/ariccio/gists{/gist_id}","starred_url":"https://api.github.com/users/ariccio/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ariccio/subscriptions","organizations_url":"https://api.github.com/users/ariccio/orgs","repos_url":"https://api.github.com/users/ariccio/repos","events_url":"https://api.github.com/users/ariccio/events{/privacy}","received_events_url":"https://api.github.com/users/ariccio/received_events","type":"User","site_admin":false},"body":"`/analyze` complained:\r\n\r\n```\r\nC28182 Dereferencing a copy of a null pointer 'pdidd' contains the same\r\nNULL value as 'LocalAlloc()`128' did.\r\n139 'pdidd' may be NULL\r\n140 'pdidd' is dereferenced, but may still be NULL\r\n```\r\n\r\nSo, I inserted a `NULL` check, and simply `std::terminate()` if\r\n`LocalAlloc` fails, as it means we're probably OOM (unless there's an\r\n***absurdly*** large `DevicePath` in `SP_DEVICE_INTERFACE_DETAIL_DATA`).\r\n\r\nThere were so many levels of indentation in\r\n`CPowerStatusMonitor::SampleBatteryStat` that it was giving me a\r\nheadache, so I went ahead and 'unrolled' it. Is that ok?\r\n\r\nBecause I'm crazy about checking return values, I added two wrapper\r\nfunctions to `Utility.h`/`Utility.cpp` (`FreeLocalAlloc` and\r\n`CloseValidHandle`), as if the wrapped system calls ever fail, there's a\r\nbug on our side of things.\r\n\r\nAs a drive-by fix, I also changed the `OUT` pseudo-annotated parameters\r\nof `NtQueryTimerResolution` (`OUT` is defined to ` ` - blank text) to\r\n`_Out_`, so that `/analyze` treats them as actual out parameters.\r\n\r\n**PLEASE NOTE**:\r\n\r\nThere's another code analysis warning that is ***NOT*** correct\r\n\r\n```\r\nC6102\t\tUsing 'bytesNeeded' from failed function call at line '132'.\r\n\t128\t'bytesNeeded' is not initialized\r\n\t135\tSkip this branch, (assume 'getDetailResult' is false)\r\n\t140\tSkip this branch, (assume 'GetLastError()!=122' is false)\r\n\t146\t'bytesNeeded' is an Input to 'LocalAlloc' (declared at c:\\program files (x86)\\windows kits\\8.1\\include\\um\\winbase.h:1039)\r\n\t146\t'bytesNeeded' is used, but may not have been initialized\r\n```\r\n\r\n... Microsoft just hasn't properly annotated `SetupDiGetDeviceInterfaceDetail`\r\nto say that the 5th parameter is valid under certain failure conditions\r\n(i.e. `_On_failure_( _When_( ( _Param_( 3 ) == 0 ) && ( _Param_( 4 ) == 0 ), _Post_valid_ ) )`...)","created_at":"2015-07-19T06:10:03Z","updated_at":"2015-07-21T00:12:58Z","closed_at":null,"merged_at":null,"merge_commit_sha":"5b836b3c3e30cc4b959f12d86aa232077b49ab27","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/google/UIforETW/pulls/41/commits","review_comments_url":"https://api.github.com/repos/google/UIforETW/pulls/41/comments","review_comment_url":"https://api.github.com/repos/google/UIforETW/pulls/comments{/number}","comments_url":"https://api.github.com/repos/google/UIforETW/issues/41/comments","statuses_url":"https://api.github.com/repos/google/UIforETW/statuses/e914aa2576b49f763ff39cc8228f225aa1e0ee6a","head":{"label":"ariccio:fix-analyze-null-deref-warn","ref":"fix-analyze-null-deref-warn","sha":"e914aa2576b49f763ff39cc8228f225aa1e0ee6a","user":{"login":"ariccio","id":2142308,"avatar_url":"https://avatars.githubusercontent.com/u/2142308?v=3","gravatar_id":"","url":"https://api.github.com/users/ariccio","html_url":"https://github.com/ariccio","followers_url":"https://api.github.com/users/ariccio/followers","following_url":"https://api.github.com/users/ariccio/following{/other_user}","gists_url":"https://api.github.com/users/ariccio/gists{/gist_id}","starred_url":"https://api.github.com/users/ariccio/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ariccio/subscriptions","organizations_url":"https://api.github.com/users/ariccio/orgs","repos_url":"https://api.github.com/users/ariccio/repos","events_url":"https://api.github.com/users/ariccio/events{/privacy}","received_events_url":"https://api.github.com/users/ariccio/received_events","type":"User","site_admin":false},"repo":{"id":33953307,"name":"UIforETW","full_name":"ariccio/UIforETW","owner":{"login":"ariccio","id":2142308,"avatar_url":"https://avatars.githubusercontent.com/u/2142308?v=3","gravatar_id":"","url":"https://api.github.com/users/ariccio","html_url":"https://github.com/ariccio","followers_url":"https://api.github.com/users/ariccio/followers","following_url":"https://api.github.com/users/ariccio/following{/other_user}","gists_url":"https://api.github.com/users/ariccio/gists{/gist_id}","starred_url":"https://api.github.com/users/ariccio/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ariccio/subscriptions","organizations_url":"https://api.github.com/users/ariccio/orgs","repos_url":"https://api.github.com/users/ariccio/repos","events_url":"https://api.github.com/users/ariccio/events{/privacy}","received_events_url":"https://api.github.com/users/ariccio/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/ariccio/UIforETW","description":"User interface for recording and managing ETW traces","fork":true,"url":"https://api.github.com/repos/ariccio/UIforETW","forks_url":"https://api.github.com/repos/ariccio/UIforETW/forks","keys_url":"https://api.github.com/repos/ariccio/UIforETW/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ariccio/UIforETW/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ariccio/UIforETW/teams","hooks_url":"https://api.github.com/repos/ariccio/UIforETW/hooks","issue_events_url":"https://api.github.com/repos/ariccio/UIforETW/issues/events{/number}","events_url":"https://api.github.com/repos/ariccio/UIforETW/events","assignees_url":"https://api.github.com/repos/ariccio/UIforETW/assignees{/user}","branches_url":"https://api.github.com/repos/ariccio/UIforETW/branches{/branch}","tags_url":"https://api.github.com/repos/ariccio/UIforETW/tags","blobs_url":"https://api.github.com/repos/ariccio/UIforETW/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ariccio/UIforETW/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ariccio/UIforETW/git/refs{/sha}","trees_url":"https://api.github.com/repos/ariccio/UIforETW/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ariccio/UIforETW/statuses/{sha}","languages_url":"https://api.github.com/repos/ariccio/UIforETW/languages","stargazers_url":"https://api.github.com/repos/ariccio/UIforETW/stargazers","contributors_url":"https://api.github.com/repos/ariccio/UIforETW/contributors","subscribers_url":"https://api.github.com/repos/ariccio/UIforETW/subscribers","subscription_url":"https://api.github.com/repos/ariccio/UIforETW/subscription","commits_url":"https://api.github.com/repos/ariccio/UIforETW/commits{/sha}","git_commits_url":"https://api.github.com/repos/ariccio/UIforETW/git/commits{/sha}","comments_url":"https://api.github.com/repos/ariccio/UIforETW/comments{/number}","issue_comment_url":"https://api.github.com/repos/ariccio/UIforETW/issues/comments{/number}","contents_url":"https://api.github.com/repos/ariccio/UIforETW/contents/{+path}","compare_url":"https://api.github.com/repos/ariccio/UIforETW/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ariccio/UIforETW/merges","archive_url":"https://api.github.com/repos/ariccio/UIforETW/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ariccio/UIforETW/downloads","issues_url":"https://api.github.com/repos/ariccio/UIforETW/issues{/number}","pulls_url":"https://api.github.com/repos/ariccio/UIforETW/pulls{/number}","milestones_url":"https://api.github.com/repos/ariccio/UIforETW/milestones{/number}","notifications_url":"https://api.github.com/repos/ariccio/UIforETW/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ariccio/UIforETW/labels{/name}","releases_url":"https://api.github.com/repos/ariccio/UIforETW/releases{/id}","created_at":"2015-04-14T19:47:38Z","updated_at":"2015-05-05T07:00:31Z","pushed_at":"2015-07-19T06:08:07Z","git_url":"git://github.com/ariccio/UIforETW.git","ssh_url":"git@github.com:ariccio/UIforETW.git","clone_url":"https://github.com/ariccio/UIforETW.git","svn_url":"https://github.com/ariccio/UIforETW","homepage":"","size":2992,"stargazers_count":0,"watchers_count":0,"language":"C++","has_issues":false,"has_downloads":false,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"google:master","ref":"master","sha":"65cfc3affdf40f41f6491923e9a9b4e9b101ccc1","user":{"login":"google","id":1342004,"avatar_url":"https://avatars.githubusercontent.com/u/1342004?v=3","gravatar_id":"","url":"https://api.github.com/users/google","html_url":"https://github.com/google","followers_url":"https://api.github.com/users/google/followers","following_url":"https://api.github.com/users/google/following{/other_user}","gists_url":"https://api.github.com/users/google/gists{/gist_id}","starred_url":"https://api.github.com/users/google/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/google/subscriptions","organizations_url":"https://api.github.com/users/google/orgs","repos_url":"https://api.github.com/users/google/repos","events_url":"https://api.github.com/users/google/events{/privacy}","received_events_url":"https://api.github.com/users/google/received_events","type":"Organization","site_admin":false},"repo":{"id":33694418,"name":"UIforETW","full_name":"google/UIforETW","owner":{"login":"google","id":1342004,"avatar_url":"https://avatars.githubusercontent.com/u/1342004?v=3","gravatar_id":"","url":"https://api.github.com/users/google","html_url":"https://github.com/google","followers_url":"https://api.github.com/users/google/followers","following_url":"https://api.github.com/users/google/following{/other_user}","gists_url":"https://api.github.com/users/google/gists{/gist_id}","starred_url":"https://api.github.com/users/google/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/google/subscriptions","organizations_url":"https://api.github.com/users/google/orgs","repos_url":"https://api.github.com/users/google/repos","events_url":"https://api.github.com/users/google/events{/privacy}","received_events_url":"https://api.github.com/users/google/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/google/UIforETW","description":"User interface for recording and managing ETW traces","fork":false,"url":"https://api.github.com/repos/google/UIforETW","forks_url":"https://api.github.com/repos/google/UIforETW/forks","keys_url":"https://api.github.com/repos/google/UIforETW/keys{/key_id}","collaborators_url":"https://api.github.com/repos/google/UIforETW/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/google/UIforETW/teams","hooks_url":"https://api.github.com/repos/google/UIforETW/hooks","issue_events_url":"https://api.github.com/repos/google/UIforETW/issues/events{/number}","events_url":"https://api.github.com/repos/google/UIforETW/events","assignees_url":"https://api.github.com/repos/google/UIforETW/assignees{/user}","branches_url":"https://api.github.com/repos/google/UIforETW/branches{/branch}","tags_url":"https://api.github.com/repos/google/UIforETW/tags","blobs_url":"https://api.github.com/repos/google/UIforETW/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/google/UIforETW/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/google/UIforETW/git/refs{/sha}","trees_url":"https://api.github.com/repos/google/UIforETW/git/trees{/sha}","statuses_url":"https://api.github.com/repos/google/UIforETW/statuses/{sha}","languages_url":"https://api.github.com/repos/google/UIforETW/languages","stargazers_url":"https://api.github.com/repos/google/UIforETW/stargazers","contributors_url":"https://api.github.com/repos/google/UIforETW/contributors","subscribers_url":"https://api.github.com/repos/google/UIforETW/subscribers","subscription_url":"https://api.github.com/repos/google/UIforETW/subscription","commits_url":"https://api.github.com/repos/google/UIforETW/commits{/sha}","git_commits_url":"https://api.github.com/repos/google/UIforETW/git/commits{/sha}","comments_url":"https://api.github.com/repos/google/UIforETW/comments{/number}","issue_comment_url":"https://api.github.com/repos/google/UIforETW/issues/comments{/number}","contents_url":"https://api.github.com/repos/google/UIforETW/contents/{+path}","compare_url":"https://api.github.com/repos/google/UIforETW/compare/{base}...{head}","merges_url":"https://api.github.com/repos/google/UIforETW/merges","archive_url":"https://api.github.com/repos/google/UIforETW/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/google/UIforETW/downloads","issues_url":"https://api.github.com/repos/google/UIforETW/issues{/number}","pulls_url":"https://api.github.com/repos/google/UIforETW/pulls{/number}","milestones_url":"https://api.github.com/repos/google/UIforETW/milestones{/number}","notifications_url":"https://api.github.com/repos/google/UIforETW/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/google/UIforETW/labels{/name}","releases_url":"https://api.github.com/repos/google/UIforETW/releases{/id}","created_at":"2015-04-09T21:46:04Z","updated_at":"2015-07-19T14:43:57Z","pushed_at":"2015-07-19T06:10:03Z","git_url":"git://github.com/google/UIforETW.git","ssh_url":"git@github.com:google/UIforETW.git","clone_url":"https://github.com/google/UIforETW.git","svn_url":"https://github.com/google/UIforETW","homepage":"https://randomascii.wordpress.com/2015/04/14/uiforetw-windows-performance-made-easier/","size":3099,"stargazers_count":81,"watchers_count":81,"language":"C++","has_issues":true,"has_downloads":false,"has_wiki":false,"has_pages":false,"forks_count":10,"mirror_url":null,"open_issues_count":9,"forks":10,"open_issues":9,"watchers":81,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/google/UIforETW/pulls/41"},"html":{"href":"https://github.com/google/UIforETW/pull/41"},"issue":{"href":"https://api.github.com/repos/google/UIforETW/issues/41"},"comments":{"href":"https://api.github.com/repos/google/UIforETW/issues/41/comments"},"review_comments":{"href":"https://api.github.com/repos/google/UIforETW/pulls/41/comments"},"review_comment":{"href":"https://api.github.com/repos/google/UIforETW/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/google/UIforETW/pulls/41/commits"},"statuses":{"href":"https://api.github.com/repos/google/UIforETW/statuses/e914aa2576b49f763ff39cc8228f225aa1e0ee6a"}}}}