Use of mp_ilogb in mp_radix_size

[czurnieden]

PR slicing as asked for in #343

[minad]

Btw, why are we having ilog and not ilog_u32 like for expt. Furthermore why ilogb and not ilog, I mean it is integer logarithm and not logbrithm. Sorry for the off topic comment.

[minad]

Hmm, I am unsure about this one. In general I like it but I am a bit hesitant of adding it to 1.2, since we are delaying the radix overhaul to 2.0

[minad]

Ping @sjaeckel

[czurnieden]

why ilogb and not ilog, I mean it is integer logarithm and not logbrithm.

It is integer logarithm for different integer bases.
If you think that it might clash with the meaning of C's ilogb and C++'s std::ilogb: the name is easily changeable.

why are we having ilog and not ilog_u32 like for expt.

Seems to sound better, yes, so if you want that: I can do that, no problem.

Now that you mention it: didn't we have a discussion about that or was it only about mp_expt? But as log is the inverse of exp and vice versa they should have a common naming scheme, so a 👍 from me for the renaming.

[minad]

Ok, I didn't know ilogb was also in the c11 standard. I've read in the docs that the b refers to base, but this only makes sense if there are other functions with a fixed base like log and lg. We also don't have exptb, right? But given the C name I am ok with your choice.

[sjaeckel]

I'm also pro mp_ilogb_u32() and mp_ilogb() is only on develop in tommath.h, not released yet, so it's only a rename and we're fine.

[minad]

Hmm, I've lost track on what is realeased and what not. If we can still change the names I would prefer mp_log and mp_exp (maybe with _u32 suffix if desired). No b suffix for both since both have a base specified and we don't provide specialised functions with fixed base. Maybe also call it ilog, but I think it is already obvious that we are talking about an integer logarithm Here. I think c had to use the weird name due to all the many c log functions already existing.

[sjaeckel]

Hmm, I've lost track on what is realeased and what not.

$ git diff master -- tommath.h | grep -e exp -e log
 /* define this to use lower memory usage routines (exptmods mostly) */
 /* export binary data */
-int mp_export(void *rop, size_t *countp, int order, size_t size, int endian, size_t nails, const mp_int *op);
+MP_DEPRECATED(mp_pack) mp_err mp_export(void *rop, size_t *countp, int order, size_t size,
@@ -252,37 +394,43 @@ int mp_export(void *rop, size_t *countp, int order, size_t size, int endian, siz
-int mp_2expt(mp_int *a, int b);
+mp_err mp_2expt(mp_int *a, int b) MP_WUR;
-int mp_expt_d(const mp_int *a, mp_digit b, mp_int *c);
-int mp_expt_d_ex(const mp_int *a, mp_digit b, mp_int *c, int fast);
-int mp_exptmod(const mp_int *G, const mp_int *X, const mp_int *P, mp_int *Y);
+mp_err mp_exptmod(const mp_int *G, const mp_int *X, const mp_int *P, mp_int *Y) MP_WUR;
+/* Integer logarithm to integer base */
+mp_err mp_ilogb(const mp_int *a, uint32_t base, mp_int *c) MP_WUR;
+mp_err mp_expt_u32(const mp_int *a, uint32_t b, mp_int *c) MP_WUR;
+MP_DEPRECATED(mp_expt_u32) mp_err mp_expt_d(const mp_int *a, mp_digit b, mp_int *c) MP_WUR;
+MP_DEPRECATED(mp_expt_u32) mp_err mp_expt_d_ex(const mp_int *a, mp_digit b, mp_int *c, int fast) MP_WUR;

If we can still change the names I would prefer mp_log and mp_exp (maybe with _u32 suffix if desired). No b suffix for both since both have a base specified and we don't provide specialised functions with fixed base. Maybe also call it ilog, but I think it is already obvious that we are talking about an integer logarithm Here. I think c had to use the weird name due to all the many c log functions already existing.

fine by me to change it to mp_log_u32() and mp_exp_u32().

Edit considering the existence of mp_exptmod() maybe we should stay with mp_expt_u32()?

[czurnieden]

So it's mp_log_u32() and mp_expt_u32() (with the "t") ?
Yeah, looks good to me.