relay_config.h: avoid redundant typedef for or_options_t

Freebsd cc doesn't like us re-declaring the or_options_t typedef in relay_config.h: we need to use the struct declaration instead.

changed milestone to %Tor: 0.4.3.x-final

added actualpoints::0.1 component::core tor/tor milestone::Tor: 0.4.3.x-final owner::nickm priority::high resolution::fixed reviewer::catalyst severity::normal sponsor::31-can status::closed tor-ci-failure type::defect labels

Trac:
Summary: relay_config.c: avoid redundant typedef for or_options_t to relay_config.h: avoid redundant typedef for or_options_t

Patch in bug32495; bug not in any Tor release. PR at https://github.com/torproject/tor/pull/1536

Trac:
Cc: N/A to teor

Trac:
Status: assigned to needs_review

Trac:
Reviewer: N/A to catalyst

A quick grep shows some additional possible duplicates:

../src/core/or/or.h:typedef struct or_options_t or_options_t;
../src/feature/dirauth/dirauth_config.h:typedef struct or_options_t or_options_t;
../src/feature/relay/relay_config.h:typedef struct or_options_t or_options_t;
../src/feature/relay/transport_config.h:typedef struct or_options_t or_options_t;

Do we know why relay_config.h is the only one that causes problems?

I'm afraid I don't; it might have to do with whether the typedef is seen after the definition of struct or_options_t or not.

The Jenkins log at https://jenkins.torproject.org/job/tor-ci-freebsd-amd64-master/388/consoleFull does show the other headers causing problems with redefining or_options_t.

Also from the Jenkins log, I also see redefinitions of smartlist_t that occur in relay_config.h, transport_config.h, but not dirauth_config.h.

For or_options_t, I see several possible options:

• Leave the or_options_t typedef in or.h, but require that inclusion of or.h precede any other headers that require that typedef. That's not great from a standpoint of modularizing our headers.
• Remove the or_options_t typedef from or.h, and define it in a shared header file.
• Leave the or_options_t typedef in or.h, but remove all references to the typedef in header files (using the struct tag instead).

Other possibilities might exist. What do you think?

For the smartlist_t redefinitions, maybe we can explicitly include smartlist.h in those headers? Is there a reason not to?

Also, it's probably worth noting that when I tried write minimal test cases to produce this warning, I was only able to do so on clang, and even then only if using options such as -std=c99. So maybe FreeBSD adjusts some of the warning behaviors. Note this means that Travis CI probably won't detect future instances of this problem.

Trac:
Status: needs_review to needs_revision

On IRC, catalyst helped me figure out how to reproduce this: using --std=gnu99.

The configure line that worked for me was: ./configure --enable-fatal-warnings CFLAGS='-std=gnu99 -O2' CC=clang.

It does seem that the other redundant typedefs were also causing warnings. I've force-pushed a better version of this fix.

Trac:
Status: needs_revision to needs_review

Trac:
Actualpoints: 0 to 0.1

Created #32500 (moved) for possible CI improvement to avoid recurrence.

Replying to nickm:

It does seem that the other redundant typedefs were also causing warnings. I've force-pushed a better version of this fix. Thanks! Looks good.

Trac:
Status: needs_review to merge_ready

CI passes, and it also passes with -std=gnu99 in #32500 (moved). Merged to master.

Trac:
Resolution: N/A to fixed
Status: merge_ready to closed

closed

added 48m of time spent

mentioned in issue #32500 (moved)

moved to tpo/core/tor#32495 (closed)

mentioned in issue tpo/core/tor#32500 (closed)