[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Review of SSH1 code removal


On Friday, 29 June 2018 09:41:25 CEST Jakub Jelen wrote:
> Changeset:
> https://git.libssh.org/users/asn/libssh.git/commit/?h=master-ssh1

Hi Jakub,


thank you very much for the review.

> 
> client.c:
>              /* If SSHv1 is disabled, we can send the banner immedietly
> */

Fixed.

>  -- there is no need to leave comments referencing ssh1 anymore.
> 
> 
> obj/build_make.sh
>                       OPTIONS="${OPTIONS} -DWITH_SSH1=ON"

Fixed.

>  -- This should not reference SSH1 either
> 
> buffer.{c,h}
> 
> struct ssh_string_struct *ssh_buffer_get_mpint(struct ssh_buffer_struct
> *buffer) {

Fixed.

>  -- this function is unused: * @note This function is SSH-1 only.
> 
> 
> I am wondering what about the SSH_KEYTYPE_RSA1 key format. This is used
> all over the code and (even though it is probably unreachable) it
> should go away too. There is no good use of these keys in SSH2.
> 
> Otherwise the changes look good and running the tests on this branch
> gives no error.

Yes, you're right. I've handled them. We can't remove the enum as this is 
public API, but we should not handle it and return an error.


I've pushed an update to the branch. Could you take another look?


Thanks!


	Andreas


-- 
Andreas Schneider                 asn@xxxxxxxxxxxxxx
GPG-ID:     8DFF53E18F2ABC8D8F3C92237EE0FC4DCC014E3D



Follow-Ups:
Re: Review of SSH1 code removalJakub Jelen <jjelen@xxxxxxxxxx>
References:
Review of SSH1 code removalJakub Jelen <jjelen@xxxxxxxxxx>
Archive administrator: postmaster@lists.cynapses.org