Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

md5: Added mbedtls implementation #4980

Closed
wants to merge 3 commits into from

Conversation

captain-caveman2k
Copy link
Member

@captain-caveman2k captain-caveman2k commented Feb 25, 2020

No description provided.

lib/md5.c Outdated
(void) mbedtls_md5_update_ret(ctx, data, length);
}

static void MD5_Final(unsigned char digest[16], MD5_CTX *ctx)
Copy link
Member

@jay jay Feb 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get why you did this (because mbedtls does I assume) but I think the fixed size array style is misleading, if you sizeof(digest) it's going to be the size of the pointer not of the array. If we are passing *input I think it should just be passed as that.

Copy link
Member

@jay jay Feb 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NEvermind it looks like that's the current convention, I'd stick with it.

Copy link
Member Author

@captain-caveman2k captain-caveman2k Feb 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately it's not related to mbedtls, the other (well... GnuTLS based) functions use this style and you'll also note that the SHA256 code does this too (I'm guessing as the MD5 code was used as a template).

I'm personally not a fan if it because of it for the reason you gave ;-) As such you'll note I've used pointer notation in PR #4956 for my new SHA256 functions and corrected the existing function to match.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants