Bug 216447

Summary: Replace formatForDebugger() which uses raw char* with debugDescription()
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: WebCore Misc.Assignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, cdumez, cfleizach, cmarcelo, darin, dmazzoni, esprehn+autocc, ews-watchlist, jcraig, jdiggs, kangil.han, mifenton, rniwa, samuel_white, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
darin: review+
Patch none

Simon Fraser (smfr)
Reported 2020-09-12 15:32:06 PDT
Replace formatForDebugger() which uses raw char* with debugDescription()
Attachments
Patch (15.89 KB, patch)
2020-09-12 15:33 PDT, Simon Fraser (smfr)
darin: review+
Patch (15.96 KB, patch)
2020-09-12 15:59 PDT, Simon Fraser (smfr)
no flags
Simon Fraser (smfr)
Comment 1 2020-09-12 15:33:03 PDT
Darin Adler
Comment 2 2020-09-12 15:38:15 PDT
Comment on attachment 408617 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408617&action=review Looks great! > Source/WebCore/dom/Node.cpp:1767 > StringBuilder builder; > - builder.append(nodeName(), " 0x"_s, hex(reinterpret_cast<uintptr_t>(this), Lowercase)); > + > + auto name = nodeName(); > + if (name.isEmpty()) > + name = "<none>"_s; > + > + builder.append(name, " 0x"_s, hex(reinterpret_cast<uintptr_t>(this), Lowercase)); > return builder.toString(); There’s no need to use a StringBuilder here. auto name = nodeName(); return makeString(name.isEmpty() ? "<none>" : "", name, " 0x"_s, hex(reinterpret_cast<uintptr_t>(this), Lowercase)); > Source/WebCore/dom/Position.cpp:1395 > StringBuilder result; > > if (isNull()) > result.appendLiteral("<null>"); > else { > - char s[1024]; > result.appendLiteral("offset "); > result.appendNumber(m_offset); > result.appendLiteral(" of "); > - deprecatedNode()->formatForDebugger(s, sizeof(s)); > - result.append(s); > + result.append(deprecatedNode()->debugDescription()); > } > > - strncpy(buffer, result.toString().utf8().data(), length - 1); > + return result.toString(); There’s no need to use a StringBuilder here: if (isNull()) return "<null>"_s; return makeString("offset ", m_offset, " of ", deprecatedNode()->debugDescription()); > Source/WebCore/dom/Range.cpp:855 > StringBuilder result; > > - const int FormatBufferSize = 1024; > - char s[FormatBufferSize]; > result.appendLiteral("from offset "); > result.appendNumber(m_start.offset()); > result.appendLiteral(" of "); > - startContainer().formatForDebugger(s, FormatBufferSize); > - result.append(s); > + result.append(startContainer().debugDescription()); > result.appendLiteral(" to offset "); > result.appendNumber(m_end.offset()); > result.appendLiteral(" of "); > - endContainer().formatForDebugger(s, FormatBufferSize); > - result.append(s); > + result.append(endContainer().debugDescription()); > > - strncpy(buffer, result.toString().utf8().data(), length - 1); > + return result.toString(); Again: return makeString("from offset ", m_start.offset(), " of ", startContainer().debugDescription(), " to offset ", m_end.offset(), " of ", endContainer().debugDescription()); > Source/WebCore/editing/VisibleSelection.cpp:691 > StringBuilder result; > - String s; > > if (isNone()) { > result.appendLiteral("<none>"); > } else { > - const int FormatBufferSize = 1024; > - char s[FormatBufferSize]; > result.appendLiteral("from "); > - start().formatForDebugger(s, FormatBufferSize); > - result.append(s); > + result.append(start().debugDescription()); > result.appendLiteral(" to "); > - end().formatForDebugger(s, FormatBufferSize); > - result.append(s); > + result.append(end().debugDescription()); > } > > - strncpy(buffer, result.toString().utf8().data(), length - 1); > + return result.toString(); One more time: if (isNone()) return "<none>"_s; return makeString("from ", start().debugDescription(), " to ", end().debugDescription());
Simon Fraser (smfr)
Comment 3 2020-09-12 15:59:26 PDT
EWS
Comment 4 2020-09-12 18:31:32 PDT
Committed r266986: <https://trac.webkit.org/changeset/266986> All reviewed patches have been landed. Closing bug and clearing flags on attachment 408622 [details].
Radar WebKit Bug Importer
Comment 5 2020-09-12 18:32:19 PDT
Note You need to log in before you can comment on or make changes to this bug.