herumi / mcl

a portable and fast pairing-based cryptography library
BSD 3-Clause "New" or "Revised" License
450 stars 152 forks source link

Same conditional expression #69

Closed janus closed 4 years ago

janus commented 4 years ago
#if defined(MCL_USE_LLVM) || defined(MCL_USE_XBYAK)
    if (mode == FP_AUTO || mode == FP_LLVM || mode == FP_XBYAK) {
        const char *pStr = "0xfffffffffffffffffffffffffffffffeffffffffffffffff";
        bool b;
        mpz_class p192;
        gmp::setStr(&b, p192, pStr);
        if (b && mp == p192) {
            primeMode = PM_NIST_P192;
            isMont = false;
            isFastMod = true;
        }
    }
    if (mode == FP_AUTO || mode == FP_LLVM || mode == FP_XBYAK) {
        const char *pStr = "0x1ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff";
        bool b;
        mpz_class p521;
        gmp::setStr(&b, p521, pStr);
        if (b && mp == p521) {
            primeMode = PM_NIST_P521;
            isMont = false;
            isFastMod = true;
        }
    }
#endif

The IF statements have same conditional expression, however one is meant to set primeMode = PM_NIST_P192; and the other primeMode = PM_NIST_P521;, is the correct? If yes, what do you intend to achieve by this coding structure?

herumi commented 4 years ago

The original conditions were different, but they eventually became the same except for the value pStr. So it may be better to unify them as you say. Thank you for pointing it.

janus commented 4 years ago

I would like to read your code that covers only bls12_381 , which files should I go through?

herumi commented 4 years ago

mcl supports both BN and BLS12 and they are not separated. At first, please could you see https://github.com/herumi/mcl/blob/master/include/mcl/bn.hpp#L1672-L1727 and search inner functions as you need?

herumi commented 4 years ago

I've fixed it. Thank you. https://github.com/herumi/mcl/commit/b399592446caae925cb775456ac4952da7de3872