#18320: Clear old entries from the key-pinning journal file -------------------------------------------------+------------------------- Reporter: teor | Owner: andrea Type: defect | Status: Priority: Medium | needs_revision Component: Core Tor/Tor | Milestone: Tor: Severity: Normal | 0.2.9.x-final Keywords: tor-dos, TorCoreTeam201606, review- | Version: group-3 | Resolution: Parent ID: #17293 | Actual Points: Reviewer: dgoulet | Points: 3 | Sponsor: | SponsorU-can -------------------------------------------------+------------------------- Changes (by dgoulet):
* status: needs_review => needs_revision * reviewer: => dgoulet Comment: General comment. I'm not so cheerful about the keypin code directly using HT_* instead of the tested and robust interface we have that are `digestmap` and `digest256map` _especially_ when we use RSA and ed25519 digest as keys. Maybe there is a reason? I don't see one for now nor see one in the comment. So I understand why this patch is using the HT_* interface but it adds quite a bit of complexity for review... Re-implementing the add/remove/iteration is really not ideal as we duplicate lots of things. Why the double linked list? Isn't a `smartlist_t` ordered and when removing we can make sure to keep it with `smartlist_del_keeporder()`? This would simplify things since implementing a double linked list in this file is not ideal I would say. Some code comment: DG1: `keypin_add_or_replace_entry_in_map()` can return -2 I believe but it's not documented. DG2: This `if(pruner)` is not useful there: `if (pruner) keypin_free_pruner(pruner);` DG3: In `keypin_prune_journal()`, the pruned journal is written to disk only if `r == 0` that is we parsed it successfully. However, on error the `log_info(...)` indicating that we've pruned the journal is given. Maybe if `r` is an error, we should `goto err`? (might be confusing with `strerror(errno)` though). DG4: `keypin_add_line_to_pruner()`, I think `len` should be `size_t` here. Ok, done for now. -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/18320#comment:20> Tor Bug Tracker & Wiki <https://trac.torproject.org/> The Tor Project: anonymity online _______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs