From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 110485 invoked by alias); 5 Sep 2017 17:15:38 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 110415 invoked by uid 89); 5 Sep 2017 17:15:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-qt0-f172.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=wLFegUMXUV2zqaXqwIep9/k0hxAGxpicpQTuC/ggfJ4=; b=i6jqR9KyBS1B0qvNotPBDPw/42NawnYYo+7uXf5e9Si7+DbytNg15Ud9p1AhxRnL4q hq56y4Dz55YItT1Qiq/2IPjE1XF/oRCp7JTy7/3qXD+b0ugdHDETcxab4n/L2hfv2MJA lBkuZ10nw7KDCRlI307ZSKteEF5ufrWgdkKrv6eIf/w/t+y3I8goWbf4mEzJMan6cYr2 W4uwIzRm5iwn5CJ7U4lzZKT83Z2F8HFjJDcMDkqPeo4ZR4MvFKNyi8uWUI3rf/ftaYvt MuquL9m6NhtWkHPxIi/5FQeDBDPpUnQrc+V6mM3h9yAFK6dTHGiycML+VVJMvu4nRFv2 KMqg== X-Gm-Message-State: AHPjjUjWdZ6Deuo7N+L2uVIAXkl9H7BEXIlTMSykpKrY7hr08tYVhb1U +uFlRzB1l1GBP4Guy89zew== X-Google-Smtp-Source: ADKCNb6DeZDGjCx9TDEgDJPXPl5vp0FKeSqfYtp05r56Zqmun3qI44sbE/gLY+aSP7GUV8/stlZuZQ== X-Received: by 10.200.18.133 with SMTP id y5mr6245175qti.82.1504631709998; Tue, 05 Sep 2017 10:15:09 -0700 (PDT) Subject: Re: [PATCH] nss_files: Refactor gethostbyname3 multi case into separate function To: libc-alpha@sourceware.org References: <20170904173046.DA957439942E3@oldenburg.str.redhat.com> From: Adhemerval Zanella Message-ID: <9a115191-1153-db72-10ae-3d6eee187999@linaro.org> Date: Tue, 05 Sep 2017 17:15:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170904173046.DA957439942E3@oldenburg.str.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-09/txt/msg00194.txt.bz2 On 04/09/2017 14:30, Florian Weimer wrote: > This is in preparation of further cleanup work. > > 2017-09-04 Florian Weimer > > * nss/nss_files/files-hosts.c (gethostbyname3_multi): New > function. > (_nss_files_gethostbyname3_r): Call it. LGTM. > > diff --git a/nss/nss_files/files-hosts.c b/nss/nss_files/files-hosts.c > index bccb6a5780..867c10c2ef 100644 > --- a/nss/nss_files/files-hosts.c > +++ b/nss/nss_files/files-hosts.c > @@ -115,6 +115,206 @@ DB_LOOKUP (hostbyaddr, ,,, > }, const void *addr, socklen_t len, int af) > #undef EXTRA_ARGS_VALUE > > +static enum nss_status > +gethostbyname3_multi (FILE * stream, const char *name, int af, > + struct hostent *result, char *buffer, size_t buflen, > + int *errnop, int *herrnop, int flags) > +{ > + /* We have to get all host entries from the file. */ > + size_t tmp_buflen = MIN (buflen, 4096); > + char tmp_buffer_stack[tmp_buflen] > + __attribute__ ((__aligned__ (__alignof__ (struct hostent_data)))); > + char *tmp_buffer = tmp_buffer_stack; > + struct hostent tmp_result_buf; > + int naddrs = 1; > + int naliases = 0; > + char *bufferend; > + bool tmp_buffer_malloced = false; > + enum nss_status status; > + > + while (result->h_aliases[naliases] != NULL) > + ++naliases; > + > + bufferend = (char *) &result->h_aliases[naliases + 1]; > + > + again: > + while ((status = internal_getent (stream, &tmp_result_buf, tmp_buffer, > + tmp_buflen, errnop, herrnop, af, > + flags)) > + == NSS_STATUS_SUCCESS) > + { > + int matches = 1; > + struct hostent *old_result = result; > + result = &tmp_result_buf; > + /* The following piece is a bit clumsy but we want to use the > + `LOOKUP_NAME_CASE' value. The optimizer should do its > + job. */ > + do > + { > + LOOKUP_NAME_CASE (h_name, h_aliases) > + result = old_result; > + } > + while ((matches = 0)); > + > + if (matches) > + { > + /* We could be very clever and try to recycle a few bytes > + in the buffer instead of generating new arrays. But > + we are not doing this here since it's more work than > + it's worth. Simply let the user provide a bit bigger > + buffer. */ > + char **new_h_addr_list; > + char **new_h_aliases; > + int newaliases = 0; > + size_t newstrlen = 0; > + int cnt; > + > + /* Count the new aliases and the length of the strings. */ > + while (tmp_result_buf.h_aliases[newaliases] != NULL) > + { > + char *cp = tmp_result_buf.h_aliases[newaliases]; > + ++newaliases; > + newstrlen += strlen (cp) + 1; > + } > + /* If the real name is different add it also to the > + aliases. This means that there is a duplication > + in the alias list but this is really the user's > + problem. */ > + if (strcmp (old_result->h_name, > + tmp_result_buf.h_name) != 0) > + { > + ++newaliases; > + newstrlen += strlen (tmp_result_buf.h_name) + 1; > + } > + > + /* Make sure bufferend is aligned. */ > + assert ((bufferend - (char *) 0) % sizeof (char *) == 0); > + > + /* Now we can check whether the buffer is large enough. > + 16 is the maximal size of the IP address. */ > + if (bufferend + 16 + (naddrs + 2) * sizeof (char *) > + + roundup (newstrlen, sizeof (char *)) > + + (naliases + newaliases + 1) * sizeof (char *) > + >= buffer + buflen) > + { > + *errnop = ERANGE; > + *herrnop = NETDB_INTERNAL; > + status = NSS_STATUS_TRYAGAIN; > + goto out; > + } > + > + new_h_addr_list = > + (char **) (bufferend > + + roundup (newstrlen, sizeof (char *)) > + + 16); > + new_h_aliases = > + (char **) ((char *) new_h_addr_list > + + (naddrs + 2) * sizeof (char *)); > + > + /* Copy the old data in the new arrays. */ > + for (cnt = 0; cnt < naddrs; ++cnt) > + new_h_addr_list[cnt] = old_result->h_addr_list[cnt]; > + > + for (cnt = 0; cnt < naliases; ++cnt) > + new_h_aliases[cnt] = old_result->h_aliases[cnt]; > + > + /* Store the new strings. */ > + cnt = 0; > + while (tmp_result_buf.h_aliases[cnt] != NULL) > + { > + new_h_aliases[naliases++] = bufferend; > + bufferend = (__stpcpy (bufferend, > + tmp_result_buf.h_aliases[cnt]) > + + 1); > + ++cnt; > + } > + > + if (cnt < newaliases) > + { > + new_h_aliases[naliases++] = bufferend; > + bufferend = __stpcpy (bufferend, > + tmp_result_buf.h_name) + 1; > + } > + > + /* Final NULL pointer. */ > + new_h_aliases[naliases] = NULL; > + > + /* Round up the buffer end address. */ > + bufferend += (sizeof (char *) > + - ((bufferend - (char *) 0) > + % sizeof (char *))) % sizeof (char *); > + > + /* Now the new address. */ > + new_h_addr_list[naddrs++] = > + memcpy (bufferend, tmp_result_buf.h_addr, > + tmp_result_buf.h_length); > + > + /* Also here a final NULL pointer. */ > + new_h_addr_list[naddrs] = NULL; > + > + /* Store the new array pointers. */ > + old_result->h_aliases = new_h_aliases; > + old_result->h_addr_list = new_h_addr_list; > + > + /* Compute the new buffer end. */ > + bufferend = (char *) &new_h_aliases[naliases + 1]; > + assert (bufferend <= buffer + buflen); > + > + result = old_result; > + } > + } > + > + if (status == NSS_STATUS_TRYAGAIN) > + { > + size_t newsize = 2 * tmp_buflen; > + if (tmp_buffer_malloced) > + { > + char *newp = realloc (tmp_buffer, newsize); > + if (newp != NULL) > + { > + assert ((((uintptr_t) newp) > + & (__alignof__ (struct hostent_data) - 1)) > + == 0); > + tmp_buffer = newp; > + tmp_buflen = newsize; > + goto again; > + } > + } > + else if (!__libc_use_alloca (buflen + newsize)) > + { > + tmp_buffer = malloc (newsize); > + if (tmp_buffer != NULL) > + { > + assert ((((uintptr_t) tmp_buffer) > + & (__alignof__ (struct hostent_data) - 1)) > + == 0); > + tmp_buffer_malloced = true; > + tmp_buflen = newsize; > + goto again; > + } > + } > + else > + { > + tmp_buffer > + = extend_alloca (tmp_buffer, tmp_buflen, > + newsize > + + __alignof__ (struct hostent_data)); > + tmp_buffer = (char *) (((uintptr_t) tmp_buffer > + + __alignof__ (struct hostent_data) > + - 1) > + & ~(__alignof__ (struct hostent_data) > + - 1)); > + goto again; > + } > + } > + else > + status = NSS_STATUS_SUCCESS; > + out: > + if (tmp_buffer_malloced) > + free (tmp_buffer); > + return status; > +} > + > enum nss_status > _nss_files_gethostbyname3_r (const char *name, int af, struct hostent *result, > char *buffer, size_t buflen, int *errnop, > @@ -143,199 +343,8 @@ _nss_files_gethostbyname3_r (const char *name, int af, struct hostent *result, > > if (status == NSS_STATUS_SUCCESS > && _res_hconf.flags & HCONF_FLAG_MULTI) > - { > - /* We have to get all host entries from the file. */ > - size_t tmp_buflen = MIN (buflen, 4096); > - char tmp_buffer_stack[tmp_buflen] > - __attribute__ ((__aligned__ (__alignof__ (struct hostent_data)))); > - char *tmp_buffer = tmp_buffer_stack; > - struct hostent tmp_result_buf; > - int naddrs = 1; > - int naliases = 0; > - char *bufferend; > - bool tmp_buffer_malloced = false; > - > - while (result->h_aliases[naliases] != NULL) > - ++naliases; > - > - bufferend = (char *) &result->h_aliases[naliases + 1]; > - > - again: > - while ((status = internal_getent (stream, &tmp_result_buf, tmp_buffer, > - tmp_buflen, errnop, herrnop, af, > - flags)) > - == NSS_STATUS_SUCCESS) > - { > - int matches = 1; > - struct hostent *old_result = result; > - result = &tmp_result_buf; > - /* The following piece is a bit clumsy but we want to use the > - `LOOKUP_NAME_CASE' value. The optimizer should do its > - job. */ > - do > - { > - LOOKUP_NAME_CASE (h_name, h_aliases) > - result = old_result; > - } > - while ((matches = 0)); > - > - if (matches) > - { > - /* We could be very clever and try to recycle a few bytes > - in the buffer instead of generating new arrays. But > - we are not doing this here since it's more work than > - it's worth. Simply let the user provide a bit bigger > - buffer. */ > - char **new_h_addr_list; > - char **new_h_aliases; > - int newaliases = 0; > - size_t newstrlen = 0; > - int cnt; > - > - /* Count the new aliases and the length of the strings. */ > - while (tmp_result_buf.h_aliases[newaliases] != NULL) > - { > - char *cp = tmp_result_buf.h_aliases[newaliases]; > - ++newaliases; > - newstrlen += strlen (cp) + 1; > - } > - /* If the real name is different add it also to the > - aliases. This means that there is a duplication > - in the alias list but this is really the user's > - problem. */ > - if (strcmp (old_result->h_name, > - tmp_result_buf.h_name) != 0) > - { > - ++newaliases; > - newstrlen += strlen (tmp_result_buf.h_name) + 1; > - } > - > - /* Make sure bufferend is aligned. */ > - assert ((bufferend - (char *) 0) % sizeof (char *) == 0); > - > - /* Now we can check whether the buffer is large enough. > - 16 is the maximal size of the IP address. */ > - if (bufferend + 16 + (naddrs + 2) * sizeof (char *) > - + roundup (newstrlen, sizeof (char *)) > - + (naliases + newaliases + 1) * sizeof (char *) > - >= buffer + buflen) > - { > - *errnop = ERANGE; > - *herrnop = NETDB_INTERNAL; > - status = NSS_STATUS_TRYAGAIN; > - goto out; > - } > - > - new_h_addr_list = > - (char **) (bufferend > - + roundup (newstrlen, sizeof (char *)) > - + 16); > - new_h_aliases = > - (char **) ((char *) new_h_addr_list > - + (naddrs + 2) * sizeof (char *)); > - > - /* Copy the old data in the new arrays. */ > - for (cnt = 0; cnt < naddrs; ++cnt) > - new_h_addr_list[cnt] = old_result->h_addr_list[cnt]; > - > - for (cnt = 0; cnt < naliases; ++cnt) > - new_h_aliases[cnt] = old_result->h_aliases[cnt]; > - > - /* Store the new strings. */ > - cnt = 0; > - while (tmp_result_buf.h_aliases[cnt] != NULL) > - { > - new_h_aliases[naliases++] = bufferend; > - bufferend = (__stpcpy (bufferend, > - tmp_result_buf.h_aliases[cnt]) > - + 1); > - ++cnt; > - } > - > - if (cnt < newaliases) > - { > - new_h_aliases[naliases++] = bufferend; > - bufferend = __stpcpy (bufferend, > - tmp_result_buf.h_name) + 1; > - } > - > - /* Final NULL pointer. */ > - new_h_aliases[naliases] = NULL; > - > - /* Round up the buffer end address. */ > - bufferend += (sizeof (char *) > - - ((bufferend - (char *) 0) > - % sizeof (char *))) % sizeof (char *); > - > - /* Now the new address. */ > - new_h_addr_list[naddrs++] = > - memcpy (bufferend, tmp_result_buf.h_addr, > - tmp_result_buf.h_length); > - > - /* Also here a final NULL pointer. */ > - new_h_addr_list[naddrs] = NULL; > - > - /* Store the new array pointers. */ > - old_result->h_aliases = new_h_aliases; > - old_result->h_addr_list = new_h_addr_list; > - > - /* Compute the new buffer end. */ > - bufferend = (char *) &new_h_aliases[naliases + 1]; > - assert (bufferend <= buffer + buflen); > - > - result = old_result; > - } > - } > - > - if (status == NSS_STATUS_TRYAGAIN) > - { > - size_t newsize = 2 * tmp_buflen; > - if (tmp_buffer_malloced) > - { > - char *newp = realloc (tmp_buffer, newsize); > - if (newp != NULL) > - { > - assert ((((uintptr_t) newp) > - & (__alignof__ (struct hostent_data) - 1)) > - == 0); > - tmp_buffer = newp; > - tmp_buflen = newsize; > - goto again; > - } > - } > - else if (!__libc_use_alloca (buflen + newsize)) > - { > - tmp_buffer = malloc (newsize); > - if (tmp_buffer != NULL) > - { > - assert ((((uintptr_t) tmp_buffer) > - & (__alignof__ (struct hostent_data) - 1)) > - == 0); > - tmp_buffer_malloced = true; > - tmp_buflen = newsize; > - goto again; > - } > - } > - else > - { > - tmp_buffer > - = extend_alloca (tmp_buffer, tmp_buflen, > - newsize > - + __alignof__ (struct hostent_data)); > - tmp_buffer = (char *) (((uintptr_t) tmp_buffer > - + __alignof__ (struct hostent_data) > - - 1) > - & ~(__alignof__ (struct hostent_data) > - - 1)); > - goto again; > - } > - } > - else > - status = NSS_STATUS_SUCCESS; > - out: > - if (tmp_buffer_malloced) > - free (tmp_buffer); > - } > + status = gethostbyname3_multi > + (stream, name, af, result, buffer, buflen, errnop, herrnop, flags); > > internal_endent (&stream); > } >