1509562 - Organizational improvements when defining redirections

Status: RESOLVED FIXED

(Core Graveyard :: Web Replay, enhancement)

Description

[Brian Hackett [Laid off!]]

Attachment: patch

There are a couple of annoying things about how redirections are defined:

- Redirections are defined as a flat array, which makes it hard to have different redirections which are installed in different ways, or to (in the future) share sets of redirections between different platforms.

- Redirections are defined via a gigantic macro, which is ugly, breaks some editors (in particular, the editor I use), and makes it hard to diagnose certain compiler errors.

The attached patch does a bunch of reorganization to fix both of these, while avoiding making any functional changes.

Comment 1

[Brian Hackett [Laid off!]]

Attachment: Part 1 - Remove restriction on redirections being a statically defined array.

Allow platforms to internally manage how their redirections are organized, allowing other parts of the system to access the redirections with NumRedirections() and GetRedirection(n) methods.

Comment 2

[Brian Hackett [Laid off!]]

Attachment: Part 2 - Avoid UAF when resetting middleman calls.

Fix a bug exposed by later patches in this bug.  When resetting middleman calls, releasing the resources held via a call can require accessing data from earlier middleman calls.  These calls may have already been deleted, leading to a use-after-free error.  The attached patch deletes the calls in a separate pass to avoid this.

Comment 3

[Brian Hackett [Laid off!]]

Attachment: Part 3 - Remove dependence on CallEvent_* enum.

Defining the redirections outside a macro means that we will no longer be able to use the CallEvent_* enum values to refer to the original unredirected function pointer for a given redirection name.  Uses of these enums are pretty limited, and this patch adds an alternative way to refer to the original unredirected function pointer for the 25 or so functions that need this behavior.

Comment 4

[Brian Hackett [Laid off!]]

Attachment: Part 4 - Define redirections directly, instead of with a giant macro.

Replace the gigantic macro used to define the redirections with a normal static initializer.  This also renames the Middleman_* functions to MM_* for compactness and to more closely mirror the similar RR_* functions.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f77f0486954
Part 1 - Remove restriction on redirections being a statically defined array, r=lsmyth.
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb62b5dc4dbe
Part 2 - Avoid UAF when resetting middleman calls, r=lsmyth.
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0efb6f89bd0
Part 3 - Remove dependence on CallEvent_* enum, r=lsmyth.
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9f41dd78cfd
Part 4 - Define redirections directly, instead of with a giant macro, r=lsmyth.

Comment 6

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/3f77f0486954
https://hg.mozilla.org/mozilla-central/rev/bb62b5dc4dbe
https://hg.mozilla.org/mozilla-central/rev/d0efb6f89bd0
https://hg.mozilla.org/mozilla-central/rev/b9f41dd78cfd