[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wine-devel
Subject:    Re: ws2_32: WS_get_local_ips: Fix sorting IP addresses by metric if two addresses have the same metr
From:       Ken Thomases <ken () codeweavers ! com>
Date:       2014-10-28 1:59:38
Message-ID: E49B5EEC-8CC9-4BFB-8D5E-9052A74745A3 () codeweavers ! com
[Download RAW message or body]

Hi,

On Oct 24, 2014, at 3:27 PM, Joachim Priesner <joachim.priesner@web.de> 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


[prev in list] [next in list] [prev in thread] [next in thread]