Re: [PATCH 1/2] vchan-socket-proxy: add reconnect marker support

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Mon, Sep 05, 2022 at 03:50:18PM +0200, Marek Marczykowski-Górecki wrote:
> +                reconnect_marker_value = atoi(optarg);

atoi() isn't great, if there's garbage it just return 0, which is
validated below.

Would there be value here in using strtol() which can detect input
error? To at least help with maybe hard to debug issues?

> +                if (reconnect_marker_value < 0 || reconnect_marker_value > 
> 255) {
> +                    fprintf(stderr, "invalid argument for 
> --reconnect-marker, "
> +                                    "must be a number between 0 and 255\n");
> +                    usage(argv);
> +                }
> +                break;
>              case '?':
>                  usage(argv);
>          }
> @@ -509,6 +549,15 @@ int main(int argc, char **argv)
>                  ret = 1;
>                  break;
>              }
> +            if (reconnect_marker_value != -1) {
> +                const char marker_buf[] = { reconnect_marker_value };
> +
> +                if (libxenvchan_write(state.ctrl, marker_buf, 
> sizeof(marker_buf))
> +                        != sizeof(marker_buf)) {
> +                    fprintf(stderr, "failed to send reconnect marker\n");

Is this an expected failure? If so, maybe adding "(ignored)" might be
valuable to someone reading the logs?

> +                    break;
> +                }
> +            }
>              if (data_loop(&state) != 0)
>                  break;
>              /* don't reconnect if output was stdout */


Otherwise, the patch looks fine. Even if kept as is:
Acked-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

Thanks,

-- 
Anthony PERARD