[Pkg-freeciv-devel] Bug#309046: possible fix

[spikethehobbitmage at excite.com]

I have been able to trace the problem



if pf_get_path is called for a route that starts with a dangerous path, then the pf_node &pf_map->lattice[pf_map->tile->index] is never initialized.

create_danger_segment then iterates off the beginning of the list :(

As this node is the starting point, there is no previous node to get a valid dir_to_here value from.  This is not tested for, and dereferencing a NULL pointer then causes segfault.



To fix this, two changes are needed in create_danger_segment

1. do not step past the beginning of the list

  since the first node is already tested for, this can be done simply by adding an else clause to that test

  it could also be done by unrolling the last step of the loop, since the last step logic is now so much different than the others. this would also eliminate a test each loop

2. the final assert to make sure we reached a safe node is invalid if we started on an unsafe node, so it should be removed

I also suggest adding an assert to prevent segfault if mapstep returns NULL



I have not tested this extensively, but it seems to work without crashing, and it does produce valid paths.



so either



    if (i == length - 1) {

      /* The last dangerous node contains "waiting" info */

      d_node1->waited = pf_map->d_lattice[ptile->index].waited;

    } else {



      /* Step further down the tree */

      ptile = mapstep(ptile, DIR_REVERSE(node->dir_to_here));

      assert(ptile);

      node = &pf_map->lattice[ptile->index];

    }

  }



  /* Make sure we reached a safe node */

  /* assert(!pf_map->d_lattice[ptile->index].is_dangerous); */



**** or ****



  length--;

  /* Now fill the positions */

  for (i = 0; i < length; i++) {

    /* Record the direction */

    d_node1->danger_segment[i].dir = node->dir_to_here;

    d_node1->danger_segment[i].cost = node->cost;

    d_node1->danger_segment[i].extra_cost = node->extra_cost;



    /* Step further down the tree */

    ptile = mapstep(ptile, DIR_REVERSE(node->dir_to_here));

    assert(ptile);

    node = &pf_map->lattice[ptile->index];

    }

  }



  /* Record the direction */

  d_node1->danger_segment[length].dir = node->dir_to_here;

  d_node1->danger_segment[length].cost = node->cost;

  d_node1->danger_segment[length].extra_cost = node->extra_cost;



  /* The last dangerous node contains "waiting" info */

  d_node1->waited = pf_map->d_lattice[ptile->index].waited;



  /* Make sure we reached a safe node */

  /* assert(!pf_map->d_lattice[ptile->index].is_dangerous); */



should fix the problem.



_______________________________________________
Join Excite! - http://www.excite.com
The most personalized portal on the Web!