Hi Greg,
On Fri, Nov 01, 2024 at 10:57:07AM -0700, Greg Minshall wrote:
> 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.
For serious programs, I prefer being POSIXly pedantic and reporting an
error for such files.
For an example program, for simplicity, we can do this:
stpcpy(strchrnul(buf, '\n'), "");
If removes the conditional, has less moving parts (no '-1'; and thus
less error-prone), and is protected by _FORTIFY_SOURCE.
>
>
> > > 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.
Hmmm, if you've copied this from existing code, we can keep it like that
and change it in a different commit.
Have a lovely night!
Alex
>
>
> again, thanks.
>
> cheers, Greg
>
>
--
<https://www.alejandro-colomar.es/>
Attachment:
signature.asc
Description: PGP signature
