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

Description Simon Fraser (smfr) 2020-09-12 15:32:06 PDT
Replace formatForDebugger() which uses raw char* with debugDescription()
Comment 1 Simon Fraser (smfr) 2020-09-12 15:33:03 PDT
Created attachment 408617 [details]
Patch
Comment 2 Darin Adler 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());
Comment 3 Simon Fraser (smfr) 2020-09-12 15:59:26 PDT
Created attachment 408622 [details]
Patch
Comment 4 EWS 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].
Comment 5 Radar WebKit Bug Importer 2020-09-12 18:32:19 PDT
<rdar://problem/68783545>