Re: [PATCH gwproxy v4 4/6] dnslookup: Initial work for implementation of C-ares-like getaddrinfo function

[Ahmad Gani <reyuki@gnuweeb.org>]

On Wed, Aug 6, 2025 at 1:41 PM Alviro Iskandar Setiawan wrote:
> On Wed, Aug 6, 2025 at 10:57 AM Ahmad Gani wrote:
> > +int gw_ares_init(gw_ares_channel_t **channel, struct gw_ares_options *opts)
> > +{
> > +       gw_ares_channel_t *c;
> > +       int ret;
> > +
> > +       if (!opts->nr_server)
> > +               return -EINVAL;
> > +
> > +       c = malloc(sizeof(*c));
> > +       if (!c)
> > +               return -ENOMEM;
> > +
> > +       *channel = c;
> > +       c->nr_server = opts->nr_server;
> > +       c->servers = malloc(c->nr_server * sizeof(*c->servers));
> > +       if (!c->servers) {
> > +               free(c);
> > +               return -ENOMEM;
> > +       }
> > +       /*
> > +        * TODO(reyuki): validate flags and
> > +        * for now use it to control recursion desired (RD) bit?
> > +        */
> > +       c->flags = opts->flags;
> > +       for (int i = 0; i < c->nr_server; i++) {
> > +               ret = convert_str_to_ssaddr(opts->servers[i], &c->servers[i], DEFAULT_DOMAIN_PORT);
> > +               if (ret) {
> > +                       free(c->servers);
> > +                       free(c);
> > +                       return ret;
> > +               }
> > +       }
> > +
> > +       return 0;
> > +}
>
> Again, I don't see any point in filling `*channel` with a dangling
> pointer on error.
>
> This part "*channel = c", can't you do that only when successful?
>
> Please address the previous review properly. Don't ignore comments.

Eh, sorry—I overlooked that. I didn't recheck it because I assumed it was
acceptable, since there were no comments on patchset v2 where the change
was first made. By "avoiding a dangling pointer", I thought the request
meant not to assign to the channel if memory allocation for c failed.

Anyway, don't worry, I will update it in the next patchset.

--
Ahmad Gani