Used and Useless variable

[AlexInsa]

This is more a style problem, but also a question about the design of the function entropy_to_mnemonic in mnemonics.c.

A variable (const struct dictionary *dict) is declared and used in the code, but it is only a copy of one of the parameter (const struct dictionary *dictionary).

During the function, the parameter is replaced by another local variable. However, this new function seems to be useless, because the variable in parameter (const struct dictionary *dictionary) can do the same job.

Current code using (const struct dictionary *dict) :

    const struct dictionary *dict = dictionary;

    if (dictionary == NULL) {
        dict = default_EN_dictionary();
    }

        if (len + strlen((char*) (dict->words[joint])) + 1 >= capacity) {
            capacity *= 2;
            unsigned char *temp = realloc(out, capacity);
            if (temp == NULL) {
                free(bytes);
                free(out);
                return EC_ALLOCATION_ERROR;
            }
            out = temp;
        }
        sprintf((char*) out + len, "%s", dict->words[joint]);
        len += strlen((char*) (dict->words[joint]));

It is not clear why this new variable is created. In addition, the program works also if we do not use this dict variable, and if we only use (const struct dictionary *dictionary) :

    if (dictionary == NULL) {
        dictionary = default_EN_dictionary();
    }

        if (len + strlen((char*) (dictionary->words[joint])) + 1 >= capacity) {
            capacity *= 2;
            unsigned char *temp = realloc(out, capacity);
            if (temp == NULL) {
                free(bytes);
                free(out);
                return EC_ALLOCATION_ERROR;
            }
            out = temp;
        }
        sprintf((char*) out + len, "%s", dictionary->words[joint]);
        len += strlen((char*) (dictionary->words[joint]));

Finally, during the compilation and according to the options and optimization, it is possible that this variable (const struct dictionary *dict) would be automatically deleted.

Is there a particular reason why this local variable (const struct dictionary *dict) has been used ?

[NimRo97]

This was intended because I as the programmer do not like changing arguments within a function. I also wanted it to be clear that the dictionary used during computation does not necessarily have to be the same that we got in an argument (it can default to our EN). There is also a point to having this argument as const, because it shows the programmer, that the provided dictionary is not changed and this function indeed treats it like const. I am aware that this variable is omitted during compilation and I deliberately chose to include it nevertheless for the sake of the code.

I will mark this as not a bug, but thank you for the question. I would also like to hear your arguments for removing it because my point of view can be absolutely stupid and it may be irrelevant to most other developers :)

[AlexInsa]

Indeed, it is only one pointer more in the stack, it is not really important.

In addition, I agree that the it can add more legibility in the code. It is something really interesting because user could think that, if they give a pointer in parameter, and if the pointer is on NULL, then they would get the default dictionary at the end of the function execution.

For instance :

struct dictionary *dictionary = NULL;
int err = entropy_to_mnemonic(my_dico, entropy, entropy_l, &output);

The function, even if my_dico is NULL before, will not change the value of my_dico.
So, creating a local variable, to show that the dictionary used in the function has not any impact on the arguments, can be useful for beginner.