Proposed fix for possible qsort callback random return value

[nawrockie]

Issue #11 in Infernal (EddyRivasLab/infernal#11) highlights the possibility of random return values from the hit_sorter* functions used by qsort when all compared keys are equal. This branch fixes analogous situations in hmmer's develop (h3) branch. Please take a look at the comparison.

There are two qsort helper functions that I did not edit. These are: cachedb.c:sort_seq() and hmmdmstr.c:hit_sorter(). Those use a different strategy that I think is 'ok' with respect to this issue of possible random return values, but see what you think. For example, cachedb:sort_seq() is:

static int
sort_seq(const void *p1, const void *p2)
{
  int cmp;

  cmp  = (((HMMER_SEQ *)p1)->idx < ((HMMER_SEQ *)p2)->idx);
  cmp -= (((HMMER_SEQ *)p1)->idx > ((HMMER_SEQ *)p2)->idx);

  return cmp;
}

Same strategy is used by hmmdmstr.c:hit_sorter().

I think that covers all qsort() helper functions.

I did not update BUGTRAX because I could not find it (no Bugs/ subdir?). Here is what I would have put as the entry if I had found it, based on Infernal's BUGTRAX i48 entry that I just wrote:

ID              ?
TITLE           qsort callback random return value
STATUS          CLOSED
XREF            /panfs/pan1/infernal/notebook/18_0521_inf_qsort_helpers_bug/infernal
                (NCBI)
REPORTED_BY     Yuri Gribov (github username: yugr)
                https://github.com/EddyRivasLab/infernal/issues/11
CLOSED_DATE     EPN, Wed May 23 09:07:57 2018
DESCRIPTION

Some of the functions used by qsort() to sort (fm_ssv.c:FM_hit_sorter(),
hmmerfm-exactmatch.c:hit_sorter(), p7_tophits.c:hit_sorter_by_sortkey(),
p7_tophits.c:hit_sorter_by_seqidx_aliposition(), and
p7_tophits.c:hit_sorter_by_modelname_aliposition()), were not
returning value '0' for input in which all compared fields for the two
objects being compared were equal. Instead, 1 or -1 was being returned,
in ways that Yuri explained 'may return random result when input
structs have all compared fields equal. This in turn may causes
inconsistent order or even crashes in some qsort implementations.' The
fix, as Yuri suggested was simply to update the final comparison so
that '0' is returned if all fields are equal, that is if both '<' and
'>' comparisons of final field checked (and all other fields checked)
returns false.

[cryptogenomicon]

good, thanks!