From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS22989 209.51.188.0/24 X-Spam-Status: No, score=-4.1 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED, SPF_HELO_NONE,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 9F4D91F461 for ; Sun, 7 Jul 2019 02:28:39 +0000 (UTC) Received: from localhost ([::1]:33654 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hjwur-0005ig-6U for normalperson@yhbt.net; Sat, 06 Jul 2019 22:28:37 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:43007) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hjwui-0005i9-DH for bug-gnulib@gnu.org; Sat, 06 Jul 2019 22:28:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hjwug-0004aG-Vz for bug-gnulib@gnu.org; Sat, 06 Jul 2019 22:28:28 -0400 Received: from zimbra.cs.ucla.edu ([131.179.128.68]:51774) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hjwug-0004Zh-Ku for bug-gnulib@gnu.org; Sat, 06 Jul 2019 22:28:26 -0400 Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id AF3FA161DBD; Sat, 6 Jul 2019 19:28:24 -0700 (PDT) Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id O7NxcDFuTj7V; Sat, 6 Jul 2019 19:28:23 -0700 (PDT) Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id A3EFB161DE9; Sat, 6 Jul 2019 19:28:23 -0700 (PDT) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id 9P1SSFxp1BFi; Sat, 6 Jul 2019 19:28:23 -0700 (PDT) Received: from [192.168.1.9] (cpe-23-242-74-103.socal.res.rr.com [23.242.74.103]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 643C9161DBD; Sat, 6 Jul 2019 19:28:23 -0700 (PDT) Subject: Re: [PATCH] areadlink-with-size: guess a buffer size with 0 size To: =?UTF-8?Q?P=c3=a1draig_Brady?= References: <7AC7AA2A-EDEA-4A80-9A5A-02FAA2D823EF@whamcloud.com> <5a1b87fe-e265-6b0f-66eb-20ad1ba9c6e5@draigBrady.com> <5f23c469-89b6-1280-a8a4-a9b0653f186a@cs.ucla.edu> <360612c8-356b-f139-507f-2282c68fae3f@draigBrady.com> From: Paul Eggert Organization: UCLA Computer Science Department Message-ID: <92271515-1daf-eb47-fa3b-65acbcd59de1@cs.ucla.edu> Date: Sat, 6 Jul 2019 19:28:23 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 MIME-Version: 1.0 In-Reply-To: <360612c8-356b-f139-507f-2282c68fae3f@draigBrady.com> Content-Type: multipart/mixed; boundary="------------DD028CE4F11B1E320E83CDB6" Content-Language: en-US X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 131.179.128.68 X-BeenThere: bug-gnulib@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Gnulib discussion list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Gnulib bugs , Andreas Dilger Errors-To: bug-gnulib-bounces+normalperson=yhbt.net@gnu.org Sender: "bug-gnulib" This is a multi-part message in MIME format. --------------DD028CE4F11B1E320E83CDB6 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable P=C3=A1draig Brady wrote: > Your patch has the advantage of allocating the exact right sized buffer > in the usual case, but the disadvantage of CPU overhead in string lengt= h determination, > and some extra code complexity in the separate small buffer handling. I think the code complexity is worth it, to avoid calling realloc in the = typical=20 case when the size is not known. The string length determination can easi= ly be=20 avoided, so I installed the attached which does that. --------------DD028CE4F11B1E320E83CDB6 Content-Type: text/x-patch; name="0001-areadlink-with-size-avoid-realloc-when-size-0.patch" Content-Disposition: attachment; filename="0001-areadlink-with-size-avoid-realloc-when-size-0.patch" Content-Transfer-Encoding: quoted-printable >From b979980a653ce610fe5f64935adedda425da3ca4 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 6 Jul 2019 19:25:24 -0700 Subject: [PATCH] areadlink-with-size: avoid realloc when size=3D=3D0 * lib/areadlink-with-size.c (areadlink_with_size): * lib/areadlinkat-with-size.c (areadlinkat_with_size): Reallocate at the end to the actual size, to avoid memory waste, as suggested by Bruno Haible. But when the guessed size is zero - useful when the size is unknown - do the initial small readlink into the stack, to avoid that realloc in the usual case. --- ChangeLog | 10 ++++++++++ lib/areadlink-with-size.c | 31 +++++++++++++++++++++++-------- lib/areadlinkat-with-size.c | 31 +++++++++++++++++++++++-------- 3 files changed, 56 insertions(+), 16 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0b061317c..f7e031d9b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2019-07-06 Paul Eggert + + areadlink-with-size: avoid realloc when size=3D=3D0 + * lib/areadlink-with-size.c (areadlink_with_size): + * lib/areadlinkat-with-size.c (areadlinkat_with_size): + Reallocate at the end to the actual size, to avoid memory waste, + as suggested by Bruno Haible. But when the guessed size is zero - + useful when the size is unknown - do the initial small readlink + into the stack, to avoid that realloc in the usual case. + 2019-07-06 P=C3=A1draig Brady =20 areadlink-with-size: guess a buffer size with 0 size diff --git a/lib/areadlink-with-size.c b/lib/areadlink-with-size.c index b9cd05cba..ae3c782a4 100644 --- a/lib/areadlink-with-size.c +++ b/lib/areadlink-with-size.c @@ -60,19 +60,28 @@ areadlink_with_size (char const *file, size_t size) ? symlink_max + 1 : INITIAL_LIMIT_BOUND); =20 + enum { stackbuf_size =3D 128 }; + /* The initial buffer size for the link value. */ - size_t buf_size =3D (size =3D=3D 0 ? 128 + size_t buf_size =3D (size =3D=3D 0 ? stackbuf_size : size < initial_limit ? size + 1 : initial_limit); =20 while (1) { ssize_t r; size_t link_length; - char *buffer =3D malloc (buf_size); + char stackbuf[stackbuf_size]; + char *buf =3D stackbuf; + char *buffer =3D NULL; + + if (! (size =3D=3D 0 && buf_size =3D=3D stackbuf_size)) + { + buf =3D buffer =3D malloc (buf_size); + if (!buffer) + return NULL; + } =20 - if (buffer =3D=3D NULL) - return NULL; - r =3D readlink (file, buffer, buf_size); + r =3D readlink (file, buf, buf_size); link_length =3D r; =20 /* On AIX 5L v5.3 and HP-UX 11i v2 04/09, readlink returns -1 @@ -87,10 +96,16 @@ areadlink_with_size (char const *file, size_t size) =20 if (link_length < buf_size) { - buffer[link_length] =3D 0; - /* Shrink BUFFER before returning it. */ - if (link_length + 1 < buf_size) + buf[link_length] =3D 0; + if (!buffer) + { + buffer =3D malloc (link_length + 1); + if (buffer) + return memcpy (buffer, buf, link_length + 1); + } + else if (link_length + 1 < buf_size) { + /* Shrink BUFFER before returning it. */ char *shrinked_buffer =3D realloc (buffer, link_length + 1= ); if (shrinked_buffer !=3D NULL) buffer =3D shrinked_buffer; diff --git a/lib/areadlinkat-with-size.c b/lib/areadlinkat-with-size.c index d39096f2b..ed46d5911 100644 --- a/lib/areadlinkat-with-size.c +++ b/lib/areadlinkat-with-size.c @@ -65,19 +65,28 @@ areadlinkat_with_size (int fd, char const *file, size= _t size) ? symlink_max + 1 : INITIAL_LIMIT_BOUND); =20 + enum { stackbuf_size =3D 128 }; + /* The initial buffer size for the link value. */ - size_t buf_size =3D (size =3D=3D 0 ? 128 + size_t buf_size =3D (size =3D=3D 0 ? stackbuf_size : size < initial_limit ? size + 1 : initial_limit); =20 while (1) { ssize_t r; size_t link_length; - char *buffer =3D malloc (buf_size); + char stackbuf[stackbuf_size]; + char *buf =3D stackbuf; + char *buffer =3D NULL; + + if (! (size =3D=3D 0 && buf_size =3D=3D stackbuf_size)) + { + buf =3D buffer =3D malloc (buf_size); + if (!buffer) + return NULL; + } =20 - if (buffer =3D=3D NULL) - return NULL; - r =3D readlinkat (fd, file, buffer, buf_size); + r =3D readlinkat (fd, file, buf, buf_size); link_length =3D r; =20 /* On AIX 5L v5.3 and HP-UX 11i v2 04/09, readlink returns -1 @@ -92,10 +101,16 @@ areadlinkat_with_size (int fd, char const *file, siz= e_t size) =20 if (link_length < buf_size) { - buffer[link_length] =3D 0; - /* Shrink BUFFER before returning it. */ - if (link_length + 1 < buf_size) + buf[link_length] =3D 0; + if (!buffer) + { + buffer =3D malloc (link_length + 1); + if (buffer) + return memcpy (buffer, buf, link_length + 1); + } + else if (link_length + 1 < buf_size) { + /* Shrink BUFFER before returning it. */ char *shrinked_buffer =3D realloc (buffer, link_length + 1= ); if (shrinked_buffer !=3D NULL) buffer =3D shrinked_buffer; --=20 2.17.1 --------------DD028CE4F11B1E320E83CDB6--