Bug 160850 – [Win] Hardening of getLinkedFonts function.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Per Arne Vollan]

The SUCCEEDED macro should only be used for functions returning a HRESULT type.
Also, make sure a string array index will not exceed the string length.

[Per Arne Vollan]

Created attachment 286053 [details]
Patch

[Brent Fulgham]

Comment on attachment 286053 [details]
Patch

Interesting! I didn't know that SUCCEEDED was inappropriate here. Are there return values from RegQueryValueEx that might fool SUCCEEDED?

It also makes me wonder if we should be using the FAILED macro a few lines above this one, since it also returns a LONG instead of an HRESULT.

I also wonder if we should call "family.charactersWithNullTermination().data()" so many times -- I think (but didn't look) that it creates a new string each time we call that.

r=me! :-)

[Per Arne Vollan]

(In reply to comment #2)
> Comment on attachment 286053 [details]
> Patch

Thanks for reviewing :-)

> 
> Interesting! I didn't know that SUCCEEDED was inappropriate here. Are there
> return values from RegQueryValueEx that might fool SUCCEEDED?
> 

Yes I believe there are.

> It also makes me wonder if we should be using the FAILED macro a few lines
> above this one, since it also returns a LONG instead of an HRESULT.
> 
> I also wonder if we should call
> "family.charactersWithNullTermination().data()" so many times -- I think
> (but didn't look) that it creates a new string each time we call that.
> 

Yes I believe you're right. I can look into that :-)

[Per Arne Vollan]

rdar://problem/17471209

[WebKit Commit Bot]

Comment on attachment 286053 [details]
Patch

Rejecting attachment 286053 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 286053, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

/Volumes/Data/EWS/WebKit/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://webkit-queues.webkit.org/results/1875986

[Myles C. Maxfield]

Please write a test for the comma change.

[Per Arne Vollan]

Committed r204502: <https://trac.webkit.org/changeset/204502>