From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id AFFD33857C4A for ; Thu, 17 Mar 2022 05:05:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AFFD33857C4A Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-340-huN52SXmN3e14yPBR8Ohew-1; Thu, 17 Mar 2022 01:05:42 -0400 X-MC-Unique: huN52SXmN3e14yPBR8Ohew-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id DEFFA299E750; Thu, 17 Mar 2022 05:05:41 +0000 (UTC) Received: from greed.delorie.com (ovpn-112-4.rdu2.redhat.com [10.10.112.4]) by smtp.corp.redhat.com (Postfix) with ESMTPS id CBBE941136E2; Thu, 17 Mar 2022 05:05:41 +0000 (UTC) Received: from greed.delorie.com.redhat.com (localhost [127.0.0.1]) by greed.delorie.com (8.15.2/8.15.2) with ESMTP id 22H55f6C1745292; Thu, 17 Mar 2022 01:05:41 -0400 From: DJ Delorie To: Siddhesh Poyarekar Cc: libc-alpha@sourceware.org Subject: Re: [PATCH v2 11/12] gaih_inet: Split result generation into its own function In-Reply-To: <20220314094835.1159523-12-siddhesh@sourceware.org> (message from Siddhesh Poyarekar via Libc-alpha on Mon, 14 Mar 2022 15:18:34 +0530) Date: Thu, 17 Mar 2022 01:05:41 -0400 Message-ID: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Mar 2022 05:05:44 -0000 Siddhesh Poyarekar via Libc-alpha writes: > Simplify the loop a wee bit and clean up variable names too. FYI combining "split out" and "minor changes" makes it harder to review, because the minor changes are nearly impossible to spot in the large chunks of "moving". I had one comment about a leak (see below) but it's independent of this patch set (maybe?). LGTM. Reviewed-by: DJ Delorie > diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c > index 3aeeb0d1e0..eebed824a8 100644 > --- a/sysdeps/posix/getaddrinfo.c > +++ b/sysdeps/posix/getaddrinfo.c > @@ -1028,6 +1028,87 @@ get_local_addresses (const struct addrinfo *req, struct gaih_result *res) > } > } > > +/* Generate results in PAI and its count in NADDRS. Return 0 on success or an > + error code on failure. */ > + > +static int > +generate_addrinfo (const struct addrinfo *req, struct gaih_result *res, > + const struct gaih_servtuple *st, struct addrinfo **pai, > + unsigned int *naddrs) > +{ > + . . . Ok. > + for (int i = 0; st[i].set; i++) > + { > + struct addrinfo *ai; > + ai = *pai = malloc (sizeof (struct addrinfo) + socklen); > + if (ai == NULL) > + return -EAI_MEMORY; I think there might be a memory leak here if malloc fails on the second or later iteration of the loop. However, we would have had that leak before, too, and if we're out of memory... this type of leak is likely not what you'd be worried about. > + . . . > + return 0; > +} > + Ok. > > process_list: > + /* Set up the canonical name if we need it. */ > + if ((result = process_canonname (req, orig_name, &res)) != 0) > + goto free_and_return; > - { > - /* Set up the canonical name if we need it. */ > - if ((result = process_canonname (req, orig_name, &res)) != 0) > - goto free_and_return; > - > - . . . > - } > - } Ok. > + result = generate_addrinfo (req, &res, st, pai, naddrs); Ok. > - free_and_return: > +free_and_return: Ok.