Re: getaddrinfo_a man page: add notification example?

[Alejandro Colomar <alx@kernel.org>]

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

Hi Greg,

On Sun, Aug 25, 2024 at 02:29:13PM +0300, Greg Minshall wrote:
> Alex,
> 
> > Would you mind sending the extracted files?  That way I can comment on
> > those directly.
> 
> here you go.
> 
> cheers, Greg
> 
> 
> ----
> 
> #define _GNU_SOURCE
> #include <assert.h>
> #include <err.h>
> #include <netdb.h>
> #include <signal.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> 
> #define CALLOC(n, type)  ((type *) calloc(n, sizeof(type)))
> 
> #define REALLOCF(ptr, n, type)                                          \
> ({                                                                      \
>     static_assert(__builtin_types_compatible_p(typeof(ptr), type *));   \
>                                                                         \
>     (type *) reallocarrayf(ptr, n, sizeof(type));                       \
> })
> 
> static struct gaicb **reqs = NULL;
> static size_t nreqs = 0;
> /* default is "no notification" ('n') */
> static char notification = 'n';

Would it be better to use an enum instead of comments?

	enum {
		NOTIFICATION_NONE = 'n',
		NOTIFICATION_SIGNAL = 's',
		NOTIFICATION_CALLBACK = 'c'
	};

> 
> /* forward declaration for callback routines */
> static void list_requests(void);
> 
> 
> static inline void *
> reallocarrayf(void *p, size_t nmemb, size_t size)
> {
>     void  *q;
> 
>     q = reallocarray(p, nmemb, size);
>     if (q == NULL && nmemb != 0 && size != 0)
>         free(p);
>     return q;
> }
> 
> static char *
> getcmd(void)
> {
>     static char buf[256];
> 
>     fputs("> ", stdout); fflush(stdout);
>     if (fgets(buf, sizeof(buf), stdin) == NULL)
>         return NULL;
> 
>     if (buf[strlen(buf) - 1] == '\n')
>         buf[strlen(buf) - 1] = 0;

If the string does not contain a newline, it probably means something is
wrong.  Returning as if all were good is probably not a good idea.

I suggest

	static inline char *
	stpsep(char *s, const char *delim)
	{
		strsep(&s, delim);
		return s;
	}

and then:

	if (stpsep(buf, "\n") == NULL)
		return NULL;



> 
>     return buf;
> }
> 
> /* Set notification type: none (default), signal, callback */
> static void
> notification_type(void)
> {
>     char *type = strtok(NULL, " ");
> 
>     switch (type[0]) {
>     case 'c':
>     case 'n':
>     case 's':
>         notification = type[0];
>         break;
>     default:
>         fprintf(stderr, "Bad type: '%c' (expecting 'c', 's', or 'n')\n", type[0]);
>         break;
>     }
> }
> 
> /* callback routine for signal notifications */
> 
> static void
> signal_handler(int signo) {
>     fprintf(stdout, "notified by signal\n");
>     list_requests();
>     fprintf(stdout, "> ");
>     fflush(stdout);
> }
> 
> /* callback routine for thread/callback notifications */
> 
> static void
> callback_handler(union sigval sev_value) {
>     fprintf(stdout, "notified by callback\n");
>     list_requests();
>     fprintf(stdout, "> ");
>     fflush(stdout);
> }
> 
> /* Add requests for specified hostnames. */
> static void
> add_requests(void)
> {
>     static struct sigevent senull; /* static, so initialized to zero */
>     static struct sigaction sanull; /* static, so intitialized to zero */

These comments are redundant.  Please remove them.  Maybe add a blank
line between static variables and automatic ones to make it more
evident.

>     struct sigevent se = senull;
>     struct sigaction sa = sanull;
>     size_t nreqs_base = nreqs;
>     char *host;
>     int ret;
> 
>     while ((host = strtok(NULL, " "))) {
>         nreqs++;
>         reqs = REALLOCF(reqs, nreqs, struct gaicb *);
>         if (reqs == NULL)
>             err(EXIT_FAILURE, "reallocf");
> 
>         reqs[nreqs - 1] = CALLOC(1, struct gaicb);
>         if (reqs[nreqs - 1] == NULL)
>             err(EXIT_FAILURE, "calloc");
> 
>         reqs[nreqs - 1]->ar_name = strdup(host);
>     }
> 
>     switch (notification) {
>     case 'c':
>         /* notify via a callback */
>         se.sigev_notify = SIGEV_THREAD;
>         se.sigev_notify_function = callback_handler;
>         break;
>     case 'n':
>         /* nothing to do */
>         break;
>     case 's':
>         se.sigev_notify = SIGEV_SIGNAL;
>         se.sigev_signo = SIGUSR1;
>         sa.sa_handler = signal_handler;
>         /* set SA_RESTART so read(2) in main doesn't get an EINTR */
>         sa.sa_flags = SA_RESTART;
>         ret = sigaction(SIGUSR1, &sa, NULL);
>         if (ret) {
>             err(EXIT_FAILURE, "sigaction");
>         }
>         break;
>     default:
>         fprintf(stderr, "program error: `notification` not one of [cns]: %c\n",
>                 notification);
>         exit(4);
>     }
> 
>     /* Queue nreqs_base..nreqs requests. */
> 
>     ret = getaddrinfo_a(GAI_NOWAIT, &reqs[nreqs_base],
>                         nreqs - nreqs_base, notification == 'n' ? NULL : &se);
>     if (ret) {
>         fprintf(stderr, "getaddrinfo_a() failed: %s\n",
>                 gai_strerror(ret));
>         exit(EXIT_FAILURE);
>     }
> }
> 
> /* Wait until at least one of specified requests completes. */
> static void
> wait_requests(void)
> {
>     char *id;
>     int ret;
>     size_t n;
>     struct gaicb const **wait_reqs;
> 
>     wait_reqs = CALLOC(nreqs, const struct gaicb *);
>     if (wait_reqs == NULL)
>         err(EXIT_FAILURE, "calloc");
> 
>                 /* NULL elements are ignored by gai_suspend(). */
> 
>     while ((id = strtok(NULL, " ")) != NULL) {
>         n = atoi(id);
> 
>         if (n >= nreqs) {
>             printf("Bad request number: %s\n", id);
>             return;
>         }
> 
>         wait_reqs[n] = reqs[n];
>     }
> 
>     ret = gai_suspend(wait_reqs, nreqs, NULL);
>     if (ret) {
>         printf("gai_suspend(): %s\n", gai_strerror(ret));
>         return;
>     }
> 
>     for (size_t i = 0; i < nreqs; i++) {
>         if (wait_reqs[i] == NULL)
>             continue;
> 
>         ret = gai_error(reqs[i]);
>         if (ret == EAI_INPROGRESS)
>             continue;
> 
>         printf("[%02zu] %s: %s\n", i, reqs[i]->ar_name,
>                ret == 0 ? "Finished" : gai_strerror(ret));
>     }
> }
> 
> /* Cancel specified requests. */
> static void
> cancel_requests(void)
> {
>     char *id;
>     int ret;
>     size_t n;
> 
>     while ((id = strtok(NULL, " ")) != NULL) {
>         n = atoi(id);
> 
>         if (n >= nreqs) {
>             printf("Bad request number: %s\n", id);
>             return;
>         }
> 
>         ret = gai_cancel(reqs[n]);
>         printf("[%s] %s: %s\n", id, reqs[atoi(id)]->ar_name,
>                gai_strerror(ret));
>     }
> }
> 
> /* List all requests. */
> static void
> list_requests(void)
> {
>     int ret;
>     char host[NI_MAXHOST];
>     struct addrinfo *res;
> 
>     for (size_t i = 0; i < nreqs; i++) {
>         printf("[%02zu] %s: ", i, reqs[i]->ar_name);
>         ret = gai_error(reqs[i]);
> 
>         if (!ret) {
>             res = reqs[i]->ar_result;
> 
>             ret = getnameinfo(res->ai_addr, res->ai_addrlen,
>                               host, sizeof(host),
>                               NULL, 0, NI_NUMERICHOST);
>             if (ret) {
>                 fprintf(stderr, "getnameinfo() failed: %s\n",
>                         gai_strerror(ret));
>                 exit(EXIT_FAILURE);
>             }
>             puts(host);
>         } else {
>             puts(gai_strerror(ret));

If you invert the conditional, you can add a continue after this, and
unindent the non-error code.

Thanks for the emanple program!

Have a lovely day!
Alex

>         }
>     }
> }
> 
> int
> main(void)
> {
>     char *cmdline;
>     char *cmd;
> 
>     while ((cmdline = getcmd()) != NULL) {
>         cmd = strtok(cmdline, " ");
> 
>         if (cmd == NULL) {
>             list_requests();
>         } else {
>             switch (cmd[0]) {
>             case 'a':
>                 add_requests();
>                 break;
>             case 'w':
>                 wait_requests();
>                 break;
>             case 'c':
>                 cancel_requests();
>                 break;
>             case 'l':
>                 list_requests();
>                 break;
>             case 'n':
>                 notification_type();
>                 break;
>             default:
>                 fprintf(stderr, "Bad command: %c\n", cmd[0]);
>                 break;
>             }
>         }
>     }
>     exit(EXIT_SUCCESS);
> }
> 

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]