Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add UPnP support #1537

Closed
wants to merge 3 commits into from
Closed

Add UPnP support #1537

wants to merge 3 commits into from

Conversation

Ansa89
Copy link
Contributor

@Ansa89 Ansa89 commented Mar 4, 2016

Rebase of #969.

@alexbakker
Copy link
Contributor

I would like to have UPnP configurable in Tox_Options. Something like bool upnp_enabled; would suffice.

@GrayHatter
Copy link
Collaborator

@tycho
Copy link
Contributor

tycho commented Mar 4, 2016

Tried this change out locally. Sadly failed to compile against miniupnpc v1.9.20151026:

../toxcore/network.c: In function ‘upnp_map_port’:
../toxcore/network.c:130:52: warning: passing argument 6 of ‘upnpDiscover’ makes integer from pointer without a cast [-Wint-conversion]
     devlist = upnpDiscover(1000, NULL, NULL, 0, 0, &error);
                                                    ^
In file included from ../toxcore/network.c:47:0:
/usr/include/miniupnpc/miniupnpc.h:62:1: note: expected ‘unsigned char’ but argument is of type ‘int *’
 upnpDiscover(int delay, const char * multicastif,
 ^
../toxcore/network.c:130:15: error: too few arguments to function ‘upnpDiscover’
     devlist = upnpDiscover(1000, NULL, NULL, 0, 0, &error);
               ^
In file included from ../toxcore/network.c:47:0:
/usr/include/miniupnpc/miniupnpc.h:62:1: note: declared here
 upnpDiscover(int delay, const char * multicastif,
 ^

The function prototype for upnpDiscover in miniupnpc is:

MINIUPNP_LIBSPEC struct UPNPDev *
upnpDiscover(int delay, const char * multicastif,
             const char * minissdpdsock, int localport,
             int ipv6, unsigned char ttl,
             int * error);

This is probably the change that broke it: miniupnp/miniupnp@1da63fa

@tycho
Copy link
Contributor

tycho commented Mar 4, 2016

One way to fix would be to check if MINIUPNPC_API_VERSION is >= 14, and add a default TTL argument of 2 if so. Or just make it require MINIUPNPC_API_VERSION >= 14 at configure time.

@Ansa89
Copy link
Contributor Author

Ansa89 commented Mar 5, 2016

@Impyy: done.
@tycho: done.
@GrayHatter: lol.

@Ansa89
Copy link
Contributor Author

Ansa89 commented Mar 6, 2016

Superseded by #1539.

@Ansa89 Ansa89 closed this Mar 6, 2016
@Ansa89 Ansa89 deleted the upnp branch March 27, 2016 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants