Closed Bug 1344830 Opened 3 years ago Closed 3 years ago

Use range-based 'for' loops in U2F.cpp

Categories

(Core :: DOM: Device Interfaces, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox54 --- wontfix
firefox55 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(1 file)

While fixing bug 1344816, I noticed some counter-based 'for' loops in U2F.cpp, which I believe could be upgraded to use range-based 'for' loops.  I believe these all came from bug 1297552 -- adding dependency on that bug.

Side note: many of these start out with a copy-construction -- something like...
  for (size_t i = 0; i < aRegisteredKeys.Length(); ++i) {
    RegisteredKey key(aRegisteredKeys[i]);

I'm guessing that copy-construction is unnecessary, so I'm removing it in this patch. Please let me know if it's actually needed though! (e.g. maybe the data we're copying from can get destroyed during the loop? I hope not! But if so, then that would be one reason we do actually need to copy.)

Patch coming up.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Comment on attachment 8844112 [details]
Bug 1344830: Convert to range-based 'for' loops in U2F.cpp.

https://reviewboard.mozilla.org/r/117640/#review119324

::: dom/u2f/U2F.cpp:623
(Diff revision 1)
>      status->Stop(appIdResult);
>    }
>  
>    // First, we must determine if any of the RegisteredKeys are already
>    // registered, e.g., in the whitelist.
> -  for (LocalRegisteredKey key: mRegisteredKeys) {
> +  for (LocalRegisteredKey key : mRegisteredKeys) {

(Note: while I'm here, I made a whitespace-only change to this already-existing range-based 'for' loop -- I just added a space character before the ":" for consistency.)
Comment on attachment 8844112 [details]
Bug 1344830: Convert to range-based 'for' loops in U2F.cpp.

https://reviewboard.mozilla.org/r/117640/#review119332

This looks good to me, too -- assuming it works, of course. ;) At the time this was written, the WebIDL `DictionaryBase`-inheriting types weren't iterable like this.
Attachment #8844112 - Flags: review?(jjones) → review+
Makes sense. This compiles successfully for me on my local machine, at least -- I'll give it a Try run before landing, to be sure it works elsewhere too.
Flags: needinfo?(dholbert)
Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/072038384d3e
Convert to range-based 'for' loops in U2F.cpp. r=jcj
Flags: needinfo?(dholbert)
https://hg.mozilla.org/mozilla-central/rev/072038384d3e
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.