[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #13827 [Core Tor/Tor]: Cell handling code duplication in channel.c
#13827: Cell handling code duplication in channel.c
-----------------------------------------------+---------------------------
Reporter: rl1987 | Owner: pingl
Type: defect | Status:
| needs_revision
Priority: Medium | Milestone: Tor:
| 0.2.9.x-final
Component: Core Tor/Tor | Version:
Severity: Normal | Resolution:
Keywords: refactoring, easy, review-group-9 | Actual Points:
Parent ID: | Points: 0.1
Reviewer: dgoulet | Sponsor:
-----------------------------------------------+---------------------------
Comment (by pingl):
Please check the new channel.c attached in the previous post. I think you
are right about DG1 and also there is an issue about the fact that the
dereferencing is not available outside the switch scope. That means that
cell would remain of type void*. For these reasons I've moved the (still a
bit duplicated) code into the switch cases. I'll add the default case (I
wasn't sure how to manage it) and change the `ctype`.
Thanks for the comments, I am new to tor.
Replying to [comment:12 dgoulet]:
> (I've pull your patch into `bug13827_029_01` if anyone wants to see it
from git.tpo in my repo.)
>
> DG1: This worries me:
> {{{
> - q.u.var.var_cell = var_cell;
> + q.u.fixed.cell = cell;
> }}}
>
> In theory, that could work since it's a union and all cell points there
but kind of recipe for disaster and bad semantic. What you could do is
take a reference on the right cell member of the union while in the switch
case and then assign it after.
>
> DG2: Can't you use `CELL_QUEUE_*` as the cell type?
>
> DG3: Few things. I would rename `ctype` to `cell_type`. The switch case
MUST have a default branch that you could do a BUG() on and bail. Finally,
no need for extra space between the case and the end of the function.
>
> Thanks for this!
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/13827#comment:13>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs