Re: [PATCH v2] alfred: add more verbose error message

[Sven Eckelmann <sven@narfation.org>]

[-- Attachment #1: Type: text/plain, Size: 2559 bytes --]

On Tuesday, 10 September 2024 05:43:25 CEST noahbpeterson1997@gmail.com wrote:

You must have a proper "From:" in your patch mail - not only your mail 
address. And it should match your Signed-off-by line.

> There is an issue with some Linux distributions where network interfaces are up and
> active, but do not have the correct link-local address. This condition is now checked and
> output to stderr to help better users troubleshoot this issue.

Please don't use overlong lines in the commit message. We normally use the 75 
chars per line limit from Linux's checkpatch.

> 
> Signed-off-by: Noah Peterson noahbpeterson1997@gmail.com

This signed-off-by has an invalid formatted e-mail. I think it should have 
been:

Signed-off-by: Noah Peterson <noahbpeterson1997@gmail.com>

> ---

Changelog to v1 is missing. You can use tools like b4 [1] to improve your 
patch workflow.

> netsock.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
> diff --git a/netsock.c b/netsock.c
> index feed21d..a3101df 100644
> --- a/netsock.c
> +++ b/netsock.c
> @@ -21,6 +21,7 @@
>  #include <sys/types.h>
>  #include <stdlib.h>
>  #include <sys/epoll.h>
> +#include <arpa/inet.h>
>  #ifdef CONFIG_ALFRED_CAPABILITIES
>  #include <sys/capability.h>
>  #endif
> @@ -321,8 +322,19 @@ static int netsock_open(struct globals *globals, struct interface *interface)
>         }
>         enable_raw_bind_capability(0);
>  
> +

This newline doesn't make any sense.

>         if (bind(sock, (struct sockaddr *)&sin6, sizeof(sin6)) < 0) {
> -               perror("can't bind");
> +               char ipstr_buf[INET6_ADDRSTRLEN];
> +               const char *ipstr;

Missing blank line after declaration

> +               ipstr = inet_ntop(AF_INET6, &interface->address.ipv6.s6_addr,
> +                                       ipstr_buf, INET6_ADDRSTRLEN);

Start of line doesn't match open parenthesis

> +               if (errno == EADDRNOTAVAIL)
> +                       fprintf(stderr, "can't bind to interface %s; "
> +                               "expected ipv6 address not found: %s\n",
> +                               interface->interface,
> +                               ipstr);
> +               else
> +                       perror("can't bind");
>                 goto err;
>         }

You should precheck your patches with the most recent linux-next's checkpatch. 
Something like:

    ~/linux-next/scripts/checkpatch.pl --strict 0001-alfred-add-more-verbose-error-message.patch

Kind regards,
	Sven

[1] https://b4.docs.kernel.org/

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]