Message ID | CAGQ9bdzqT1EyXYMwACrHpPU=vPjM_b72LJRjb7BW_OzJRXG3bw@mail.gmail.com |
---|---|
State | Committed |
Headers |
Return-Path: <x14307373@homiemail-mx22.g.dreamhost.com> X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx22.g.dreamhost.com (mx2.sub5.homie.mail.dreamhost.com [208.113.200.128]) by wilcox.dreamhost.com (Postfix) with ESMTP id 5611F36007B for <siddhesh@wilcox.dreamhost.com>; Tue, 20 May 2014 05:56:54 -0700 (PDT) Received: by homiemail-mx22.g.dreamhost.com (Postfix, from userid 14307373) id F0F045CF4141; Tue, 20 May 2014 05:56:53 -0700 (PDT) X-Original-To: glibc@patchwork.siddhesh.in Delivered-To: x14307373@homiemail-mx22.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx22.g.dreamhost.com (Postfix) with ESMTPS id C119E5CF4140 for <glibc@patchwork.siddhesh.in>; Tue, 20 May 2014 05:56:53 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:mime-version:from:date:message-id:subject:to :content-type; q=dns; s=default; b=iMlPl8jRDK73ZL/QXDfIu/kuk33sT XHA+fspUelIGYkGokYPZnpsm6EYLjqK5QfWF1SsI/4+vXzU/fwU/yKHSlKPjXcjq TvMtPpN4uBBTuGsgMixwl3m1BqJMZH2EEbwTgtVfbzXdUZRS+O31kemA4Yf4M3EN +OpQZicY7SlRNM= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:mime-version:from:date:message-id:subject:to :content-type; s=default; bh=LPNsIf8sW08gS2L4Vj6BNTZJT08=; b=pwI vGCYHi7vBnfX5a6HSEBbt8q5qSYMEk0x6HaXophn6/aYkkc4wJp5436i1eWwrLR0 yRlCSUUcaMp/n8nKUD1mQ1ELiOSNkJlBQedquPHARI1xvs965flwGM008jSkYVbz 0F/mP7lXnqdOitBhyzBt2Th0gljCtm6h9HLdAckI= Received: (qmail 26855 invoked by alias); 20 May 2014 12:56:51 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-glibc=patchwork.siddhesh.in@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 26843 invoked by uid 89); 20 May 2014 12:56:51 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-vc0-f178.google.com X-Received: by 10.220.175.70 with SMTP id w6mr225373vcz.72.1400590607667; Tue, 20 May 2014 05:56:47 -0700 (PDT) MIME-Version: 1.0 From: Konstantin Serebryany <konstantin.s.serebryany@gmail.com> Date: Tue, 20 May 2014 16:56:27 +0400 Message-ID: <CAGQ9bdzqT1EyXYMwACrHpPU=vPjM_b72LJRjb7BW_OzJRXG3bw@mail.gmail.com> Subject: [PATCH 1/25] Remove nested functions: crypt/md5-crypt.c To: GNU C Library <libc-alpha@sourceware.org> Content-Type: text/plain; charset=UTF-8 X-DH-Original-To: glibc@patchwork.siddhesh.in |
Commit Message
Kostya Serebryany
May 20, 2014, 12:56 p.m. UTC
Hi, This patch is the first in the series of patches that remove nested functions from glibc. Rationale: nested functions is a non-standard language feature; removing nested functions will allow to compile glibc with compilers other than GCC and thus benefit from other compilers and code analysis tools. Discussed previously here: https://sourceware.org/ml/libc-alpha/2014-05/msg00400.html No functionality change intended. (If approved, this will be my first commit to glibc, so some more actions may be required) 2014-05-20 Kostya Serebryany <konstantin.s.serebryany@gmail.com> * crypt/md5-crypt.c (__md5_crypt_r): Remove a nested function. (b64_from_24bit): New function. libcs. */ @@ -268,24 +281,18 @@ __md5_crypt_r (key, salt, buffer, buflen) --buflen; } - void b64_from_24bit (unsigned int b2, unsigned int b1, unsigned int b0, - int n) - { - unsigned int w = (b2 << 16) | (b1 << 8) | b0; - while (n-- > 0 && buflen > 0) - { - *cp++ = b64t[w & 0x3f]; - --buflen; - w >>= 6; - } - } - - b64_from_24bit (alt_result[0], alt_result[6], alt_result[12], 4); - b64_from_24bit (alt_result[1], alt_result[7], alt_result[13], 4); - b64_from_24bit (alt_result[2], alt_result[8], alt_result[14], 4); - b64_from_24bit (alt_result[3], alt_result[9], alt_result[15], 4); - b64_from_24bit (alt_result[4], alt_result[10], alt_result[5], 4); - b64_from_24bit (0, 0, alt_result[11], 2); + b64_from_24bit (&cp, &buflen, + alt_result[0], alt_result[6], alt_result[12], 4); + b64_from_24bit (&cp, &buflen, + alt_result[1], alt_result[7], alt_result[13], 4); + b64_from_24bit (&cp, &buflen, + alt_result[2], alt_result[8], alt_result[14], 4); + b64_from_24bit (&cp, &buflen, + alt_result[3], alt_result[9], alt_result[15], 4); + b64_from_24bit (&cp, &buflen, + alt_result[4], alt_result[10], alt_result[5], 4); + b64_from_24bit (&cp, &buflen, + 0, 0, alt_result[11], 2); if (buflen <= 0) { __set_errno (ERANGE);
Comments
On Tue, May 20, 2014 at 04:56:27PM +0400, Konstantin Serebryany wrote: > Hi, > > This patch is the first in the series of patches that remove nested > functions from glibc. > Rationale: nested functions is a non-standard language feature; > removing nested functions > will allow to compile glibc with compilers other than GCC and thus > benefit from other compilers > and code analysis tools. > Discussed previously here: > https://sourceware.org/ml/libc-alpha/2014-05/msg00400.html > No functionality change intended. > > (If approved, this will be my first commit to glibc, so some more > actions may be required) I believe initially you'll need one of us to commit for you, which we will do once the patch is found suitable. You may want to review the contribution checklist as well: http://sourceware.org/glibc/wiki/Contribution%20checklist > > 2014-05-20 Kostya Serebryany <konstantin.s.serebryany@gmail.com> Two spaces between the date and name, and name and email. An extra newline between the ChangeLog content and header. > * crypt/md5-crypt.c (__md5_crypt_r): Remove a nested function. > (b64_from_24bit): New function. > > > diff --git a/crypt/md5-crypt.c b/crypt/md5-crypt.c > index d1b92d7..29f66ce 100644 > --- a/crypt/md5-crypt.c > +++ b/crypt/md5-crypt.c > @@ -88,6 +88,19 @@ extern char *__md5_crypt_r (const char *key, const > char *salt, > char *buffer, int buflen); > extern char *__md5_crypt (const char *key, const char *salt); > > +static void b64_from_24bit (char **cp, int *buflen, 'static void' should be on a separate line on its own. > + unsigned int b2, unsigned int b1, unsigned int b0, > + int n) > +{ > + unsigned int w = (b2 << 16) | (b1 << 8) | b0; > + while (n-- > 0 && *buflen > 0) > + { > + *(*cp)++ = b64t[w & 0x3f]; > + --*buflen; > + w >>= 6; > + } > +} > + > > /* This entry point is equivalent to the `crypt' function in Unix > libcs. */ > @@ -268,24 +281,18 @@ __md5_crypt_r (key, salt, buffer, buflen) > --buflen; > } > > - void b64_from_24bit (unsigned int b2, unsigned int b1, unsigned int b0, > - int n) > - { > - unsigned int w = (b2 << 16) | (b1 << 8) | b0; > - while (n-- > 0 && buflen > 0) > - { > - *cp++ = b64t[w & 0x3f]; > - --buflen; > - w >>= 6; > - } > - } > - > - b64_from_24bit (alt_result[0], alt_result[6], alt_result[12], 4); > - b64_from_24bit (alt_result[1], alt_result[7], alt_result[13], 4); > - b64_from_24bit (alt_result[2], alt_result[8], alt_result[14], 4); > - b64_from_24bit (alt_result[3], alt_result[9], alt_result[15], 4); > - b64_from_24bit (alt_result[4], alt_result[10], alt_result[5], 4); > - b64_from_24bit (0, 0, alt_result[11], 2); > + b64_from_24bit (&cp, &buflen, > + alt_result[0], alt_result[6], alt_result[12], 4); > + b64_from_24bit (&cp, &buflen, > + alt_result[1], alt_result[7], alt_result[13], 4); > + b64_from_24bit (&cp, &buflen, > + alt_result[2], alt_result[8], alt_result[14], 4); > + b64_from_24bit (&cp, &buflen, > + alt_result[3], alt_result[9], alt_result[15], 4); > + b64_from_24bit (&cp, &buflen, > + alt_result[4], alt_result[10], alt_result[5], 4); > + b64_from_24bit (&cp, &buflen, > + 0, 0, alt_result[11], 2); > if (buflen <= 0) > { > __set_errno (ERANGE); It would be useful if you could post some kind of performance analysis of the crypt or crypt_r to show that making this change does not cause a significant drop in performance. In fact, I'd be really interested in having a microbenchmark added for this function in benchtests. Instructions in benchtests/README should be useful but if not, please feel free to reach out to me for help. Siddhesh
On Tue, 20 May 2014, Konstantin Serebryany wrote: > (If approved, this will be my first commit to glibc, so some more > actions may be required) I don't see you in copyright.list - are you covered by a corporate copyright assignment? Even if individual patches in this series don't have much copyrightable creativity, the series as a whole still may. (Also: anyone working on this should *not* look at Qualcomm's patches for building glibc for Hexagon with Clang, as Qualcomm does not have a copyright assignment.)
On Tue, May 20, 2014 at 6:32 PM, Joseph S. Myers <joseph@codesourcery.com> wrote: > On Tue, 20 May 2014, Konstantin Serebryany wrote: > >> (If approved, this will be my first commit to glibc, so some more >> actions may be required) > > I don't see you in copyright.list - are you covered by a corporate > copyright assignment? Err. Most likely. (My other e-mail is kcc@google.com) > Even if individual patches in this series don't > have much copyrightable creativity, the series as a whole still may. > > (Also: anyone working on this should *not* look at Qualcomm's patches for > building glibc for Hexagon with Clang, as Qualcomm does not have a > copyright assignment.) Never heard of such :) > > -- > Joseph S. Myers > joseph@codesourcery.com
On Tue, 20 May 2014, Konstantin Serebryany wrote: > On Tue, May 20, 2014 at 6:32 PM, Joseph S. Myers > <joseph@codesourcery.com> wrote: > > On Tue, 20 May 2014, Konstantin Serebryany wrote: > > > >> (If approved, this will be my first commit to glibc, so some more > >> actions may be required) > > > > I don't see you in copyright.list - are you covered by a corporate > > copyright assignment? > Err. Most likely. (My other e-mail is kcc@google.com) Yes, there's a corporate assignment from Google for all GNU packages, dated 2007-03-15.
diff --git a/crypt/md5-crypt.c b/crypt/md5-crypt.c index d1b92d7..29f66ce 100644 --- a/crypt/md5-crypt.c +++ b/crypt/md5-crypt.c @@ -88,6 +88,19 @@ extern char *__md5_crypt_r (const char *key, const char *salt, char *buffer, int buflen); extern char *__md5_crypt (const char *key, const char *salt); +static void b64_from_24bit (char **cp, int *buflen, + unsigned int b2, unsigned int b1, unsigned int b0, + int n) +{ + unsigned int w = (b2 << 16) | (b1 << 8) | b0; + while (n-- > 0 && *buflen > 0) + { + *(*cp)++ = b64t[w & 0x3f]; + --*buflen; + w >>= 6; + } +} + /* This entry point is equivalent to the `crypt' function in Unix