public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Zack Weinberg <zackw@panix.com>
To: Paul Eggert <eggert@cs.ucla.edu>
Cc: Joseph Myers <joseph@codesourcery.com>,
	 Adhemerval Zanella <adhemerval.zanella@linaro.org>,
	Andreas Schwab <schwab@linux-m68k.org>,
	 GNU C Library <libc-alpha@sourceware.org>
Subject: Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
Date: Mon, 11 Sep 2017 17:56:00 -0000	[thread overview]
Message-ID: <CAKCAbMh5Xgcfqf3=EzYe+R+dcnF9EiQyztRt62Rk68ctzef8CA@mail.gmail.com> (raw)
In-Reply-To: <7bf58b8a-afc9-b934-9a4a-1616e71c03ff@cs.ucla.edu>

On Mon, Sep 11, 2017 at 1:38 PM, Paul Eggert <eggert@cs.ucla.edu> wrote:
> On 09/11/2017 10:25 AM, Zack Weinberg wrote:
>>
>>
>> It should be enough to make a dummy call, e.g.
>>
>>   pglob->gl_lstat(".", &statbuf);
>>
>
> Unfortunately calling lstat is quite expensive on some (non-POSIX)
> platforms, even on the working directory. So we can't do the above in the
> Gnulib version.

I have trouble believing this will be a measurable performance hit,
considering how much other expensive work glob has to do.

> Besides, under the proposed patch glob is going to use
> gl_lstat instead of gl_stat in almost all cases, so the dummy call won't add
> much extra checking.

The point is not to add _extra_ checking; the point is to ensure that
gl_lstat (and gl_stat) are valid on all calls, _even if_ they wouldn't
otherwise have been used.  I'm trying to turn "may fail at runtime
under rare circumstances" into "will definitely fail at runtime on the
first use", which is the best we can do in C.

> I suppose we could valid gl_stat instead, as gl_stat usage will become rare
> (used only if GLOB_MARK is also specified, just before returning results).
> But we don't have any code in the wild that is giving us invalid gl_stat
> pointers, so it wouldn't be that helpful to try to validate gl_stat either.

So here's an alternative, less thorough but perhaps also less costly
approach: when GLOB_ALTDIRFUNCS is set, call both gl_stat and gl_lstat
on the first name that's going to be returned, even if we have no
other reason to do this.  Optionally, memoize the function pointer and
don't bother making the extra call again if we recognize that it's
known to work.

(Maybe also it would be a good idea to check up front for any NULL
callbacks in the ALTDIRFUNCS case.)

zw

  reply	other threads:[~2017-09-11 17:56 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-05 20:25 [PATCH 0/9] posix: glob fixes and refactor Adhemerval Zanella
2017-09-05 20:25 ` [PATCH 7/9] posix: Consolidate glob implementation Adhemerval Zanella
2017-09-12  7:35   ` Andreas Schwab
2017-09-12 14:08     ` Adhemerval Zanella
2017-09-12 14:17       ` Andreas Schwab
2017-09-12 14:29     ` Joseph Myers
2017-09-12 14:39       ` Andreas Schwab
2017-09-12 14:50         ` Joseph Myers
2017-09-12 12:56   ` Andreas Schwab
2017-09-12 14:22     ` Adhemerval Zanella
2017-09-12 14:34       ` Andreas Schwab
2017-09-13 12:26         ` Adhemerval Zanella
2017-09-05 20:25 ` [PATCH 2/9] posix: accept inode 0 is a valid inode number (BZ #19971) Adhemerval Zanella
2017-09-05 20:25 ` [PATCH 4/9] Sync scratch_buffer with gnulib Adhemerval Zanella
2017-09-18  6:09   ` Florian Weimer
2017-09-18 11:43     ` Adhemerval Zanella
2017-09-18 11:57       ` Florian Weimer
2017-09-18 12:25         ` Adhemerval Zanella
2017-09-05 20:25 ` [PATCH 1/9] posix: Sync glob with gnulib [BZ #1062] Adhemerval Zanella
2017-09-06  2:01   ` Paul Eggert
2017-09-06 12:52     ` Adhemerval Zanella
2017-09-12 14:20   ` Andreas Schwab
2017-09-12 17:06     ` Adhemerval Zanella
2017-09-05 20:25 ` [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866] Adhemerval Zanella
2017-09-06  1:27   ` Paul Eggert
2017-09-06 12:57     ` Adhemerval Zanella
2017-09-09  9:50   ` Andreas Schwab
2017-09-09 11:56     ` Adhemerval Zanella
2017-09-09 17:02       ` Paul Eggert
2017-09-09 17:11         ` Zack Weinberg
2017-09-09 17:26           ` Andreas Schwab
2017-09-09 17:33             ` Zack Weinberg
2017-09-10  8:19         ` Adhemerval Zanella
2017-09-10 17:13           ` Paul Eggert
2017-09-11 14:34           ` Joseph Myers
2017-09-11 14:38             ` Zack Weinberg
2017-09-11 16:53               ` Paul Eggert
2017-09-11 17:25                 ` Zack Weinberg
2017-09-11 17:38                   ` Paul Eggert
2017-09-11 17:56                     ` Zack Weinberg [this message]
2017-09-11 18:03                       ` Paul Eggert
2017-09-11 20:09                         ` Adhemerval Zanella
2017-09-13  9:14                           ` Paul Eggert
2017-09-13 12:22                             ` Adhemerval Zanella
2017-09-14 10:05                               ` Szabolcs Nagy
2017-09-14 13:43                                 ` Adhemerval Zanella
2017-09-15 20:18                             ` Florian Weimer
2017-09-15 20:27                               ` Adhemerval Zanella
2017-09-17  7:16                               ` Paul Eggert
2017-09-17  7:48                                 ` Florian Weimer
2017-09-17 14:18                                   ` Adhemerval Zanella
2017-09-05 20:25 ` [PATCH 6/9] posix: fix glob bugs with long login names Adhemerval Zanella
2017-09-05 20:25 ` [PATCH 5/9] posix: Fix getpwnam_r usage (BZ #1062) Adhemerval Zanella
2017-09-05 20:25 ` [PATCH 8/9] posix: Use enum for __glob_pattern_type result Adhemerval Zanella
2017-09-06  1:30   ` Paul Eggert
2017-09-06  4:18   ` Paul Eggert
2017-09-06 13:04     ` Adhemerval Zanella
2017-09-06 16:18       ` Paul Eggert
2017-09-06 16:54         ` Adhemerval Zanella
2017-09-05 20:25 ` [PATCH 9/9] posix: Fix glob with GLOB_NOCHECK returning modified patterns (BZ#10246) Adhemerval Zanella
2017-09-07 22:14   ` Paul Eggert
2017-09-08  9:16     ` Adhemerval Zanella

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKCAbMh5Xgcfqf3=EzYe+R+dcnF9EiQyztRt62Rk68ctzef8CA@mail.gmail.com' \
    --to=zackw@panix.com \
    --cc=adhemerval.zanella@linaro.org \
    --cc=eggert@cs.ucla.edu \
    --cc=joseph@codesourcery.com \
    --cc=libc-alpha@sourceware.org \
    --cc=schwab@linux-m68k.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).