1334265 - 'find' called with a string literal consisting of a single character; consider using the more effective overload accepting a character

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect, P2)

Description

[Sylvestre Ledru [:Sylvestre]]

A trivial bug for a new contributor to learn about our workflow

here:
https://dxr.mozilla.org/mozilla-central/source/media/mtransport/test/ice_unittest.cpp?q=ice_unittest.cpp&redirect_type=direct#239

The "" should be replaced by '' to use a better method

Comment 1

[Eric Rescorla (:ekr)]

I'm having a lot of trouble getting excited about a trivial performance bug in a unit test, especially given that this syntax is consistent with the rest of the find() invocations.

Comment 2

[Sylvestre Ledru [:Sylvestre]]

I won't argue: I am just using that a material for a good first bug for someone to get familiar with our workflow and tools.

Comment 3

[Daniel]

Attachment: The "" have been replaced by ''

Comment 4

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8831490 [details] [diff] [review]
The "" have been replaced by ''

You should generate a diff and no upload the full file:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch

Comment 5

[Srivatsav Gunisetty]

Attachment: bug-1334265-fix.patch

I have changed to quotation marks as suggested. Please review it. I don't know exactly whom to ask for review.

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8832818 [details] [diff] [review]
bug-1334265-fix.patch

Review of attachment 8832818 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 7

[Srivatsav Gunisetty]

Hey Randell, When will this patch get landed?

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Well done, this is explained here:
 https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction#Step_6_-_Actually_get_the_code_into_the_tree

Comment 9

[Srivatsav Gunisetty]

Can you push the patch to try server for me?

Comment 10

[Sylvestre Ledru [:Sylvestre]]

I think we can skip that step :)
Just add the keyword

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Please keep the keyword

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4d40791b6d0a
Replace double quotation marks with single quotation marks in ice_unittest.cpp. r=jesup

Comment 13

[Srivatsav Gunisetty]

Why is the checkin-needed keyword removed? Is there any fault with the patch?

Comment 14

[Sylvestre Ledru [:Sylvestre]]

Because Ryan landed the patch already for you in mozilla-inbound.
Oncee it reaches mozilla-central, this bug will be closed! :)

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4d40791b6d0a