Opened 4 months ago

Closed 3 months ago

#32415 closed task (fixed)

Rename every struct we declare to end with _t.

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.4.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: network-team-roadmap-november
Cc: Actual Points: .2
Parent ID: Points: .1
Reviewer: teor Sponsor: Sponsor31-can

Description

Per our style poll, we prefer this:

typedef struct foo_t foo_t;

over this:

typedef struct foo_s foo_t;

So let's replace all the cases where we do the latter.

Child Tickets

Attachments (2)

rename.sh (2.0 KB) - added by nickm 4 months ago.
rename.2.sh (2.0 KB) - added by nickm 4 months ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 4 months ago by nickm

Here is a list:

src/app/main/ntmain.c:struct service_fns {
src/core/mainloop/cpuworker.c:typedef struct worker_state_s {
src/core/mainloop/cpuworker.c:typedef struct cpuworker_job_u {
src/core/or/channel.c:typedef struct channel_idmap_entry_s {
src/core/or/channel.h:struct channel_s {
src/core/or/channel.h:struct channel_listener_s {
src/core/or/channeltls.h:struct channel_tls_s {
src/core/or/circuitmux.c:struct circuit_muxinfo_s {
src/core/or/circuitmux.h:struct circuitmux_policy_s {
src/core/or/circuitmux.h:struct circuitmux_policy_data_s {
src/core/or/circuitmux.h:struct circuitmux_policy_circ_data_s {
src/core/or/circuitmux.h:struct circuitmux_s {
src/core/or/circuitmux_ewma.h:struct cell_ewma_s {
src/core/or/circuitmux_ewma.h:struct ewma_policy_data_s {
src/core/or/circuitmux_ewma.h:struct ewma_policy_circ_data_s {
src/core/or/circuitstats.h:struct circuit_build_times_s {
src/core/or/relay.h:typedef struct address_ttl_s {
src/core/or/scheduler.h:typedef struct scheduler_s {
src/core/or/scheduler.h:typedef struct socket_table_ent_s {
src/core/or/scheduler_kist.c:typedef struct outbuf_table_ent_s {
src/feature/client/entrynodes.h:struct guard_selection_s {
src/feature/control/control_events.c:typedef struct queued_event_s {
src/feature/control/control_events.c:static struct cached_bw_event_s {
src/feature/dirauth/bwauth.c:typedef struct mbw_cache_entry_s {
src/feature/dirauth/dircollate.c:typedef struct ddmap_entry_s {
src/feature/dirauth/dircollate.h:struct dircollator_s {
src/feature/dircache/dircache.c:typedef struct url_table_ent_s {
src/feature/dircommon/fp_pair.c:struct fp_pair_map_entry_s {
src/feature/dircommon/fp_pair.c:struct fp_pair_map_s {
src/lib/cc/compat_compiler.h: *   struct a { int foo; int bar; } x;
src/lib/cc/compat_compiler.h: *   struct base { ... };
src/lib/cc/compat_compiler.h: *   struct subtype { int x; struct base b; } x;
src/lib/container/handles.h: *     struct walrus {
src/lib/container/map.c:  struct maptype {                                        \
src/lib/crypt_ops/aes_openssl.c:struct aes_cnt_cipher {
src/lib/crypt_ops/crypto_rand_fast.c:  struct cbuf {
src/lib/evloop/compat_libevent.h:typedef struct tor_libevent_cfg {
src/lib/evloop/timers.c:struct timeout_cb {
src/lib/evloop/workqueue.c:struct threadpool_s {
src/lib/evloop/workqueue.c:struct workqueue_entry_s {
src/lib/evloop/workqueue.c:struct replyqueue_s {
src/lib/evloop/workqueue.c:typedef struct workerthread_s {
src/lib/math/prob_distr.h:*     struct foo {
src/lib/math/prob_distr.h:struct dist_ops {
src/lib/math/prob_distr.h:struct geometric {
src/lib/math/prob_distr.h:struct genpareto {
src/lib/math/prob_distr.h:struct weibull {
src/lib/math/prob_distr.h:struct log_logistic {
src/lib/math/prob_distr.h:struct logistic {
src/lib/math/prob_distr.h:struct uniform {
src/lib/net/inaddr_st.h:struct sockaddr_in6 {
src/lib/sandbox/sandbox.c:#define SCMP_CMP(a,b,c) ((struct scmp_arg_cmp){(a),(b),(c),0})
src/lib/sandbox/sandbox.c:  ((struct scmp_arg_cmp) {(a),(b),(intptr_t)(void*)(c),0})
src/lib/sandbox/sandbox.c:#define SCMP_CMP4(a,b,c,d) ((struct scmp_arg_cmp){(a),(b),(c),(d)})
src/lib/sandbox/sandbox.h:typedef struct smp_param {
src/lib/sandbox/sandbox.h:struct sandbox_cfg_elem {
src/lib/thread/threads.h:typedef struct tor_threadlocal_s {
src/lib/time/compat_time.h:struct timeval {
src/test/test-memwipe.c:static struct testcase {
src/test/test_addr.c:  static const struct loopback_item {
src/test/test_dispatch.c:struct coord { int x; int y; };
src/test/test_link_handshake.c:typedef struct certs_data_s {
src/test/test_link_handshake.c:typedef struct authchallenge_data_s {
src/test/test_link_handshake.c:typedef struct authenticate_data_s {
src/test/test_threads.c:typedef struct cv_testinfo_s {
src/test/test_workqueue.c:typedef struct state_s {
src/test/test_workqueue.c:typedef struct rsa_work_s {
src/test/test_workqueue.c:typedef struct ecdh_work_s {

I made it with the command:

git grep 'struct *[a-zA-Z0-9_]*[^t ] *{' \
      src/{app,core,feature,lib,test,tools} |grep -v '^static '

Changed 4 months ago by nickm

Attachment: rename.sh added

Changed 4 months ago by nickm

Attachment: rename.2.sh added

comment:2 Changed 4 months ago by nickm

Actual Points: .1
Owner: set to nickm
Status: newaccepted

Okay, that was fun. See branch ticket32415 with PR at https://github.com/torproject/tor/pull/1517 .

This is a mostly automated branch, with a couple of special cases that needed to be done by hand.

comment:3 Changed 4 months ago by nickm

Status: acceptedneeds_review

comment:4 Changed 4 months ago by asn

Reviewer: teor

comment:5 Changed 3 months ago by teor

Actual Points: .1.2
Status: needs_reviewneeds_revision

Here are some missed structs, excluding those structs that are defined by libraries:

$ git grep 'struct *[a-zA-Z0-9_]*[^t ] *{' \
    src/{app,core,feature,lib,test,tools} \
  | grep -v ': *static '
src/lib/cc/compat_compiler.h: *   struct a { int foo; int bar; } x;
src/lib/cc/compat_compiler.h: *   struct base { ... };
src/lib/cc/compat_compiler.h: *   struct subtype { int x; struct base b; } x;
src/lib/container/handles.h: *     struct walrus {
src/lib/container/map.c:  struct maptype {                                        \
src/lib/math/prob_distr.h:*     struct foo {
... (spurious matches) ...
$ git grep 'struct  *[a-zA-Z0-9_]*[^t ] \**[a-zA-Z0-9_]*[,)]' \
    src/{app,core,feature,lib,test,tools} \
  | grep -v -e time -e addr -e sipkey -e tm -e SEC -e PR -e passwd -e event
src/core/or/channel.c:channel_rsa_id_group_set_badness(struct channel_list_s *lst, int force)
src/lib/cc/compat_compiler.h: *   ptrdiff_t bar_offset = offsetof(struct a, bar);
src/lib/cc/compat_compiler.h: *   struct *sp = SUBTYPE_P(bp, struct subtype, b);
src/lib/container/handles.h: *     struct wlr_handle_t *wlr_handle_new(struct walrus *);
src/lib/container/handles.h: *     void wlr_handles_clear(struct walrus *);
src/lib/container/handles.h:  linkage  name ## _handle_t *name ## _handle_new(struct structname *object); \
src/lib/container/handles.h:  linkage void name ## _handles_clear(struct structname *object);
src/lib/container/handles.h:  name ## _handle_new(struct structname *object)                        \
src/lib/container/handles.h:  name ## _handles_clear(struct structname *object)                     \
... (spurious matches) ...

Most of these are comments, which should be manually fixed to match our style.

struct maptype is a macro argument, we should go a global rename, just like we did for the probability dist functions:

  • struct map_t - too short?
  • struct maptype_t - seems repetitive?
  • struct map_value_t ?

struct channel_list_s is defined by TOR_LIST_HEAD(), I'm not sure if it will work if we pass channel_t twice. If not, we should pass something ending in _t.

struct structname should probably become struct handle_value_t, using the same pattern we use for map.

comment:6 Changed 3 months ago by nickm

Status: needs_revisionneeds_review

Okay, I've added more commits to the branch, to try to handle the issues above.

comment:7 Changed 3 months ago by teor

Keywords: network-team-roadmap-november added
Resolution: fixed
Status: needs_reviewclosed

Thanks, merged to master.

Note: See TracTickets for help on using tickets.