Summary: | Fix clang static analyzer warning in StyleBuilderConverter.h | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||
Component: | CSS | Assignee: | David Kilzer (:ddkilzer) <ddkilzer> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, don.olmstead, joepeck, koivisto, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Local Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
David Kilzer (:ddkilzer)
2018-10-25 10:54:05 PDT
Created attachment 353094 [details]
Patch v1
This fixes 7 clang static analyzer warnings in WebCore where this code is inlined. Comment on attachment 353094 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=353094&action=review > Source/WebCore/css/StyleBuilderConverter.h:1069 > case CSSValueDense: > if (second && second->valueID() == CSSValueColumn) > - return AutoFlowColumnDense; > - return AutoFlowRowDense; > + autoFlow = AutoFlowColumnDense; > + else > + autoFlow = AutoFlowRowDense; > + break; BTW, if the preference is to change this `switch` statement to use early returns everywhere, I'm happy to do that instead. (In reply to David Kilzer (:ddkilzer) from comment #3) > Comment on attachment 353094 [details] > Patch v1 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=353094&action=review > > > Source/WebCore/css/StyleBuilderConverter.h:1069 > > case CSSValueDense: > > if (second && second->valueID() == CSSValueColumn) > > - return AutoFlowColumnDense; > > - return AutoFlowRowDense; > > + autoFlow = AutoFlowColumnDense; > > + else > > + autoFlow = AutoFlowRowDense; > > + break; > > BTW, if the preference is to change this `switch` statement to use early > returns everywhere, I'm happy to do that instead. Patch looks good to me since its keeping with the style in the rest of the method. If you're going to do the early return maybe change them to use a ternary? Looks fine, stylistic improvements can be done separately. Comment on attachment 353094 [details] Patch v1 Clearing flags on attachment: 353094 Committed r237557: <https://trac.webkit.org/changeset/237557> All reviewed patches have been landed. Closing bug. |