Modify parameter order of internal functions to match API parameter order #407

The below commits modify the parameter orders for the following internal functions based on the API parameter ordering guide outlined in secp256k1.h. This helps make the internal code's structure match the external API's structure. These changes shouldn't harm any downstream applications that rely on this library as it only affects internal function parameter ordering, not external ones.

Each of the blow commits corresponds with modifying a single function's parameter order and updating all locations where it is used.

secp256k1_fe_inv_all_var

secp256k1_fe_inv_all_var(secp256k1_fe *r, const secp256k1_fe *a, size_t len)

Declared in: src/field.h Defined in: src/field_impl.h Used in: src/group_impl.h, src/tests.c

• size_t len now comes after const secp256k1_fe *a as it defines its length

secp256k1_ge_globalz_set_table_gej

secp256k1_ge_globalz_set_table_gej(secp256k1_ge *r, secp256k1_fe *globalz, const secp256k1_gej *a, const secp256k1_fe *zr, size_t len)

Declared in: src/group.h Defined in: src/group_impl.h Used in: src/ecmult_impl.h

• size_t len now comes after const secp256k1_gej *a, const secp256k1_fe *zr as it describes both of their lengths
• The API parameter ordering guide (rule 2) doesn't explicitly say what to do in this situation.
• Other options included:
  • const secp256k1_gej *a, size_t len, const secp256k1_fe *zr implying that len only describes *a and that *zr may be a single element.
  • const secp256k1_gej *a, size_t a_len, const secp256k1_fe *zr, size_t zr_len implying that *a and *zr are arrays, but possibly that a_len != zr_len. This also means the caller must pass the same length value twice.

secp256k1_ge_set_all_gej_var

secp256k1_ge_set_all_gej_var(secp256k1_ge *r, const secp256k1_gej *a, size_t len, const secp256k1_callback *cb)

Declared in: src/group.h Defined in: src/group_impl.h Used in: src/ecmult_gen_impl.h, src/tests.c

• size_t len now comes after const secp256k1_gej *a as it describes its length "even if this violates rule 1"

secp256k1_ge_set_table_gej_var

secp256k1_ge_set_table_gej_var(secp256k1_ge *r, const secp256k1_gej *a, const secp256k1_fe *zr, size_t len)

Declared in: src/group.h Defined in: src/group_impl.h Used in: src/ecmult_impl.h, src/tests.c

• size_t len now comes last, same reasoning as secp256k1_ge_globalz_set_table_gej

secp256k1_ecmult_odd_multiples_table

secp256k1_ecmult_odd_multiples_table(secp256k1_gej *prej, secp256k1_fe *zr, const secp256k1_gej *a, int n)

Declared/Declared in: src/ecmult_impl.h Used in: src/ecmult_impl.h

• int n now comes last because it is a count parameter, not a length parameter
  • Length parameters are subject to rule 2, count parameters are subject to rule 4

secp256k1_ecmult_odd_multiples_table_storage_var

secp256k1_ecmult_odd_multiples_table_storage_var(secp256k1_ge_storage *pre, const secp256k1_gej *a, const secp256k1_callback *cb, int n)

Declared/Declared in: src/ecmult_impl.h Used in: src/ecmult_impl.h

• int n now comes last because it is a count parameter, not a length parameter
• It comes after const secp256k1_callback *cb because rule 4 states "Arguments that are not data pointers go last, from more complex to less complex: function pointers, ..., counts, ...".

I've compiled and run the included test utilities which reported that no errors were found. However, I'd appreciate it if another developer reviewed these changes just in case I missed anything.

Cheers!

I think the ecmult tables ones, first two commits, ought to be left alone. These sizes don't feel like array lengths to be, rather they're their own parameter determining the size of the table that these functions are operating on. (Even though this literally corresponds to the size of various arrays.)

Can you rebase -i on master to remove the commits?

Also there is a third commit which also has "table" in the name, I'd like that one removed as well.

@apoelstra - Are you referring to secp256k1_ge_globalz_set_table_gej or secp256k1_ge_set_table_gej_var?

Oh, secp256k1_ge_globalz_set_table_gej. The other one is OK.

utACK

utACK