From patchwork Sat Nov 17 03:22:12 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: DJ Delorie X-Patchwork-Id: 30174 Received: (qmail 127840 invoked by alias); 17 Nov 2018 03:22:19 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 127830 invoked by uid 89); 17 Nov 2018 03:22:18 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, KAM_SHORT, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=enoent, ENOENT X-HELO: mx1.redhat.com From: DJ Delorie To: Florian Weimer , Joseph Myers Cc: libc-alpha@sourceware.org Subject: Re: swbz #23501 - define stat buffer for FTW_SLN case In-Reply-To: <87o9apuxhi.fsf@oldenburg.str.redhat.com> (message from Florian Weimer on Fri, 16 Nov 2018 11:29:13 +0100) Date: Fri, 16 Nov 2018 22:22:12 -0500 Message-ID: MIME-Version: 1.0 Florian Weimer writes: > “ISO” should be “POSIX”. This comment is very unclear to me. Should > we use memset/bzero to get st into a defined state on error? I don't expect that to ever happen, but I added a memset for it anyway. Joseph Myers writes: > I'd expect a testcase added to the testsuite here, or some explanation of > why the issue is problematic to test for. Added. I first thought "perfect case for test-container's template directory" but it doesn't support symlinks yet... * io/ftw.c (process_entry): Define the stat buf in the case of dangling symlinks, despite FTW_PHYS flag. * io/tst-ftw-lnk.c: New. * io/Makefile: Add it to the test list. diff --git a/io/Makefile b/io/Makefile index ec5c6d7a2f..4bfba97c7d 100644 --- a/io/Makefile +++ b/io/Makefile @@ -72,6 +72,7 @@ tests := test-utime test-stat test-stat2 test-lfs tst-getcwd \ tst-posix_fallocate tst-posix_fallocate64 \ tst-fts tst-fts-lfs tst-open-tmpfile \ tst-copy_file_range tst-getcwd-abspath \ + tst-ftw-lnk # This test includes the compat implementation of copy_file_range, # which uses internal, unexported libc functions. diff --git a/io/ftw.c b/io/ftw.c index f6ecbe7d90..6201436974 100644 --- a/io/ftw.c +++ b/io/ftw.c @@ -424,7 +424,31 @@ process_entry (struct ftw_data *data, struct dir_data *dir, const char *name, else if (data->flags & FTW_PHYS) flag = FTW_NS; else if (d_type == DT_LNK) - flag = FTW_SLN; + { + int err; + + flag = FTW_SLN; + + /* Old code left ST undefined; a clarification at the POSIX + level suggests it should contain information about the + link (ala lstat ()). We do our best to fill in what data + we can. */ + if (dir->streamfd != -1) + err = FXSTATAT (_STAT_VER, dir->streamfd, name, &st, + AT_SYMLINK_NOFOLLOW); + else + err = LXSTAT (_STAT_VER, name, &st); + + if (err < 0) + { + /* Stat failed. We have three choices: don't report + this file, report with invalid data, or report with + zero'd data. Let's prefer obvious over subtle so + zero the data here. This should happen extremely + rarely. */ + memset (&st, 0, sizeof (st)); + } + } else { if (dir->streamfd != -1) diff --git a/io/tst-ftw-lnk.c b/io/tst-ftw-lnk.c new file mode 100644 index 0000000000..2846ad04ff --- /dev/null +++ b/io/tst-ftw-lnk.c @@ -0,0 +1,239 @@ +/* Test for ftw function related to symbolic links. + Copyright (C) 2018 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +#define TSTDIR "tst-ftw-lnk.d" + +static void +un (const char *file) +{ + struct stat st; + /* Does the file exist? */ + if (lstat (file, &st) < 0 + && errno == ENOENT) + return; + + /* If so, try to remove it. */ + if (unlink (file) < 0) + FAIL_EXIT1 ("Unable to unlink %s", file); +} + +static void +debug_cb (const char *which, const char *fpath, + const struct stat *sb, int typeflags) +{ + const char *sb_type = "???"; + const char *ftw_type = "???"; + + /* Coding style here is intentionally "wrong" to increase readability. */ + if (S_ISREG (sb->st_mode)) sb_type = "REG"; + if (S_ISDIR (sb->st_mode)) sb_type = "DIR"; + if (S_ISLNK (sb->st_mode)) sb_type = "LNK"; + + if (typeflags == FTW_F) ftw_type = "F"; + if (typeflags == FTW_D) ftw_type = "D"; + if (typeflags == FTW_DNR) ftw_type = "DNR"; + if (typeflags == FTW_DP) ftw_type = "DP"; + if (typeflags == FTW_NS) ftw_type = "NS"; + if (typeflags == FTW_SL) ftw_type = "SL"; + if (typeflags == FTW_SLN) ftw_type = "SLN"; + + printf ("%s %5d %-3s %-3s %s\n", which, (int)(sb->st_ino % 100000), sb_type, ftw_type, fpath); +} + +int good_cb = 0; +#define EXPECTED_GOOD 12 + +/* See if the stat buffer SB refers to the file AS_FNAME. */ +static void +check_same_stats (const struct stat *sb, const char *as_fname) +{ + struct stat as; + if (lstat (as_fname, &as) < 0) + FAIL_EXIT1 ("unable to stat %s for comparison", as_fname); + + if (as.st_mode == sb->st_mode + && as.st_ino == sb->st_ino + && as.st_size == sb->st_size) + good_cb ++; + else + printf ("statbuf data doesn't match %s\n", as_fname); +} + +static int +callback_phys (const char *fpath, const struct stat *sb, int typeflags, struct FTW *ftwbuf) +{ + debug_cb ("P", fpath, sb, typeflags); + + /* This callback is for when the FTW_PHYS flag is set. The results + should reflect the physical filesystem entry, not what it might + point to. */ + + /* link1-bad is a dangling symlink, but we're reporting on the link + anyway (ala lstat ()). */ + if (strcmp (fpath, "./link1-bad") == 0) + { + if (S_ISLNK (sb->st_mode) && typeflags == FTW_SL) + good_cb ++; + else + printf ("link1-bad had wrong phys stats\n"); + + check_same_stats (sb, "link1-bad"); + } + + /* link2-ok is a regular non-dangling symlink. */ + if (strcmp (fpath, "./link2-ok") == 0) + { + if (S_ISLNK (sb->st_mode) && typeflags == FTW_SL) + good_cb ++; + else + printf ("link2-ok had wrong phys stats\n"); + + check_same_stats (sb, "link2-ok"); + } + + /* This is the file link2-ok points to. */ + if (strcmp (fpath, "./link2-tgt") == 0) + { + if (S_ISREG (sb->st_mode) && typeflags == FTW_F) + good_cb ++; + else + printf ("link2-tgt had wrong phys stats\n"); + + check_same_stats (sb, "link2-tgt"); + } + + return 0; +} + +static int +callback_log (const char *fpath, const struct stat *sb, int typeflags, struct FTW *ftwbuf) +{ + debug_cb ("L", fpath, sb, typeflags); + + /* This callback is for when the FTW_PHYS flags is NOT set. The + results should reflect the logical file, i.e. symlinks should be + followed. */ + + /* We would normally report what link1-bad links to, but link1-bad + is a dangling symlink. This is an exception to FTW_PHYS in that + we report FTW_SLN (dangling symlink) but the stat data is + correctly set to the link itself (ala lstat ()). */ + if (strcmp (fpath, "./link1-bad") == 0) + { + if (S_ISLNK (sb->st_mode) && typeflags == FTW_SLN) + good_cb ++; + else + printf ("link1-bad had wrong logical stats\n"); + + check_same_stats (sb, "link1-bad"); + } + + /* link2-ok points to link2-tgt, so we expect data reflecting + link2-tgt (ala stat ()). */ + if (strcmp (fpath, "./link2-ok") == 0) + { + if (S_ISREG (sb->st_mode) && typeflags == FTW_F) + good_cb ++; + else + printf ("link2-ok had wrong logical stats\n"); + + check_same_stats (sb, "link2-tgt"); + } + + /* This is the file link2-ok points to. */ + if (strcmp (fpath, "./link2-tgt") == 0) + { + if (S_ISREG (sb->st_mode) && typeflags == FTW_F) + good_cb ++; + else + printf ("link2-tgt had wrong logical stats\n"); + + check_same_stats (sb, "link2-tgt"); + } + + return 0; +} + +static int +do_test (void) +{ + struct stat st; + + if (chdir (support_objdir_root) < 0) + FAIL_EXIT1 ("cannot chdir to objdir root"); + + if (chdir ("io") < 0) + FAIL_EXIT1 ("cannot chdir to objdir/io subdir"); + + if (stat (TSTDIR, &st) >= 0) + { + /* Directory does exist, delete any potential conflicts. */ + if (chdir (TSTDIR) < 0) + FAIL_EXIT1 ("cannot chdir to %s\n", TSTDIR); + un ("link1-bad"); + un ("link1-tgt"); + un ("link2-ok"); + un ("link2-tgt"); + } + else + { + /* Directory does not exist, create it. */ + mkdir (TSTDIR, 0777); + if (chdir (TSTDIR) < 0) + FAIL_EXIT1 ("cannot chdir to %s\n", TSTDIR); + } + + /* At this point, we're inside our test directory, and need to + prepare it. */ + + if (symlink ("link1-tgt", "link1-bad") < 0) + FAIL_EXIT1 ("symlink link1-bad failed"); + if (symlink ("link2-tgt", "link2-ok") < 0) + FAIL_EXIT1 ("symlink link2-ok failed"); + if (open ("link2-tgt", O_RDWR|O_CREAT, 0777) < 0) + FAIL_EXIT1 ("create of link2-tgt failed"); + + /* Now we run the tests. */ + + nftw (".", callback_phys, 10, FTW_PHYS); + nftw (".", callback_log, 10, 0); + + /* Did we see the expected number of correct callbacks? */ + + if (good_cb != EXPECTED_GOOD) + { + FAIL_EXIT1 ("Saw %d good callbacks, expected %d\n", + good_cb, EXPECTED_GOOD); + } + + return 0; +} + +#include