trade_route_is_valid(id) looks broken.

[ambre-m]

Skipping the long story, while doing some code review, I wound a potential error.

We have the following definitions, in src/empire/trade_route.c:

int trade_route_is_valid(int route_id)
{
    route_resource *route = array_item(routes, route_id);
    return route != 0;
}

and in src/core/array.h:

#define array_item(a, position) \
( \
    &(a).items[(position) >> (a).bit_offset][(position) & (a).block_offset] \
)

That means the actual code is:

int trade_route_is_valid(int route_id)
{
    return (&routes.items[route_id >> routes.bit_offset][route_id & routes.block_offset]) != 0;
}

Actually, &a[i] is never 0, except if a == 0 and i == 0. Which means the function returns false only if routes has not been initialised (AND the index is 0). That could and should not happen (since that would be a conception error).

I think the function should be something like:

int trade_route_is_valid(int route_id)
{
    return route_id >= 0 && route_id < routes.size;
}

[crudelios]

Good catch, it is indeed broken and your solution is correct!