Re: [Qemu-devel] [PATCH v9] ssh: switch from libssh2 to libssh

[Philippe Mathieu-Daudé]

On 6/14/19 4:34 PM, Max Reitz wrote:
> On 14.06.19 16:29, Pino Toscano wrote:
>> On Thursday, 13 June 2019 21:31:40 CEST Max Reitz wrote:
>>> On 13.06.19 15:20, Pino Toscano wrote:
[...]
>>>> -    case LIBSSH2_KNOWNHOST_CHECK_NOTFOUND:
>>>> +    case SSH_KNOWN_HOSTS_OTHER:
>>>>          ret = -EINVAL;
>>>> -        session_error_setg(errp, s, "no host key was found in 
>>>> known_hosts");
>>>> +        error_setg(errp,
>>>> +                   "host key for this server not found, another type 
>>>> exists");
>>>>          goto out;
>>>> -    case LIBSSH2_KNOWNHOST_CHECK_FAILURE:
>>>> +    case SSH_KNOWN_HOSTS_UNKNOWN:
>>>>          ret = -EINVAL;
>>>> -        session_error_setg(errp, s,
>>>> -                      "failure matching the host key with known_hosts");
>>>> +        error_setg(errp, "no host key was found in known_hosts");
>>>> +        goto out;
>>>> +    case SSH_KNOWN_HOSTS_NOT_FOUND:
>>>> +        ret = -ENOENT;
>>>> +        error_setg(errp, "known_hosts file not found");
>>>> +        goto out;
>>>> +    case SSH_KNOWN_HOSTS_ERROR:
>>>> +        ret = -EINVAL;
>>>> +        error_setg(errp, "error while checking the host");
>>>>          goto out;
>>>>      default:
>>>>          ret = -EINVAL;
>>>> -        session_error_setg(errp, s, "unknown error matching the host key"
>>>> -                      " with known_hosts (%d)", r);
>>>> +        error_setg(errp, "error while checking for known server");
>>>>          goto out;
>>>>      }
>>>> +#else /* !HAVE_LIBSSH_0_8 */
>>>> +    int state;
>>>> +
>>>> +    state = ssh_is_server_known(s->session);
>>>> +    trace_ssh_server_status(state);
>>>> +
>>>> +    switch (state) {
>>>> +    case SSH_SERVER_KNOWN_OK:
>>>> +        /* OK */
>>>> +        trace_ssh_check_host_key_knownhosts();
>>>> +        break;
>>>> +    case SSH_SERVER_KNOWN_CHANGED:
>>>> +        ret = -EINVAL;
>>>> +        error_setg(errp, "host key does not match the one in 
>>>> known_hosts");
>>>> +        goto out;
>>>> +    case SSH_SERVER_FOUND_OTHER:
>>>> +        ret = -EINVAL;
>>>> +        error_setg(errp,
>>>> +                   "host key for this server not found, another type 
>>>> exists");
>>>> +        goto out;
>>>> +    case SSH_SERVER_FILE_NOT_FOUND:
>>>> +        ret = -ENOENT;
>>>> +        error_setg(errp, "known_hosts file not found");
>>>> +        goto out;
>>>> +    case SSH_SERVER_NOT_KNOWN:
>>>> +        ret = -EINVAL;
>>>> +        error_setg(errp, "no host key was found in known_hosts");
>>>> +        goto out;
>>>> +    case SSH_SERVER_ERROR:
>>>> +        ret = -EINVAL;
>>>> +        error_setg(errp, "server error");
>>>> +        goto out;
>>>
>>> No default here?
>>
>> This switch is for libssh < 0.8.0, so enumerating all the possible
>> values of the enum of the old API is enough.
> 
> state is an integer.  I feel very uneasy about not having a default
> clause for a plain integer, especially if it is supplied by an external
> library.

Agreed. What's odd is I tested it on Ubuntu Xenial which is 0.6.3 and no
got no cpp warning. I wonder if it is using a backported patch adding
ssh_session_is_known_server(), like 0.7.1 on Ubuntu Bionic. Anyway,
better add a default.

signature.asc
Description: OpenPGP digital signature