Opened 2 years ago

Last modified 14 months ago

#22220 new enhancement

hs: Move cell encoding/decoding out of hs_intropoint.c to hs_cell.c

Reported by: dgoulet Owned by:
Priority: Low Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-hs, prop224, prop224-extra refactor code-movement
Cc: Actual Points:
Parent ID: Points: 0.1
Reviewer: Sponsor: SponsorR-can

Description

The hs_cell.c file is added by #20657 so once it's merged, let's do this so we don't have cell mangling in different files everywhere.

Child Tickets

Change History (9)

comment:1 Changed 2 years ago by dgoulet

Keywords: prop224-extra added

comment:2 Changed 2 years ago by dgoulet

Milestone: Tor: 0.3.2.x-finalTor: unspecified

We can't make those for 032 so for now they go in Unspecified.

comment:3 Changed 2 years ago by nickm

Keywords: refactor code-movement added

comment:4 Changed 15 months ago by neel

Cc: neel@… added
Owner: set to neel
Status: newassigned

comment:5 Changed 15 months ago by neel

In hs_intropoint.c, I noticed code like:

  encoded_len = trn_cell_intro_established_encoded_len(cell);
  tor_assert(encoded_len > 0);
  encoded_cell = tor_malloc_zero(encoded_len);
  result_len = trn_cell_intro_established_encode(encoded_cell, encoded_len,
                                                cell);
  tor_assert(encoded_len == result_len);

in hs_intro_send_intro_established_cell(), and:

  encoded_len = trn_cell_introduce_ack_encoded_len(cell);
  tor_assert(encoded_len > 0);
  encoded_cell = tor_malloc_zero(encoded_len);
  result_len = trn_cell_introduce_ack_encode(encoded_cell, encoded_len, cell);
  tor_assert(encoded_len == result_len);

in send_introduce_ack_cell().

Would it be okay if I have a function in hs_cell.c which does the encoding, and I pass in the function calls to get the cell length and encoded cells? The functions I am thinking about passing are:

  • Either trn_cell_intro_established_encoded_len() or trn_cell_introduce_ack_encoded_len(), and
  • Either trn_cell_intro_established_encode() or trn_cell_introduce_ack_encode()

And in the new function, I return a ssize_t for encoded_len, and pass in the functions, the cell itself, and the pointer to *encoded_cell to get returned the cell itself.

comment:6 Changed 15 months ago by dgoulet

I think for this case, a #define macro would be more appropriate. You would pass the "get encoded len" fn() and the "do encoding" fn() with other things.

comment:7 Changed 15 months ago by neel

If I do a #define macro, I don't think I would be able to place it in hs_cell.c. Would it make more sense to put this macro in hs_cell.h or hs_intropoint.c?

comment:8 Changed 14 months ago by neel

Cc: neel@… removed
Owner: neel deleted

comment:9 Changed 14 months ago by neel

Status: assignednew
Note: See TracTickets for help on using tickets.