From: Greg Minshall <minshall@umich.edu>
To: Alejandro Colomar <alx@kernel.org>
Cc: linux-man@vger.kernel.org
Subject: Re: getaddrinfo_a man page: add notification example?
Date: Fri, 01 Nov 2024 10:57:07 -0700 [thread overview]
Message-ID: <76180.1730483827@archlinux> (raw)
In-Reply-To: <20241101134653.3vwbgzk3ffegckzh@devuan>
hi, Alejandro,
thanks for the e-mail and code inspection.
> > 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'
> };
that works. i like that, by initializing the tags with, e.g., " = 'n'",
i can still use the user's input to set values, without needing some
sort of a lookup.
: echo -ne 'n signal\na example.com\nw 0' | ./manpage-like-gai
> > 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.
here i'm thinking of the case where the program gets its input via a
pipe, which may present an EOF without a trailing newline. i'll be
to follow your guidance here.
> > 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.
sure, thanks.
> > /* 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.
that seems nice. i think i didn't touch this code, but let me know if
you'd like me to add this to my submission.
again, thanks.
cheers, Greg
next prev parent reply other threads:[~2024-11-01 17:59 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-21 8:50 getaddrinfo_a man page: add notification example? Greg Minshall
2024-08-21 10:36 ` Alejandro Colomar
2024-08-21 13:09 ` Greg Minshall
2024-08-25 10:17 ` Greg Minshall
2024-08-25 10:48 ` Alejandro Colomar
2024-08-25 11:29 ` Greg Minshall
2024-11-01 13:46 ` Alejandro Colomar
2024-11-01 17:57 ` Greg Minshall [this message]
2024-11-01 20:01 ` Alejandro Colomar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=76180.1730483827@archlinux \
--to=minshall@umich.edu \
--cc=alx@kernel.org \
--cc=linux-man@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.