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

Update to libupnp 1.8 #135

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@mrjimenez
Copy link

mrjimenez commented Oct 3, 2018

Hi folks,

This is to upgrade aMule to libupnp 1.8. The guys at Debian are trying to upgrade everything to 1.8, so lets not leave aMule out of it.

Maybe this should go on a branch that could be merged later.

Regards,
Marcelo.

@Stoatwblr

This comment has been minimized.

Copy link

Stoatwblr commented Oct 21, 2018

After patching in, compiles fine with the default 1.6 libs

With the 1.8 libupnp : (Ubuntu 18.10)

Compiling LoggerConsole.cpp
UPnPBase.cpp: In static member function ‘static int CUPnPControlPoint::Callback(Upnp_EventType_e, const void*, void*)’:
UPnPBase.cpp:1189:16: error: ‘expires’ was not declared in this scope
location, expires);
^~~~~~~
make[3]: *** [Makefile:1999: libmuleappcore_a-UPnPBase.o] Error 1
make[3]: *** Waiting for unfinished jobs....

@mrjimenez

This comment has been minimized.

Copy link

mrjimenez commented Oct 22, 2018

Compiles cleanly here, I am using libupnp 1.8 upstream. Check the second patch, line 1198. Expires is declared right before being used. It is likely that the patch was not correctly applied.

// Add the root device to our list    
int expires = UpnpDiscovery_get_Expires(d_event);    
upnpCP->AddRootDevice(rootDevice, urlBase,    
    location, expires);
@Stoatwblr

This comment has been minimized.

Copy link

Stoatwblr commented Oct 28, 2018

You're right, I hand patched and missed that line.

@HumanG33k

This comment has been minimized.

Copy link

HumanG33k commented Oct 29, 2018

its a good practice to compile with package lib not only with upstream.
And use the parent distro instead son. (debian instead ubuntu)
But good work

@gonosztopi

This comment has been minimized.

Copy link
Contributor

gonosztopi commented Nov 17, 2018

The change of void * to const void * in the declaration of CUPnPControlPoint::Callback makes compilation fail for me (using libupnp-1.6.24):

/home/devait/repos/git/amule-dev/src/UPnPBase.cpp: In constructor ‘CUPnPControlPoint::CUPnPControlPoint(short unsigned int)’:
/home/devait/repos/git/amule-dev/src/UPnPBase.cpp:843:56: error: invalid static_cast from type ‘int (*)(Upnp_EventType_e, const void*, void*)’ to type ‘Upnp_FunPtr {aka int (*)(Upnp_EventType_e, void*, void*)}’
   static_cast<Upnp_FunPtr>(&CUPnPControlPoint::Callback),
                                                        ^
Makefile:1964: recipe for target 'libmuleappcore_a-UPnPBase.o' failed
@sc0w

This comment has been minimized.

Copy link
Contributor

sc0w commented Nov 20, 2018

@gonosztopi

I can't see that warning, building with libupnp 1.8.4

I think is time to change the minimal version requirement

The build is ok here with this PR

@mrjimenez

This comment has been minimized.

Copy link

mrjimenez commented Nov 20, 2018

The change of void * to const void * in the declaration of CUPnPControlPoint::Callback makes compilation fail for me (using libupnp-1.6.24):

Not really a surprise, since the title of the pull request is "Update to libupnp 1.8" ;)

As stated in the first post, Debian will upgrade (soon ? :) ) to 1.8.

Is it worth testing for 1.6 in the patch?

Cheers!

@gonosztopi

This comment has been minimized.

Copy link
Contributor

gonosztopi commented Nov 21, 2018

The change of void * to const void * in the declaration of CUPnPControlPoint::Callback makes compilation fail for me (using libupnp-1.6.24):

Not really a surprise, since the title of the pull request is "Update to libupnp 1.8" ;)

My fault, I assumed it was a backwards-compatible upgrade :)

As stated in the first post, Debian will upgrade (soon ? :) ) to 1.8.

Is it worth testing for 1.6 in the patch?

I guess will upgrade means there are a lot of systems still using 1.6. Thus we need to be compatible with both.

@mrjimenez

This comment has been minimized.

Copy link

mrjimenez commented Nov 21, 2018

I guess will upgrade means there are a lot of systems still using 1.6. Thus we need to be compatible with both.

You are totally right, I'll find some time and fix it.

Cheers!

@sc0w

This comment has been minimized.

Copy link
Contributor

sc0w commented Nov 24, 2018

I fixed the build with libupnp 1.6 in #148

I just respect the commits in this PR and I added one more commit with the fix, and the build must work with libupnp 1.6 and 1.8

Please test #148

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment