On Wed, May 10, 2017 at 5:58 PM, Heikki Linnakangas <hlinn...@iki.fi> wrote:
> On 05/10/2017 08:01 AM, Michael Paquier wrote: > >> On Wed, May 10, 2017 at 10:51 AM, Vaishnavi Prabakaran >> <vaishnaviprabaka...@gmail.com> wrote: >> >>> Following recent removal of support to store password in plain text, >>> modified the code to >>> >>> 1. Remove "PASSWORD_TYPE_PLAINTEXT" macro >>> 2. Instead of using "get_password_type" to retrieve the encryption method >>> AND to check if the password is already encrypted or not, modified the >>> code >>> to >>> a. Use "get_password_encryption_type" function to retrieve encryption >>> method. >>> b. Use "isPasswordEncrypted" function to check if the password is already >>> encrypted or not. >>> >>> These changes are mainly to increase code readability and does not change >>> underlying functionality. >>> >> >> Actually, this patch reduces readability. >> > > Yeah, I don't think this is an improvement. Vaishnavi, did you find the > current code difficult? Perhaps some extra comments somewhere would help? > Thanks for willing to add extra comments, And current code is not difficult but kind of gives the impression that still plaintext password storage exists by holding "PASSWORD_TYPE_PLAINTEXT" macro and having switch case checks for this macro. I see "get_password_type" function call is used for 2 purposes. One is to check the actual password encryption type during authentication process and another purpose is to verify if the password passed is encrypted or not - used in create/alter role command before checking the strength of password(via passwordcheck module). So, I thought my patch will make this purpose clear. Nevertheless, if people thinks this actually reduces their readability then I don't mind to go with the flow. Thanks & Regards, Vaishnavi Fujitsu Australia.
