Re: [PATCH] diffie-hellman-group-exchange-sha256

[Andreas Schneider <asn@xxxxxxxxxxxxxx>]

On Tuesday 20 January 2015 12:46:22 Yanis Kurganov wrote:
> Hi, Andreas!

Hi Yanis,

> Server side is ready!

That's great. Thanks for your contribution! I would like to see some changes 
to the code for inclusion. It following our CodingStyle, small cleanups and 
splitting it into more patches.

> My tests with dh-group-exchange client (PuTTY) are OK.
> What about tests? What format of the tests do you want?

Please do not call functions in if-statement or in function parameters. They 
make it harder to debug code.

if (buffer_get_u32(session->in_buffer, &pbits) != sizeof(uint32_t)) {
	...
}

should be:

int rc;

rc = buffer_get_u32(session->in_buffer, &pbits);
if (rc != sizeof(uint32_t)) {
	...
}

It makes it easier to debug code, also please also add braces to all 
if/for/while/etc. clauses!

See also:
http://git.libssh.org/projects/libssh.git/tree/README.CodingStyle
and
https://blog.cryptomilk.org/2013/03/28/writing-and-reading-code/


I would also split the commit. Renames like dh_import_p -> dh_import_p_string 
should be in an own commit. They are not really related to diffie-hellman-
group-exchange-sha256.


dh_send_group_server() should use ssh_buffer_pack().



Thanks!


	-- andreas






-- 
Andreas Schneider                   GPG-ID: CC014E3D
www.cryptomilk.org                asn@xxxxxxxxxxxxxx