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

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/