From patchwork Thu Dec 24 15:17:01 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 41552 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7541A3898509; Thu, 24 Dec 2020 15:17:19 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7541A3898509 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1608823039; bh=Ht1dTKRROJmZQkqkZu30EBoWAfJB8EQoCdZ1WFhpD4Y=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=ZlMC5AklIdIKGYbHtGCdwy7rWalObMnpK9y8aozvxcMvRdK/fnBn7tTj7bXSlzq8u SNoq5Y9+lnL3sB+8DHjkqn9Ty0AUp27XghypW2pMqT8g0W0TI6GsQPlDO5Ka6B1Ntc Nu699VZJQ6XPArvi9Xdcy2sYMH9t3NTwpqglGOVI= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-qv1-xf29.google.com (mail-qv1-xf29.google.com [IPv6:2607:f8b0:4864:20::f29]) by sourceware.org (Postfix) with ESMTPS id 9351D388703E for ; Thu, 24 Dec 2020 15:17:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 9351D388703E Received: by mail-qv1-xf29.google.com with SMTP id p12so1271156qvj.13 for ; Thu, 24 Dec 2020 07:17:16 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=Ht1dTKRROJmZQkqkZu30EBoWAfJB8EQoCdZ1WFhpD4Y=; b=HDowM0estlVoWViXS2OWCJdGTPwLFIKB7Q6tXTkisC4W7qa4RSOBHFYY1WQQZZizTg 1MYKFGOrNM4+aJl0hWxcVaL7Uyg6gyIPXYrYzzp12PpgiOjgBVUPnPPKuVRAYjLbnGYm Ot800sk+gsijGnzLZh/2etYOL1R9FBG3zW6e5aJvs0OffsGyMyz3PeNIfyJk8IQ4/nv2 Gyu33S592MWcuN+pOmusp5uO2UpzoLi2Z9pq04K033DxAMz4SQbh/UgZSORQRWEOLP2O RGIpCVvT0l25q80wqVIXde3x2MdgLNmvVB1C9kIkbCeQD4SPsktKYeWA0PS9YIrs057N GzVQ== X-Gm-Message-State: AOAM53386Wnfzcj5Xh2bljTEuRpCu1NTe4oCMH9FiC0Rnxg79kyarD5r mt3T6I7Cp6YOPa/LlYEYaVBK1sFCPIudTA== X-Google-Smtp-Source: ABdhPJySPcQX2W0YhTJS+URXUgdyUm15Q1lsuo6z0gq3M9u3Ge0nuj7Ob5Vm1nL5MhW8qdP6CIdHPQ== X-Received: by 2002:ad4:5762:: with SMTP id r2mr32241593qvx.45.1608823035860; Thu, 24 Dec 2020 07:17:15 -0800 (PST) Received: from localhost.localdomain ([177.194.48.209]) by smtp.googlemail.com with ESMTPSA id h21sm17039460qkk.5.2020.12.24.07.17.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 24 Dec 2020 07:17:15 -0800 (PST) To: libc-alpha@sourceware.org, Paul Eggert Subject: [PATCH 5/5] stdlib: Remove lstat usage from realpath [BZ #24970] Date: Thu, 24 Dec 2020 12:17:01 -0300 Message-Id: <20201224151701.1751008-6-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20201224151701.1751008-1-adhemerval.zanella@linaro.org> References: <20201224151701.1751008-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-13.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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-Patchwork-Original-From: Adhemerval Zanella via Libc-alpha From: Adhemerval Zanella Reply-To: Adhemerval Zanella Cc: bug-gnulib@gnu.org Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" The readlink already tells whether the file is a symlink, so there is no need to call lstat to check it. However for '..' it requires an extra readlink check if the previous component can be really accessed, otherwise the next iteration will check a possible valid path and end early. It should performance-wise acceptable and a gain over lstat, afaik symlink should not update any inode information. It also fixes the stdlib/test-canon issued from the gnulib sync. Checked on x86_64-linux-gnu. --- include/scratch_buffer.h | 21 ++++++ stdlib/canonicalize.c | 139 +++++++++++++++++++++++---------------- 2 files changed, 102 insertions(+), 58 deletions(-) diff --git a/include/scratch_buffer.h b/include/scratch_buffer.h index c39da78629..7ae77e5160 100644 --- a/include/scratch_buffer.h +++ b/include/scratch_buffer.h @@ -132,4 +132,25 @@ scratch_buffer_set_array_size (struct scratch_buffer *buffer, (buffer, nelem, size)); } +/* Check if BUFFER is using the internal buffer. */ +static __always_inline bool +scratch_buffer_using_internal (struct scratch_buffer *buffer) +{ + return buffer->data == buffer->__space.__c; +} + +/* Return the internal buffer from BUFFER if it is dynamic allocated, + otherwise returns NULL. Initializes the BUFFER if the internal + dynamic buffer is returned. */ +static __always_inline void * +scratch_buffer_take_buffer (struct scratch_buffer *buffer) +{ + if (scratch_buffer_using_internal (buffer)) + return NULL; + + void *r = buffer->data; + scratch_buffer_init (buffer); + return r; +} + #endif /* _SCRATCH_BUFFER_H */ diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c index 9111d92a4c..78a06227d2 100644 --- a/stdlib/canonicalize.c +++ b/stdlib/canonicalize.c @@ -88,6 +88,21 @@ #if !FUNC_REALPATH_WORKS || defined _LIBC static idx_t +readlink_scratch_buffer (const char *path, struct scratch_buffer *buf) +{ + ssize_t r; + while (true) + { + ptrdiff_t bufsize = buf->length; + r = __readlink (path, buf->data, bufsize - 1); + if (r < bufsize - 1) + break; + if (!scratch_buffer_grow (buf)) + return -1; + } + return r; +} +static idx_t get_path_max (void) { # ifdef PATH_MAX @@ -144,12 +159,10 @@ __realpath (const char *name, char *resolved) struct scratch_buffer extra_buffer, link_buffer; struct scratch_buffer rname_buffer; - struct scratch_buffer *rname_buf = &rname_buffer; scratch_buffer_init (&extra_buffer); scratch_buffer_init (&link_buffer); - scratch_buffer_init (rname_buf); - char *rname_on_stack = rname_buf->data; - char *rname = rname_on_stack; + scratch_buffer_init (&rname_buffer); + char *rname = rname_buffer.data; bool end_in_extra_buffer = false; bool failed = true; @@ -159,16 +172,16 @@ __realpath (const char *name, char *resolved) if (!IS_ABSOLUTE_FILE_NAME (name)) { - while (!__getcwd (rname, rname_buf->length)) + while (!__getcwd (rname, rname_buffer.length)) { if (errno != ERANGE) { dest = rname; goto error; } - if (!scratch_buffer_grow (rname_buf)) + if (!scratch_buffer_grow (&rname_buffer)) goto error_nomem; - rname = rname_buf->data; + rname = rname_buffer.data; } dest = __rawmemchr (rname, '\0'); start = name; @@ -188,7 +201,7 @@ __realpath (const char *name, char *resolved) start = name + prefix_len; } - for ( ; *start; start = end) + for (end = start ; *start; start = end) { /* Skip sequence of multiple file name separators. */ while (ISSLASH (*start)) @@ -206,6 +219,20 @@ __realpath (const char *name, char *resolved) /* nothing */; else if (startlen == 2 && start[0] == '.' && start[1] == '.') { + if (!ISSLASH (dest[-1])) + *dest++ = '/'; + *dest = '\0'; + + ssize_t n = readlink_scratch_buffer (rname, &link_buffer); + if (n < 0) + { + if (errno == ENOTDIR && dest[-1] == '/') + dest[-1] = '\0'; + if (errno == ENOMEM) + goto error_nomem; + if (errno != EINVAL) + goto error; + } /* Back up to previous component, ignore if at root already. */ if (dest > rname + prefix_len + 1) for (--dest; dest > rname && !ISSLASH (dest[-1]); --dest) @@ -220,46 +247,36 @@ __realpath (const char *name, char *resolved) if (!ISSLASH (dest[-1])) *dest++ = '/'; - while (rname + rname_buf->length - dest <= startlen) + while (rname + rname_buffer.length - dest <= startlen) { idx_t dest_offset = dest - rname; - if (!scratch_buffer_grow_preserve (rname_buf)) + if (!scratch_buffer_grow_preserve (&rname_buffer)) goto error_nomem; - rname = rname_buf->data; + rname = rname_buffer.data; dest = rname + dest_offset; } dest = __mempcpy (dest, start, startlen); *dest = '\0'; - /* If STARTLEN == 0, RNAME ends in '/'; use stat rather than - readlink, because readlink might fail with EINVAL without - checking whether RNAME sans '/' is valid. */ - struct stat st; - char *buf = NULL; - ssize_t n; - if (startlen != 0) + ssize_t n = readlink_scratch_buffer (rname, &link_buffer); + if (n < 0) { - while (true) - { - buf = link_buffer.data; - idx_t bufsize = link_buffer.length; - n = __readlink (rname, buf, bufsize - 1); - if (n < bufsize - 1) - break; - if (!scratch_buffer_grow (&link_buffer)) - goto error_nomem; - } - if (n < 0) - buf = NULL; + if (errno == ENOTDIR && dest[-1] == '/') + dest[-1] = '\0'; + if (errno == ENOMEM) + goto error_nomem; + if (errno != EINVAL) + goto error; } - if (buf) + else { if (++num_links > __eloop_threshold ()) { __set_errno (ELOOP); goto error; } + char *buf = (char*) link_buffer.data; buf[n] = '\0'; @@ -279,7 +296,7 @@ __realpath (const char *name, char *resolved) /* Careful here, end may be a pointer into extra_buf... */ memmove (&extra_buf[n], end, len + 1); - name = end = memcpy (extra_buf, buf, n); + name = end = memcpy (extra_buf, link_buffer.data, n); end_in_extra_buffer = true; if (IS_ABSOLUTE_FILE_NAME (buf)) @@ -309,10 +326,6 @@ __realpath (const char *name, char *resolved) dest++; } } - else if (! (startlen == 0 - ? stat (rname, &st) == 0 || errno == EOVERFLOW - : errno == EINVAL)) - goto error; } } if (dest > rname + prefix_len + 1 && ISSLASH (dest[-1])) @@ -320,34 +333,44 @@ __realpath (const char *name, char *resolved) if (DOUBLE_SLASH_IS_DISTINCT_ROOT && dest == rname + 1 && !prefix_len && ISSLASH (*dest) && !ISSLASH (dest[1])) dest++; + *dest = '\0'; failed = false; error: - *dest++ = '\0'; - if (resolved != NULL && dest - rname <= get_path_max ()) - rname = strcpy (resolved, rname); + if (resolved != NULL) + { + if (dest - rname <= get_path_max ()) + rname = strcpy (resolved, rname); + } + else + { + if (rname == resolved) + return rname; + + idx_t rname_size = dest - rname; + if (scratch_buffer_using_internal (&rname_buffer)) + { + rname = malloc (rname_size + 1); + if (rname != NULL) + { + memcpy (rname, rname_buffer.data, rname_size); + rname[rname_size] = '\0'; + } + } + else + { + rname = scratch_buffer_take_buffer (&rname_buffer); + char *result = realloc (rname, rname_size); + if (result != NULL) + rname = result; + } + } error_nomem: - scratch_buffer_free (&extra_buffer); scratch_buffer_free (&link_buffer); - if (failed || rname == resolved) - scratch_buffer_free (rname_buf); - - if (failed) - return NULL; - - if (rname == resolved) - return rname; - idx_t rname_size = dest - rname; - if (rname == rname_on_stack) - { - rname = malloc (rname_size); - if (rname == NULL) - return NULL; - return memcpy (rname, rname_on_stack, rname_size); - } - char *result = realloc (rname, rname_size); - return result != NULL ? result : rname; + scratch_buffer_free (&extra_buffer); + scratch_buffer_free (&rname_buffer); + return failed ? NULL : rname; } libc_hidden_def (__realpath) versioned_symbol (libc, __realpath, realpath, GLIBC_2_3);