I'd like to hear some other opinions on this before merging it. I can somewhat buy that not configured interface is not exceptional but, then again, we have (and use elsewhere) NotFoundException and on that particular interface IPv4 or v6 stack was not found.

Comments solicited ...

Unfortunately, Poco doesn't document what exceptions it throws in most cases, and people have to resort to reading the source code to figure out what they need to catch and when.

Clearly stating that firstAddress() returns a isWildcard() address is a lot easier to document and more intuitive.

Can I please get closure on this? I have other fixes predicated on this fix that have been waiting since mid-August...

Let's compare a simple code fragment which iterates over all the interfaces in a system and prints the interface name and whether IPv4 is configured on that interface or not.

const NetworkInterface::Map map = NetworkInterface::map();
for ( NetworkInterface::Map::const_iterator it = map.begin();
        it != map.end(); ++it ) {
    const NetworkInterface& intf = it->second;
    bool configured;

    try
    {
        (void) intf.firstAddress(IPAddress::IPv4);  // see if it throws or not
        configured = true;
    }
    catch (Exception::NotFoundException& nfe)
    {
        configured = false;
    }
    std::cout << intf.name() << " " << (configured ? "yes" : "no") << std::endl;
}

versus:

const NetworkInterface::Map map = NetworkInterface::map();
for ( NetworkInterface::Map::const_iterator it = map.begin();
        it != map.end(); ++it ) {
    const NetworkInterface& intf = it->second;
    std::cout << intf.name() << " " << (! intf.firstAddress(IPAddress::IPv4).isWildcard() ? "yes" : "no") << std::endl;
}

Again, having firstAddress() return a wildcard is (a) simpler programmatically, (b) faster since it doesn't require having to unwind the stack to handle a throw/catch pair, (c) more intuitive since it having unconfigured interfaces is hardly an "exceptional" state, and (d) doesn't require knowing which undocumented exception type you need to catch.

I can somewhat understand (a), but where you end up when that logic is universally applied is code peppered with checks for return codes (that are often ignored). (b) is not correct because when the exception does not occur, a good compiler will generate code as fast as that without exception handling (as opposed to return code checking where code is equally slow every time). (c) depends what one considers exceptional; since we have NotFoundException, something sought for and not being found is considered exceptional in Poco. (d) if it is not documented, we can definitely fix that.

FWIW, your example can be somewhat simplified:

const NetworkInterface::Map map = NetworkInterface::map();
for ( NetworkInterface::Map::const_iterator it = map.begin();
        it != map.end(); ++it ) {
    const NetworkInterface& intf = it->second;
    try
    {
        (void) intf.firstAddress(IPAddress::IPv4);  // see if it throws or not
        std::cout << intf.name() << " " << "yes" << std::endl;
    }
    catch (Exception::NotFoundException&)
    {
        std::cout << intf.name() << " " << "no" << std::endl;
    }

}

As for the second commit, perhaps I'm missing something important, but I'm wondering what is wrong with having boolean flags that allow for any interface state/configuration filtering (and return all interfaces when both of them are set to false)? What is the benefit of first generating a map of all interfaces and then copying a subset of it into another map?

Ok, how about having throwing and non-throwing versions of this function then?

BTW: as for functions not checking the return address.... Why would anyone call the function if not to capture the return value? They might not validate it, but they definitely wouldn't discard it.

What would the throw and non-throw functions exactly look like?

As for the BTW remark, people often call functions only to achieve desired side-effect and disregard or forget to check return values. Exceptions prevent disregard of errors and RAII nicely takes care of proper cleanup in complex error checking scenarios.

The problem with this request is that two concepts are mixed - (1) obtaining the first IP address on a network interface and (2) detecting if IPv4/6 is supported on the interface. If you try to obtain the first IP address of a family that is not configured on the interface, we throw. To detect whether an interface supports an IP family, there are NetworkInterface::supportsIPv4/6() functions.

I'm struggling to see how

std::cout << intf.name() << " " << (! intf.firstAddress(IPAddress::IPv4).isWildcard() ? "yes" : "no") << std::endl;

is better than

std::cout << intf.name() << " " << (! intf.supportsIPv4()) ? "yes" : "no") << std::endl;

If we had an overwhelming amount of requests for this change (and I encourage anyone following this discussion to weigh in), I'd give it more weight and consider it . But there's only one so far.

In my case code usually looks like:

const NetworkInterface::Map map = NetworkInterface::map();
for ( NetworkInterface::Map::const_iterator it = map.begin();
        it != map.end(); ++it ) {
    const NetworkInterface& intf = it->second;
    IPAddress addr = intf.firstAddress(IPAddress::IPv4);
    if (addr.isWildcard())    // not configured... skip it...
        continue;
    // do something else with addr here...
}

We could change firstAddress(IPAddress::Family family) to firstAddress(IPAddress::Family family, bool throwIfNotFound = true) ... My point is that the amount of work done by firstAddress() and supportsIPv4() is largely duplicated, and as one is currently forced to call the latter before calling the former to avoid an exception, why not just simplify that?

If I do:

    if (! intf.supportsIPv4())
        continue;
    IPAddress addr = intf.firstAddress(IPAddress::IPv4);

If one looks at supportsIPv4() and firstAddress() there's a lot of replication. Indeed. one could implement the latter via the former:

bool NetworkInterface::supportsIPv4() const
{
    try
    {
        (void) firstAddress(IPAddress::IPv4);
        return true;
    }
    catch (Exception::NotFoundException& exc)
    {
        return false;
    }
}

or conversely:

bool NetworkInterface::supportsIPv4() const
{
    return ! firstAddress(IPAddress::IPv4).isWildcard();
}

Your "fix" completely ignored my concern.

I don't think we need the overhead of "try" in every case.

I've worked with Windows web farms that had literally tens of thousands of interfaces per machine. Many of which weren't always configured (yes, there was one interface per web server instance... not an ideal architecture, but they weren't using Apache).

Many a denial-of-service attack is based on getting the party under attack to commit huge computational resources to a problem. And often times that "huge computational commitment" is a small (and even seemingly 'negligible') amount of computation that has been scaled up by several orders of magnitude.

Imagine my sample code fragment above running and throwing tens of thousands of exceptions before it completed.

You could just as easily have implemented:

const void NetworkInterfaceImpl::firstAddress(IPAddress& addr, IPAddress::Family family) const
{
    AddressList::const_iterator it = _addressList.begin();
    AddressList::const_iterator end = _addressList.end();
    for (;it != end; ++it)
    {
        const IPAddress& addr2 = it->get<NetworkInterface::IP_ADDRESS>();
        if (addr2.family() == family) {
                        addr = addr2;
                        return;
                }
    }

    addr = IPAddress(family);
}

const IPAddress& NetworkInterfaceImpl::firstAddress(IPAddress::Family family) const
{
        IPAddress addr;
        firstAddress(addr, family);
        if (addr.isWildcard())
                throw NotFoundException(format("%s family address not found.", (family == IPAddress::IPv4) ? std::string("IPv4") : std::string("IPv6")));
        return addr;
}

instead. Why incur the overhead of a try/catch when you don't have to?

My solution here presents the same API as your fix but without the unnecessary overhead in void firstAddress(IPAddress, IPAddress::Family);

Ok, with 9ab50e5 this should preserve the semantics that Alex wanted and yet un-encumber the simplified version of firstAddress() without the overhead of try/catch code generation.

Both parties get what they want!

un-encumber the simplified version of firstAddress() without the overhead of try/catch code generation.

Unfortunately, the analysis is not accurate and here's why
(quoting http://www.open-std.org/jtc1/sc22/wg21/docs/TR18015.pdf ):

1. proposed modification checks for IP wildcard address every time; with try/catch, there are well-known modern compiler techniques with no overhead when no exception is thrown.

5.4.1 Exception Handling Implementations and Techniques
5.4.1.2. The "Table" Approach

try-block This method incurs no run-time cost. All bookkeeping is precomputed as a mapping between program
counter and code to be executed in the event of an exception. Tables increase program image size but may be
moved away from working set to improve locality of reference. Tables can be placed in
ROM or, on hosted systems with virtual memory, can remain swapped out until an exception is actually thrown.

2. IPAddress allocates memory on the heap, so it

a) will cause performance penalty every time

5.6.8. Additional Suggestions
"Dynamic memory allocation and deallocation can be a bottleneck."

(b) may throw

So, to make it bullet-proof, we should wrap it in try/catch block which takes us back to square one.

Quoting a little further down on that same page:

There are two primary disadvantages of the “code” model:
• The associated stack and run-time costs can be high for try-block entry.
• Even when no exceptions are thrown, the bookkeeping of the exception handling stack must be performed for automatic, temporary and partially constructed objects.

That is, code unrelated to error handling is slowed down by the mere possibility of exceptions being used. This is similar to error-handling strategies that consistently check error state or return values.

This is similar to error-handling strategies that consistently check error state or return values.

Such as wildcard address? Precisely my point.

(b) may throw

May throw under what other circumstances?

2. IPAddress allocates memory on the heap, so it

As for the allocation of memory, etc. I had submitted a fix to make wildcard's for IPv4 and IPv6 be static const, and that fix was never accepted...

I could change the code a bit more to use references, etc. and that would eliminate any allocations/copies.

As for the allocation of memory, etc. I had submitted a fix to make wildcard's for IPv4 and IPv6 be static const, and that fix was never accepted...

I may add it, didn't get to it yet. but it would not help much in this case because IPAddress is a pImpl; we're beating the dead horse; IPAddress does need redesign but it won't happen in this iteration.

There are two primary disadvantages of the “code” model

There are two models, table and code, each with it's set of ups and downs. Table model uses more (static) memory but incurs no runtime performance and takes no additional stack space.

While one could come up with an isolated scenario where "IPAddress creation and return value check" design performs better than exception handling, the fact is that it is not universally more efficient.

Of the 'find' that I've used on Solaris, OpenBSD, MacOS X, Linux, and HP-UX I've not encountered a one that this wouldn't have worked with.