Opened 4 years ago

Closed 4 years ago

#340 closed Patch submission (fixed)

BSD net/route.h

Reported by: bluhm Owned by: cron2
Priority: minor Milestone: release 2.3.3
Component: Building / Compiling Version: OpenVPN 2.3.2 (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords:
Cc:

Description

Hi,

Your source file src/openvpn/route.c contains parts that have been
copied from the OpenBSD header file net/route.h. As a consequence
you have to update your sources whenever this system header file
changes. This will happen soon as I will change the OpenBSD
rmx_expire field to 64 bit.

The propper solution would be to #include <net/route.h> instead of
copying it. You cannot do that right now as there is a name space
conflict with the BSD struct route. Please consider renaming your
struct route to someting else as the BSD route is the older name.
Then you can just include net/route.h and also the code for the
other BSDs gets nicer.

See also http://marc.info/?l=openbsd-ports&m=138219274420294&w=2

bluhm

Attachments (1)

openvpn-route.diff (10.4 KB) - added by bluhm 4 years ago.
Patch rename struct route to route_base and include net/route.h

Download all attachments as: .zip

Change History (6)

Changed 4 years ago by bluhm

Patch rename struct route to route_base and include net/route.h

comment:1 Changed 4 years ago by cron2

I hear you, and need to think about the right way forward... (openvpn git master only, master and 2.3 branch? will this impact non-BSD platforms? who is actually using the structure we copied over? etc).

comment:2 Changed 4 years ago by samuli

  • Owner set to cron2
  • Status changed from new to assigned

comment:3 Changed 4 years ago by cron2

  • Milestone set to release 2.3.3
  • Version changed from git master branch to 2.3.2

Looked at the patch, it all makes sense to me. I want it in 2.3.3, but it's borderline, so put it up for discussion on theopenvpn-devel list.

Unless people NAK me, I'll integrate it "soonish", propably next weekend.

comment:4 Changed 4 years ago by cron2

Just as a side note: #42 has the same issue on Darwin/MacOS X, but that got fixed in the 2.1 time frame already (removing local definitions, including <net/route.h>).

comment:5 Changed 4 years ago by cron2

  • Resolution set to fixed
  • Status changed from assigned to closed

So, here we go. I splitted the patch into "rename struct route" and "remove the local definitions" to make it easier to see which changes are related, and which ones are just prerequisites.

Both has been included in master and release/2.3.

Rename 'struct route' to 'struct route_ipv4':
commit b57e005b8f232760081875937a53e8e7d235faa6 (master)
commit 07b5c1d02579b5e897a0e5e0c80f0ba0098220d7 (release/2.3)

Replace copied structure elements with including <net/route.h>:
commit 615fb9ef36310f85fd6171301128a12740444455 (master)
commit 15e0d0f5674bfe9a2de17095c6273b15b0eeec22 (release/2.3)

There will be another commit to master only which will merge the get_default_gateway() code for all 5 BSD variants. But as that's more intrusive it will not go into 2.3

Thanks for bringing this up, and for your patience. I think this ticket is solved now.

Note: See TracTickets for help on using tickets.