Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2384493003:
Fix GN issues identified by MSVC static analysis. (Closed)
|
Created:
4 years, 2 months ago by brettw Modified:
4 years, 2 months ago Reviewers:
brucedawson CC:
chromium-reviews, Dirk Pranke, tfarina, agrieve+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix GN issues identified by MSVC static analysis.
MSVC's /analyze identified two missing null checks because it noted that a
pointer was null checked in one place but not another. After some study I
determined that null checks are not necessary for these values so I removed
them.
Although isxdigit takes an int, the input must be an unsigned char or the
behavior is undefined. I added the cast. I think this API is suboptimal.
BUG=643842, 427616
Committed: https://crrev.com/e23e7021f9a06d621088bc13856db9ba93c328c0
Cr-Commit-Position: refs/heads/master@{#421945}
Patch Set 1 #
Messages
Total messages: 18 (12 generated)
brettw@chromium.org changed reviewers: + brucedawson@chromium.org
Description was changed from ========== Fix GN issues identified by MSVC static analysis. MSVC's /analtze identified two missing null checks because it noted that a pointer was null checked in one place but not another. After some study I determined that null checks are not necessary for these values so I removed them. BUG=643842 ========== to ========== Fix GN issues identified by MSVC static analysis. MSVC's /analyze identified two missing null checks because it noted that a pointer was null checked in one place but not another. After some study I determined that null checks are not necessary for these values so I removed them. BUG=643842,427616 ==========
That makes the intent clearer. Thanks. lgtm I fixed a spelling error and added bug 427616 (/analyze tracking bug) in the issue description.
xdigit
The CQ bit was checked by brettw@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Fix GN issues identified by MSVC static analysis. MSVC's /analyze identified two missing null checks because it noted that a pointer was null checked in one place but not another. After some study I determined that null checks are not necessary for these values so I removed them. BUG=643842,427616 ========== to ========== Fix GN issues identified by MSVC static analysis. MSVC's /analyze identified two missing null checks because it noted that a pointer was null checked in one place but not another. After some study I determined that null checks are not necessary for these values so I removed them. Although isxdigit takes an int, the input must be an unsigned char or the behavior is undefined. I added the cast. I think this API is dumb. BUG=643842,427616 ==========
Description was changed from ========== Fix GN issues identified by MSVC static analysis. MSVC's /analyze identified two missing null checks because it noted that a pointer was null checked in one place but not another. After some study I determined that null checks are not necessary for these values so I removed them. Although isxdigit takes an int, the input must be an unsigned char or the behavior is undefined. I added the cast. I think this API is dumb. BUG=643842,427616 ========== to ========== Fix GN issues identified by MSVC static analysis. MSVC's /analyze identified two missing null checks because it noted that a pointer was null checked in one place but not another. After some study I determined that null checks are not necessary for these values so I removed them. Although isxdigit takes an int, the input must be an unsigned char or the behavior is undefined. I added the cast. I think this API is suboptimal. BUG=643842,427616 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Patchset #2 (id:20001) has been deleted
The CQ bit was checked by brettw@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Fix GN issues identified by MSVC static analysis. MSVC's /analyze identified two missing null checks because it noted that a pointer was null checked in one place but not another. After some study I determined that null checks are not necessary for these values so I removed them. Although isxdigit takes an int, the input must be an unsigned char or the behavior is undefined. I added the cast. I think this API is suboptimal. BUG=643842,427616 ========== to ========== Fix GN issues identified by MSVC static analysis. MSVC's /analyze identified two missing null checks because it noted that a pointer was null checked in one place but not another. After some study I determined that null checks are not necessary for these values so I removed them. Although isxdigit takes an int, the input must be an unsigned char or the behavior is undefined. I added the cast. I think this API is suboptimal. BUG=643842,427616 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Fix GN issues identified by MSVC static analysis. MSVC's /analyze identified two missing null checks because it noted that a pointer was null checked in one place but not another. After some study I determined that null checks are not necessary for these values so I removed them. Although isxdigit takes an int, the input must be an unsigned char or the behavior is undefined. I added the cast. I think this API is suboptimal. BUG=643842,427616 ========== to ========== Fix GN issues identified by MSVC static analysis. MSVC's /analyze identified two missing null checks because it noted that a pointer was null checked in one place but not another. After some study I determined that null checks are not necessary for these values so I removed them. Although isxdigit takes an int, the input must be an unsigned char or the behavior is undefined. I added the cast. I think this API is suboptimal. BUG=643842,427616 Committed: https://crrev.com/e23e7021f9a06d621088bc13856db9ba93c328c0 Cr-Commit-Position: refs/heads/master@{#421945} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/e23e7021f9a06d621088bc13856db9ba93c328c0 Cr-Commit-Position: refs/heads/master@{#421945} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
