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

Build error with miniupnpc 2.2.8 (changed API for UPNP_GetValidIGD) #19333

Closed
4 of 6 tasks
VVD opened this issue Jul 17, 2024 · 10 comments
Closed
4 of 6 tasks

Build error with miniupnpc 2.2.8 (changed API for UPNP_GetValidIGD) #19333

VVD opened this issue Jul 17, 2024 · 10 comments
Milestone

Comments

@VVD
Copy link

VVD commented Jul 17, 2024

Platform

Linux / BSD

Compiler and build tool versions

Clang 18.1.5 + CMake 3.29.6

Operating system version

FreeBSD 14.1-p2 amd64

Build commands used

cd /usr/ports/emulators/ppsspp && make

What happens

I receive the following error:

/wrkdirs/usr/ports/emulators/ppsspp/work/ppsspp-1.17.1/Core/Util/PortManager.cpp:164:16: error: no matching function for call to 'UPNP_GetValidIGD'
  164 |                 int status = UPNP_GetValidIGD(devlist, urls, datas, lanaddr, sizeof(lanaddr)); //possible "status" values, 0 = NO IGD found, 1 = A valid connected IGD has been found, 2 = A valid IGD has been found but it reported as not connected, 3 = an UPnP device has been found but was not recognized as an IGD
      |                              ^~~~~~~~~~~~~~~~
/usr/local/include/miniupnpc/miniupnpc.h:122:1: note: candidate function not viable: requires 7 arguments, but 5 were provided
  122 | UPNP_GetValidIGD(struct UPNPDev * devlist,
      | ^                ~~~~~~~~~~~~~~~~~~~~~~~~~
  123 |                  struct UPNPUrls * urls,
      |                  ~~~~~~~~~~~~~~~~~~~~~~~
  124 |                  struct IGDdatas * data,
      |                  ~~~~~~~~~~~~~~~~~~~~~~~
  125 |                  char * lanaddr, int lanaddrlen,
      |                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  126 |                  char * wanaddr, int wanaddrlen);
      |                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

Commit with API breakage: miniupnp/miniupnp@c0a50ce
Downstream bug report: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=280298

Patch:

--- Core/Util/PortManager.cpp.orig      2024-02-04 13:08:02 UTC
+++ Core/Util/PortManager.cpp
@@ -161,7 +161,21 @@ bool PortManager::Initialize(const unsigned int timeou
 
                // Get LAN IP address that connects to the router
                char lanaddr[64] = "unset";
-               int status = UPNP_GetValidIGD(devlist, urls, datas, lanaddr, sizeof(lanaddr)); //possible "status" values, 0 = NO IGD found, 1 = A valid connected IGD has been found, 2 = A valid IGD has been found but it reported as not connected, 3 = an UPnP device has been found but was not recognized as an IGD
+/*
+possible "status" values:
+-1 = Internal error
+ 0 = NO IGD found
+ 1 = A valid connected IGD has been found
+ 2 = A valid connected IGD has been found but its IP address is reserved (non routable)
+ 3 = A valid IGD has been found but it reported as not connected
+ 4 = an UPnP device has been found but was not recognized as an IGD
+*/
+               int status =
+#if (MINIUPNPC_API_VERSION >= 18)
+                    UPNP_GetValidIGD(devlist, urls, datas, lanaddr, sizeof(lanaddr), nullptr, 0);
+#else
+                    UPNP_GetValidIGD(devlist, urls, datas, lanaddr, sizeof(lanaddr));
+#endif
                m_lanip = std::string(lanaddr);
                INFO_LOG(SCENET, "PortManager - Detected LAN IP: %s", m_lanip.c_str());
 

PPSSPP version affected

1.17.1

Last working version

No

Checklist

  • Test in the latest git build in case it's already fixed.
  • Make sure to run git submodule update --init --recursive before building.
  • Search for other reports of the same issue.
  • Try deleting the build directory and running the build again.
  • Check GitHub Actions, which builds every merge and PR.
  • Include logs and help us reproduce.
@hrydgard
Copy link
Owner

hrydgard commented Jul 17, 2024

I'm confused, doesn't seem like this API change has been made part of an official release of miniupnp yet? Hm, the change was in may and our fork was updated in june?

We should probably just upgrade our forked version, and do this change without the ifdef, when it's time.

@hrydgard hrydgard added this to the v1.18.0 milestone Jul 17, 2024
@VVD
Copy link
Author

VVD commented Jul 17, 2024

I'm confused, doesn't seem like this API change has been made part of an official release of miniupnp yet?

Already included in 2.2.8 release. Yes, it's API breakage in patch level update: 2.2.7 → 2.2.8. 👎

Hm, the change was in may and our fork was updated in june?

2.2.8 released month ago: https://github.com/miniupnp/miniupnp/releases/tag/miniupnpc_2_2_8

We should probably just upgrade our forked version, and do this change without the ifdef, when it's time.

Maybe. But the FreeBSD port emulators/ppsspp uses external miniupnpc from port net/miniupnpc.

@hrydgard
Copy link
Owner

Ok, I guess I'll just apply the patch with the version check then. Ugly.

@hrydgard
Copy link
Owner

Alright, PR submitted: #19335

@VVD
Copy link
Author

VVD commented Jul 17, 2024

You don't need old comment about status values.

@VVD
Copy link
Author

VVD commented Jul 17, 2024

Also status never used after call UPNP_GetValidIGD.

@hrydgard
Copy link
Owner

hrydgard commented Jul 17, 2024

I thought you wanted to add it for some reason since it was included in your patch above.

Yeah, should probably check it. I'll just log it out for now. I didn't write that code, heh.

freebsd-git pushed a commit to freebsd/freebsd-ports that referenced this issue Jul 17, 2024
Upstream bug report: hrydgard/ppsspp#19333

While here fix plist error in libretro-ppsspp and pet portclippy.

PR:	280298
@VVD
Copy link
Author

VVD commented Jul 17, 2024

I said about this line:
- int status = UPNP_GetValidIGD(devlist, urls, datas, lanaddr, sizeof(lanaddr)); //possible "status" values, 0 = NO IGD found, 1 = A valid connected IGD has been found, 2 = A valid IGD has been found but it reported as not connected, 3 = an UPnP device has been found but was not recognized as an IGD

@hrydgard
Copy link
Owner

ooh, I misunderstood, heh. Will restore it.

hrydgard added a commit that referenced this issue Jul 17, 2024
hrydgard added a commit that referenced this issue Jul 17, 2024
@VVD
Copy link
Author

VVD commented Jul 17, 2024

Thanks!

@VVD VVD closed this as completed Jul 17, 2024
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

No branches or pull requests

2 participants