Opened 6 days ago

Last modified 3 days ago

#30184 needs_review defect

release-0.2.9 doesn't compile on old rhel

Reported by: arma Owned by:
Priority: Medium Milestone: Tor: 0.2.9.x-final
Component: Core Tor/Tor Version: Tor: unspecified
Severity: Normal Keywords: no-mainline-merge, regression, fast-fix, consider-backport-after-0404
Cc: rl1987, teor Actual Points: 0.2
Parent ID: Points: 0.1
Reviewer: Sponsor: SponsorQ

Description

On rhel6, building release-0.2.9 (git commit ca008906), I get

  CC     src/or/src_or_libtor_testing_a-rephist.o
src/or/rephist.c:91: error: redefinition of typedef ‘bw_array_t’
src/or/rephist.h:120: note: previous declaration of ‘bw_array_t’ was here
make[1]: *** [src/or/src_or_libtor_testing_a-rephist.o] Error 1

Looks like when we backported some stuff, we didn't backport all of the subsequent fixes on the stuff.

Here is a fix that lets it compile again (there could for sure be a better fix than this):

index dc86fad..d8f7eb2 100644
--- a/src/or/rephist.c
+++ b/src/or/rephist.c
@@ -88,7 +88,6 @@
 static void bw_arrays_init(void);
 static void predicted_ports_init(void);
 
-typedef struct bw_array_t bw_array_t;
 STATIC uint64_t find_largest_max(bw_array_t *b);
 STATIC void commit_max(bw_array_t *b);
 STATIC void advance_obs(bw_array_t *b);
diff --git a/src/or/rephist.h b/src/or/rephist.h
index c464b34..072987f 100644
--- a/src/or/rephist.h
+++ b/src/or/rephist.h
@@ -114,10 +114,10 @@ void rep_hist_log_link_protocol_counts(void);
 
 extern uint64_t rephist_total_alloc;
 extern uint32_t rephist_total_num;
+typedef struct bw_array_t bw_array_t;
 #ifdef TOR_UNIT_TESTS
 extern int onion_handshakes_requested[MAX_ONION_HANDSHAKE_TYPE+1];
 extern int onion_handshakes_assigned[MAX_ONION_HANDSHAKE_TYPE+1];
-typedef struct bw_array_t bw_array_t;
 extern bw_array_t *write_array;
 #endif

(My bwauth still runs on 0.2.9, since I'm under the impression that's the last version that works well with bwauths. That's how I noticed.)

Child Tickets

Change History (8)

comment:1 Changed 5 days ago by rl1987

Cc: rl1987 added

comment:2 Changed 5 days ago by nickm

Cc: teor added
Keywords: regression added
Milestone: Tor: 0.2.9.x-final

I'd like to assign this one to you, Teor, if that's okay.

comment:3 Changed 5 days ago by nickm

(It is not by an means urgent)

comment:4 in reply to:  description Changed 5 days ago by teor

Actual Points: 0.1
Keywords: no-mainline-merge consider-backport-after-0404-alpha fast-fix added
Points: 0.1
Sponsor: SponsorQ
Status: newneeds_revision
Version: Tor: unspecified

Marking as SponsorQ, because this bug was introduced by a SponsorQ ticket.

Marking for backport in the current batch, because it's a trivial compilation fix that is already in 0.3.4 and later.

Replying to arma:

On rhel6, building release-0.2.9 (git commit ca008906), I get

  CC     src/or/src_or_libtor_testing_a-rephist.o
src/or/rephist.c:91: error: redefinition of typedef ‘bw_array_t’
src/or/rephist.h:120: note: previous declaration of ‘bw_array_t’ was here
make[1]: *** [src/or/src_or_libtor_testing_a-rephist.o] Error 1

Looks like when we backported some stuff, we didn't backport all of the subsequent fixes on the stuff.

We merged 1da9741bca to master, but I removed the equivalent commit from 0.2.9, because the CI failed:
https://trac.torproject.org/projects/tor/ticket/23512#comment:38

But the extern declaration is slightly different in 0.2.9 and 0.3.4, due to merge commit 813019cc57:

++extern struct bw_array_t *write_array;
++#endif
++
++#ifdef REPHIST_PRIVATE
 +typedef struct bw_array_t bw_array_t;
- extern bw_array_t *write_array;

I tried cherry-picking 1da9741bca, then adding the struct in a separate commit. (It was added to 0.3.4 in the merge commit.)

Here is the pull request:

But that failed with:

src/test/test_relay.c:22:27: error: must use 'struct' tag to refer to type
      'bw_array_t'
uint64_t find_largest_max(bw_array_t *b);
                          ^
                          struct 
src/test/test_relay.c:23:17: error: must use 'struct' tag to refer to type
      'bw_array_t'
void commit_max(bw_array_t *b);
                ^
                struct 
src/test/test_relay.c:24:18: error: must use 'struct' tag to refer to type
      'bw_array_t'
void advance_obs(bw_array_t *b);
                 ^
                 struct 

So maybe I should backport the struct tags as well?

(My bwauth still runs on 0.2.9, since I'm under the impression that's the last version that works well with bwauths. That's how I noticed.)

I'm not sure if that's true.
You should ask the other torflow operators what they're running?

(I wish I could tell you, but we didn't think of the tor version when we were doing the headers for sbws 1.0 or 1.1. We'll add it in #30196.)

comment:5 Changed 4 days ago by teor

Status: needs_revisionneeds_review

I added another typedef, we should be ok now.

The merge commit 813019cc57 was quite complex, maybe we should put fixes in separate commits after any merges.

comment:6 Changed 4 days ago by arma

confirmed that pull/957/head compiles for me, when release-0.2.9 does not. Thanks!

comment:7 Changed 4 days ago by teor

Actual Points: 0.10.2

Here are the pull requests:

I'm still going to get someone to review this, before I merge.

comment:8 Changed 3 days ago by teor

Keywords: consider-backport-after-0404 added; consider-backport-after-0404-alpha removed

Drop the -alpha from backport tags

Note: See TracTickets for help on using tickets.