Opened 4 months ago

Closed 3 months ago

#6280 closed change (fixed)

[emscripten] Map::begin should be consistent with HashContainerIterator::operator++

Reported by: sergz Assignee: hfiguiere
Priority: P2 Milestone:
Module: Core Keywords:
Cc: hfiguiere Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29692574/

Description

Background

Map::begin returns just a first entry of the internal array but HashContainerIterator::operator++ skips invalid entries.

What to change

Map::begin should also skip invalid entries.

Add tests.

Change History (8)

comment:1 Changed 4 months ago by hfiguiere

It ignore the invalid entries, but not the deleted entries.

HashContainerIterator() calls operator++ if the current entry is invalid.

I think we should also filter out deleted entries.

comment:2 Changed 4 months ago by hfiguiere

Filtering out deleted entries, that's issue #6281

Last edited 4 months ago by hfiguiere (previous) (diff)

comment:3 Changed 3 months ago by hfiguiere

  • Resolution set to worksforme
  • Status changed from new to closed

According to this:

https://hg.adblockplus.org/adblockpluscore/file/6f6a49dfae27/compiled/Map.h#l47

If we are not at the end and the current is invalid() we call operator++

It's already in there. Closing as WORKSFORME.

comment:4 Changed 3 months ago by hfiguiere

  • Resolution worksforme deleted
  • Status changed from closed to reopened

comment:5 Changed 3 months ago by hfiguiere

  • Owner set to hfiguiere

comment:6 Changed 3 months ago by hfiguiere

  • Review URL(s) modified (diff)
  • Status changed from reopened to reviewing

Adding tests.

comment:7 Changed 3 months ago by abpbot

A commit referencing this issue has landed:
Issue 6280 - Check that we don't return invalid entries

comment:8 Changed 3 months ago by hfiguiere

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.