syncthing/syncthing — pull request #775 by cqcallaw (2014-10-02 03:04:40+00:00)
Title: UPnP cleanup and fixes for #432

Code to fix the issues outlined in #432. The surgery was fairly invasive, so I thought it best to leave the commits un-squashed so as to leave the transformations of the code as granular and obvious as possible. I'm happy to squash away if that's desired.

Notes:

* The generalization from 1 thing to n things made returning an error awkward. For example, if discovery fails on one device, that shouldn't affect the discovery of any other. Mapping errors back to devices is awkward, requiring either parallel arrays or an additional field in the IGD struct. Since no error-handling logic was done by the upstream consumers, I elected to simply log errors to the debug log facilities instead of returning them to the consumer.
* I tested the changes with one of my local routers, Wireshark, and a miniupnpd instance in a VM. I didn't see a good way to automate testing of the discovery or port mapping mechanisms; the only readily testable portion of the API seems to be the search and port mapping request strings. These operations seem don't seem susceptible to regressions, but I can write tests if desired.