Re: ws2_32: WS_get_local_ips: Fix sorting IP addresses by metric if two addresses have the same metric

[Ken Thomases]

Hi,

On Oct 24, 2014, at 3:27 PM, Joachim Priesner  wrote:
> The previous implementation required the metrics to be mutually inequal
> (because of the "this_metric > last_metric" check). If the metrics were
> not mutually inequal, it would write only one address per metric value to the list
> and fill up the rest of the list with the magic loopback IP address 127.12.34.56.
>
> I take it that this is not the expected behavior, please correct me if I'm wrong!

I was recently looking at this code and noticed the same problem.  Thanks for working on this.

I don't understand why the original code ever would have wanted to return the magic loopback address, other than to cover up the brokenness of the sort.  So, why do you still have that logic in there?

Each pass of the outer (i) loop can "consume" at most one entry in the list.  So, the inner (j) loop should always find a remaining lowest-metric entry.  lowest_metric_idx should never be -1 at the end of that loop and the magic address should never be used.

More simply, this is just an attempt to sort the routes.  None should be added and none should be removed (i.e. replaced with magic).  Since it's a sort, I recommend that this loop simply be replaced with an invocation of qsort() with an appropriate comparison function.  That's what's done in iphlpapi for similar situations.  (This can be conditioned on numroutes >= 2 to avoid the overhead when it's not necessary.)

After the routes are sorted, the addresses can be copied over to the h_addr_list with a much simpler loop.

-Ken