Wrong error codes used in upnp.c

[rb07]

It seems that the wrong variable, and wrong error codes are currently being used in tr_upnpDiscover().

It should be something like this:

Index: libtransmission/upnp.c
===================================================================
--- libtransmission/upnp.c      (revision 14017)
+++ libtransmission/upnp.c      (working copy)
@@ -98,11 +98,11 @@
 #elif defined (HAVE_MINIUPNP_15)
     ret = upnpDiscover (msec, NULL, NULL, 0);
 #else
-    ret = UPNPCOMMAND_UNKNOWN_ERROR;
+    ret = UPNPDISCOVER_UNKNOWN_ERROR;
 #endif

-    if (ret != UPNPCOMMAND_SUCCESS)
-        tr_logAddNamedDbg (getKey (), "upnpDiscover failed (errno %d - %s)", err, tr_strerror (err));
+    if (err != UPNPDISCOVER_SUCCESS)
+        tr_logAddNamedDbg (getKey (), "upnpDiscover failed (errno %d - %s)", errno, tr_strerror (errno));

     return ret;
 }

this is assuming errno is preserved by the underlying function, since its not touched, and the value returned in err doesn't have the OS error code.

BTW version 1.8 of miniupnpc was released this week, no changes w.r.t. this problem or the API (except perhaps to change the guards to include the newer versions).

[jordan]

if we're going to do the symbolic values of "err" correctly, we shouldn't initialize it to zero either. :)

r14020: "(libT) #5293 'Wrong error codes used in upnp.c': patch by rb07"

[jordan]

r14021: "(2.7x) backport r14020 to fix bug #5278 in 2.7x"

[taem]

Hi,

Can't build the code with r14020

gcc -DPACKAGE_NAME=\"transmission\" -DPACKAGE_TARNAME=\"transmission\" -DPACKAGE_VERSION=\"2.76+\" -DPACKAGE_STRING=\"transmission\ 2.76+\" -DPACKAGE_BUGREPORT=\"http://trac.transmissionbt.com/newticket\" -DPACKAGE_URL=\"\" -DPACKAGE=\"transmission\" -DVERSION=\"2.76+\" -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 -DLT_OBJDIR=\".libs/\" -DSTDC_HEADERS=1 -DTIME_WITH_SYS_TIME=1 -DHAVE_STDBOOL_H=1 -DHAVE_ICONV_OPEN=1 -DHAVE_PREAD=1 -DHAVE_PWRITE=1 -DHAVE_DAEMON=1 -DHAVE_DIRNAME=1 -DHAVE_BASENAME=1 -DHAVE_STRCASECMP=1 -DHAVE_LOCALTIME_R=1 -DHAVE_FALLOCATE64=1 -DHAVE_POSIX_FALLOCATE=1 -DHAVE_MEMMEM=1 -DHAVE_STRSEP=1 -DHAVE_STRTOLD=1 -DHAVE_SYSLOG=1 -DHAVE_VALLOC=1 -DHAVE_GETPAGESIZE=1 -DHAVE_POSIX_MEMALIGN=1 -DHAVE_STATVFS=1 -DHAVE_MKDTEMP=1 -DHAVE_PTHREAD=1 -DHAVE__TMP_DUMMY1_ZLIB_H=1 -DHAVE_ZLIB=1 -D_FILE_OFFSET_BITS=64 -DHAVE_LSEEK64=1 -DHAVE_GETMNTENT=1 -DHAVE_DECL_POSIX_FADVISE=1 -DHAVE_POSIX_FADVISE=1 -DWITH_INOTIFY=1 -DHAVE_SYS_STATVFS_H=1 -DHAVE_XFS_XFS_H=1 -DWITH_UTP=1 -DHAVE_MINIUPNP_15=1 -DSYSTEM_MINIUPNP=1 -DHAVE_LIBINTL_H=1 -DGETTEXT_PACKAGE=\"transmission-gtk\" -DHAVE_LOCALE_H=1 -DHAVE_LC_MESSAGES=1 -DHAVE_BIND_TEXTDOMAIN_CODESET=1 -DHAVE_GETTEXT=1 -DHAVE_DCGETTEXT=1 -DENABLE_NLS=1 -I.  -I.. -D__TRANSMISSION__ -DPACKAGE_DATA_DIR=\""/usr/share"\" -D_FORTIFY_SOURCE=2 -I../third-party/dht -I../third-party/ -pthread  -g -O2 -fPIE -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -g -O0 -std=gnu99 -ggdb3 -Wall -W -Wpointer-arith -Wformat-security -Wcast-align -Wundef -Wcast-align -Wstrict-prototypes -Wmissing-declarations -Wmissing-format-attribute -Wredundant-decls -Wnested-externs -Wunused-parameter -Wwrite-strings -Winline -Wfloat-equal -Wextra -Wdeclaration-after-statement -Winit-self -Wvariadic-macros -c upnp.c
upnp.c: In function ‘tr_upnpDiscover’:
upnp.c:94:15: error: ‘UPNPDISCOVER_SUCCESS’ undeclared (first use in this function)
upnp.c:94:15: note: each undeclared identifier is reported only once for each function it appears in
make[3]: *** [upnp.o] Error 1
make[3]: Leaving directory `/home/taem/src/transmission/transmission/libtransmission'
make[2]: *** [all-recursive] Error 1
make[2]: Leaving directory `/home/taem/src/transmission/transmission'
dh_auto_build: make -j1 returned exit code 2
make[1]: *** [override_dh_auto_build] Error 2
make[1]: Leaving directory `/home/taem/src/transmission/transmission'
make: *** [build] Error 2

$ dpkg -l | grep miniupnp
ii  libminiupnpc-dev                      1.5-2                              i386         UPnP IGD client lightweight library development files
ii  libminiupnpc5                         1.5-2                              i386         UPnP IGD client lightweight library

[rb07]

Oops! upnp.c patch take two (added).

But looking at miniupnc's code, in particular their command line tool: upnpc.c, it doesn't use errno at all, just test if ret is NULL, and print their error code (which is one of 3: unknown-, socket-, memory- error). I don't know if logging errno will be usefull.

For debugging purposes the return from UPNP_GetValidIGD() is more interesting, its used to show:

case 1:
	printf("Found valid IGD : %s\n", urls.controlURL);
	break;
case 2:
	printf("Found a (not connected?) IGD : %s\n", urls.controlURL);
	printf("Trying to continue anyway\n");
	break;
case 3:
	printf("UPnP device found. Is it an IGD ? : %s\n", urls.controlURL);
	printf("Trying to continue anyway\n");
	break;
default:
	printf("Found device (igd ?) : %s\n", urls.controlURL);
	printf("Trying to continue anyway\n");

[taem]

rb07, the patch fixed FTBFS. Thanks.

[jordan]

rb07, thanks for the patch. taem, thanks for the testing :)

r14026: #5293 'Wrong error codes used in upnp.c': revised patch by rb07 to accommodate older versions of miniupnpc

r14027: (2.7x) backport r14026 for #5293