Incorrect value returned from `cx_bn_cnt_bits`

[aido]

The following function seems to return the bit position of the most significant bit in a big number:

https://github.com/LedgerHQ/speculos/blob/56ed99657f351ba63dd9448281e77ed9f0eea835/src/bolos/cx_mpi.c#L599-L637

Whereas the documentation suggests that it should instead return the number of bits set to 1 in a big number:

https://github.com/LedgerHQ/ledger-secure-sdk/blob/b82be6fd5a082132ee08bf0d105d1bb7bb4d0b41/include/ox_bn.h#L490-L502

Is the function incorrect or the documentation?

[aido]

Apologies, This is an accidental duplicate issue. I forgot that I opened a similar issue a few months ago. #460

However. following on from my comment above, if the cx_mpi_cnt_bits() function is supposed to return the bit position of the most significant bit in a big number could it be simplified as follows?

uint32_t cx_mpi_cnt_bits(const cx_mpi_t *x)
{
  uint32_t nbits = BN_num_bits(x);

  return nbits;
}

but if, as the documentation suggests, it is supposed to return the number of bits set to 1 would this work instead?

uint32_t cx_mpi_cnt_bits(const cx_mpi_t *x)
{
  uint32_t nbits = BN_num_bits(x);
  uint32_t count = 0;

  for (int i = 0; i < nbits; ++i) {
    count += BN_is_bit_set(x, i);
  }

  return count;
}

[tdejoigny-ledger]

cc: @srasoamiaramanana-ledger

[srasoamiaramanana-ledger]

Yes cx_mpi_cnt_bits returns the number of (significant) bits of the big number which is indeed the position of the MSB (if you start counting from 1). I will reword the documentation, thank you. And yes, the function can direclty call BN_num_bits.