Summary: | Remove Dead Stores from editing directory | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jinwoo Song <jinwoo7.song> | ||||||
Component: | WebCore Misc. | Assignee: | Jinwoo Song <jinwoo7.song> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, darin | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Jinwoo Song
2014-03-21 23:24:15 PDT
Created attachment 227536 [details]
Patch
Comment on attachment 227536 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227536&action=review > Source/WebCore/editing/CompositeEditCommand.cpp:-843 > - gapEnd = std::min(gapEnd, end); I suspect the dead store here indicates a bug. I think that gapLen needs to be recomputed. Instead of removing this line of code, we should add a line of code: gapLen = gapEnd - gapStart; But of course, we also need to construct a test case to demonstrate that the code is incorrect, which could be difficult. (In reply to comment #0) > Remove Dead Stores from editing directory which assigning a value to a variable and never reading that value. I assume you are finding these with a tool or compiler warning. Could you specify what compiler this is and what warning option, or what tool? Comment on attachment 227536 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227536&action=review >> Source/WebCore/editing/CompositeEditCommand.cpp:-843 >> - gapEnd = std::min(gapEnd, end); > > I suspect the dead store here indicates a bug. I think that gapLen needs to be recomputed. Instead of removing this line of code, we should add a line of code: > > gapLen = gapEnd - gapStart; > > But of course, we also need to construct a test case to demonstrate that the code is incorrect, which could be difficult. Then I'll remain this line and make an another bug to follow up this issue. (In reply to comment #3) > (In reply to comment #0) > > Remove Dead Stores from editing directory which assigning a value to a variable and never reading that value. > > I assume you are finding these with a tool or compiler warning. Could you specify what compiler this is and what warning option, or what tool? I just run a static analyzer built into Xcode (on Mac). :) Created attachment 227622 [details]
Patch
Do not remove dead store in CompositeEditCommand.cpp according to Darin's comment. I'll make another bug for tracking this one.
Comment on attachment 227536 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227536&action=review >>> Source/WebCore/editing/CompositeEditCommand.cpp:-843 >>> - gapEnd = std::min(gapEnd, end); >> >> I suspect the dead store here indicates a bug. I think that gapLen needs to be recomputed. Instead of removing this line of code, we should add a line of code: >> >> gapLen = gapEnd - gapStart; >> >> But of course, we also need to construct a test case to demonstrate that the code is incorrect, which could be difficult. > > Then I'll remain this line and make an another bug to follow up this issue. Created a bug, https://bugs.webkit.org/show_bug.cgi?id=130658 Comment on attachment 227622 [details] Patch Clearing flags on attachment: 227622 Committed r166150: <http://trac.webkit.org/changeset/166150> All reviewed patches have been landed. Closing bug. |