mod.openchanfix: lostOp() leaks the op set it allocates, and orphans it when the last entry is erased

[MrIron-no]

mod.openchanfix keeps, per client, the set of channels in which that client holds op, hung on the client's custom-data slot. Two allocations of that set are never freed.

1. The set allocated for a client that has none is dropped.

chanfix::findMyOps() (mod.openchanfix/chanfix.cc:2695) allocates when the client has no custom data:

chanfix::clientOpsType* chanfix::findMyOps(iClient* theClient) {
    clientOpsType* myOps = static_cast<clientOpsType*>(theClient->getCustomData(this));

    if (myOps == NULL)
        myOps = new clientOpsType;

    return myOps;
}

chanfix::lostOp() (chanfix.cc:1668) then returns at the emptiness check without freeing it or attaching it to the client:

void chanfix::lostOp(const std::string& channel, iClient* theClient, clientOpsType* myOps) {
    if (myOps == NULL)
        myOps = findMyOps(theClient);

    if (myOps->empty())
        return;

Reachable whenever lostOp() is called with a null set for a client that has never held op — the part path at chanfix.cc:671 and the deop path. On a busy network that is one std::set leaked per part by a non-op.

The quit/kill path is not affected because it handles the empty case itself (chanfix.cc:855-861, delete myOps), which is what makes the omission in lostOp() look accidental rather than intended.

2. The set is orphaned when its last entry is erased.

    clientOpsType::iterator ptr = myOps->find(channel);
    if (ptr != myOps->end()) {
        myOps->erase(ptr);
        theClient->removeCustomData(this);
        if (!myOps->empty())
            theClient->setCustomData(this, static_cast<void*>(myOps));
    }

When the erase empties the set, removeCustomData() detaches it and neither setCustomData() nor delete follows, so the object is lost. This happens to every client that loses op in the last channel it held it in.

Related, not a live bug: at chanfix.cc:858 the walk passes a reference to a set element that lostOp() then erases at :1677:

for (clientOpsType::iterator ptr = myOps->begin(); ptr != myOps->end();)
    lostOp(*ptr++, theClient, myOps);

lostOp()'s channel parameter is a const std::string& bound to that element. It is safe today only because channel is not read after the erase; any future line in lostOp() that reads it after that point would be a use-after-free.