Void function with pointers in parameters

[AlexInsa]

This is just a comment about some functions which take pointer in parameter and return nothing.

void function (type * parameter_1, ...){
  /* Code */
}

First, these functions do not check if the parameter is NULL or not. Even if these functions are not called with NULL parameter by the internal function of the API, these functions are dangerous because a NULL pointer in parameter is always possible.
So, the value of the pointer should be check at the beginning of these functions.

Then, if the value of the pointer in parameter is NULL, the function should stop and return an error. These functions should return an integer, according to errorcodes.h, so, EC_NULL_POINTER.

Finally, after each call to these functions, the return value (the error) should be checked, and execution flow should change according to the error.

This is in general a good practice, here in this project, it is not a problem because it was designed and implemented in such way that these error checks are not necessary (it works without).

However, if someone call an internal dangerous function of the API (a function which take pointer(s) in parameter and return nothing), then it can crash the developer program.

We can find in the following part of the issue the different function which are dangerous regarding the described problem.

• In example.c :

print_hex function

void print_hex(unsigned char *bytes, size_t len) {
    for (size_t i = 0; i < len; i++) {
        printf("%02x", bytes[i]);
    }
}

(Also present in playground.c, print_hex)

convert_entropy_to_mnemonics function

void convert_mnemonic_to_entropy(char *mnemonics_file_name) {
  /* Code */
}

convert_mnemonics_to_seed function

void convert_mnemonics_to_seed(char *mnemonics_file_name) {
  /* Code */
}

convert_mnemonic_to_entropy function

void convert_mnemonic_to_entropy(char *mnemonics_file_name) {
  /* Code */
}

• In mnemonics.c :

array_128_pad function

void array_128_pad(unsigned char array[128], unsigned char pad) {
    for (uint8_t i = 0; i < 128; i++) {
        array[i] ^= pad;
    }
}

array_64_xor function

void array_64_xor(uint8_t dst[64], uint8_t src[64]) {
    for (uint8_t i = 0; i < 64; i++) {
        dst[i] ^= src[i];
    }
}

[NimRo97]

For the playground, it is just a test file for the developers and we should not have included it in the same directory in the first place.

For the example file, we are trying to make it simple. It is not a part of codebase we expect to be included. We need the usage of the 3 functions from our library to be visible and clear in the example, that is why the return errors are omitted, along with other good programming practices.

For the 2 functions from mnemonics.c, these can never fail in any way other than with segfault, therefore they could never return anything else than 0. Adding return 0 (or return EC_OK) at the end is pointless and these functions are not visible through the API (.h files), so we would prefer to keep them this way.

[AlexInsa]

I understand for the playground.c file, and I understand too that, because it is not included in the API, the example.c file does not respect all the good programming practices.

It is linked to another issue with the project. I did not create an issue about it because I was not sure about the role of the example.c file.

Can you confirm that example.c is the CLI ?

If yes, there is not the verification functionality "phrase & seed --> OK/NOK".

About the two functions in the mnemonics.c, yes you are right, they cannot crash if there are only used by the internal functions, and not available for the developer. However in a binary produced by C language, these functions will always be reachable.