Bignum: Rename modulus input argument from m to N in bignum_mod

[minosgalanakis]

Suggested enhancement

Replace all references to modulus from m -> N in bignum_mod.c, bignum.h and test_suite_bignum_mod.function

Justification

This is a requirement that was discussed in the review of #6743 and aggreed upon as part of the common conventions for the new bignum interface #6684 .

[iammihirsig]

Hey , @minosgalanakis I am getting started with my open source journey and like to work on this issue.
I am unclear about :

Replace all references to modulus from m -> N

Can you please explain it further ?
Do I have to replace all m -> to N ?

Example :

mbedtls/library/bignum_mod.c

Line 42 in b402e4b

if( p_limbs != m->limbs || !mbedtls_mpi_core_lt_ct( p, m->p, m->limbs ) )

Like this :
if( p_limbs != N->limbs || !mbedtls_mpi_core_lt_ct( p, N->p, N->limbs ) )

[minosgalanakis]

Hi @Mihir-Raj-Singh, and thank you for the interest in the project.

Yes that is precicely the ask. Two different ways to represent the modulus have been introduced to the new interface:
mbedtls_mpi_mod_modulus *m and mbedtls_mpi_mod_modulus *N.

The issue is to replace it so that only mbedtls_mpi_mod_modulus *N is used for the shake of consistency.

• Code: bignum_mod.c, bignum_mod_raw.c, bignum_core.c
• Documentation: bignum_mod.h, bignum_mod_raw.h, bignum_core.h

n order to make it simpler and quicker to review, I would recommend launching a series of small PR's in succession, each responsible for changing m->N in single function, and it's documentation. They can all be linked to this task. Also since that approach will require some rebasing, it may be better to not have more than two pending review.

[iammihirsig]

Thanks @minosgalanakis ,

I would like to work on this. Please assign me this issue.

If anything is needed further, I will let you know.

[iammihirsig]

Hello @minosgalanakis , I tried renaming modulus input argument in a single function in both files bignum_mod.c and bignum_mod.h

You can refer it here : a433560

• Is this the correct way to do that ?
• When do I need to create pull request ?
• Do i have to edit all the functions separately as I did ?

I'm a little nervous and don't want things to go wrong.
It would be great If you explain things further or suggest me how to move further.

[minosgalanakis]

Yes, something like that should suffice.

But you need to make sure that the embedded tests are still passing:

• Run ctest as described in the documentation
• Check that the file style is correct by running check_files.py
• Also in accordance to our DCO we cannot accept contributions that are not signed off. In order to achive that all commits should have a Signed-off-by: Name <email> field in the end. You can add that be rewording the commit. For future commits by using git commit -s syntax will automacially append it to the text message.
• Also it would be good to follow the commit message structure of existing commits, with one short line for the description, a newline, and a small more verbose text explaining what the commit does. If the modification is minor and self explanatory then the commit body can be ommited. git log --grep='bignum' or git log --grep='Bignum' can give you pointers.

After your personal branch you have made the changes is in good shape, and has passed the tests, you can make the PR request. Again using past PR's as reference can be usefull on how to structure the PR's description.

One the first PR request is ready, and has passed CI you can repeat the process for all functions that need to be renamed.

[iammihirsig]

• I ran into a problem with cmake , so I raised an issue have a look . Error: 'MBEDTLS_ERR_SSL_FATAL_ALERT_MESSAGE' undeclared, while building with cmake. #6879 (comment)

• Although make works fine for me right now and tests are also passing. But another problem is that :
  I'm trying to run check_files.py , But gets an error . Can you please explain what it is ? @minosgalanakis

I tried searching but unable to understand myself.

[iammihirsig]

Hey @minosgalanakis , I'm getting this error while running check_files.py
Have a look :

python3 check_files.py

Traceback (most recent call last):
  File "/home/scorpion/Desktop/mbedtls/tests/scripts/check_files.py", line 409, in <module>
    run_main()
  File "/home/scorpion/Desktop/mbedtls/tests/scripts/check_files.py", line 402, in run_main
    integrity_check = IntegrityChecker(check_args.log_file)
  File "/home/scorpion/Desktop/mbedtls/tests/scripts/check_files.py", line 345, in __init__
    build_tree.check_repo_path()
  File "/home/scorpion/Desktop/mbedtls/tests/scripts/../../scripts/mbedtls_dev/build_tree.py", line 34, in check_repo_path
    raise Exception("This script must be run from Mbed TLS root")
Exception: This script must be run from Mbed TLS root

How can I fix this ?

[tom-cosgrove-arm]

How can I fix this ?

The last line in the output you pasted is Exception: This script must be run from Mbed TLS root.

That means the top of the repository, so:

macos $ ls
3rdparty/    ChangeLog.d/    CONTRIBUTING.md	    doxygen/  Makefile	 SECURITY.md
BRANCHES.md  cmake/	     DartConfiguration.tcl  include/  programs/  SUPPORT.md
BUGS.md      CMakeLists.txt  dco.txt		    library/  README.md  tests/
ChangeLog    configs/	     docs/		    LICENSE   scripts/	 visualc/
macos $ python3 tests/scripts/check_files.py
macos $ echo $?
0

[iammihirsig]

Thanks for the information. It works fine now.

May I ask you what echo $? and 0 means ? What's the purpose of writing it ? and why it outputs 0 ? If I get some error while checking the files still the output will be zero ?

[tom-cosgrove-arm]

May I ask you what echo $? and 0 means ?

echo $? means "tell me the exit status of the last command I just ran". And 0 is the usual exit status for "success".

I will normally include this on examples I post where there is no output (like above) to make it clear that the command that I ran succeeded.

If I get some error while checking the files still the output will be zero ?

No, it shouldn't be.

[iammihirsig]

Hello ,
I renamed modulus input argument from m to N in give files as suggested :

• bignum_mod.c
• bignum_mod.h
• bignum_mod_raw.c
• bignum_mod_raw.h
• test_suite_bignum_mod.function

Review the PR : #6903

• Run ctest as described in the documentation

• Check that the file style is correct by running check_files.py

• Also in accordance to our DCO we cannot accept contributions that are not signed off. In order to achive that all commits should have a Signed-off-by: Name <email> field in the end. You can add that be rewording the commit. For future commits by using git commit -s syntax will automacially append it to the text message.

• Also it would be good to follow the commit message structure of existing commits, with one short line for the description, a newline, and a small more verbose text explaining what the commit does. If the modification is minor and self explanatory then the commit body can be ommited. git log --grep='bignum' or git log --grep='Bignum' can give you pointers.

Checklist :

• Run Ctest ( All test passed successfully)
• Run check_files.py (File system are correct)
• Signed the commit