Re: [PATCH v4] Add a feature for mapping a host unix socket to a guest tcp socket

[Samuel Thibault <samuel.thibault@gnu.org>]

We're almost there :)

Viktor Kurilko, le mar. 05 août 2025 21:52:50 +0700, a ecrit:
> @@ -824,30 +827,81 @@ static int slirp_hostfwd(SlirpState *s, const char *redir_str, Error **errp)
>          is_udp = 0;
>      } else if (!strcmp(buf, "udp")) {
>          is_udp = 1;
> -    } else {
> -        fail_reason = "Bad protocol name";
> -        goto fail_syntax;
>      }
> -
> -    if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
> -        fail_reason = "Missing : separator";
> -        goto fail_syntax;
> +#if !defined(WIN32) && SLIRP_CHECK_VERSION(4, 7, 0)
> +    else if (!strcmp(buf, "unix")) {
> +        is_unix = 1;
>      }
> -    if (buf[0] != '\0' && !inet_aton(buf, &host_addr.sin_addr)) {
> -        fail_reason = "Bad host address";
> +#endif
> +    else {
> +        fail_reason = "Bad protocol name";
>          goto fail_syntax;
>      }
>  
> -    if (get_str_sep(buf, sizeof(buf), &p, '-') < 0) {
> -        fail_reason = "Bad host port separator";
> +    if (get_str_sep(buf, sizeof(buf), &p, is_unix ? '-' : ':') < 0) {

This factorization looks odd. I don't think it's actually more readable
to try to factorize some separator parsing. I'd say rather have an if
(unix) part and else (ip) part, which both make their own get_str_sep
calls, and factorize just the parsing of the guest part.

> +#endif
> +    {
> +        if (buf[0] != '\0' && !inet_aton(buf, &host_addr.in.sin_addr)) {
> +            fail_reason = "Bad host address";
> +            goto fail_syntax;
> +        }
> +
> +        if (get_str_sep(buf, sizeof(buf), &p, '-') < 0) {
> +            fail_reason = "Bad host port separator";
> +            goto fail_syntax;
> +        }
> +
> +        err = qemu_strtoi(buf, &end, 0, &host_port);
> +        if (err || host_port < 0 || host_port > 65535) {
> +            fail_reason = "Bad host port";
> +            goto fail_syntax;
> +        }
> +        host_addr.in.sin_family = AF_INET;
> +        host_addr.in.sin_addr.s_addr = INADDR_ANY;

This is overwriting the inet_aton call above?

> +        host_addr.in.sin_port = htons(host_port);
> +        host_addr_size = sizeof(host_addr.in);
>      }
> -    host_addr.sin_port = htons(host_port);
>  
>      if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
>          fail_reason = "Missing guest address";

Samuel