From patchwork Mon Feb 17 11:28:32 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 38123 X-Patchwork-Delegate: stli@linux.vnet.ibm.com Received: (qmail 28785 invoked by alias); 17 Feb 2020 11:28:48 -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 28767 invoked by uid 89); 17 Feb 2020 11:28:47 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-19.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=measurement, ino X-HELO: us-smtp-delivery-1.mimecast.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1581938923; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=92oRKWKBc2l0+BSZkyyTQ8ZydkjAdLKxZ5FP2ufBgtA=; b=h3FhHFVvv67yqhicKVAq72Ufap2D2xI4On+hgCZHSZhCJU4V2oDB7uZdIU4pm09uggVXFg DqBrJLJGiEODk28xu4P5bjd4QgyG7FdnTh2ZFlB49XL2LamjpM8jY7WJWJhxxrt0EqlWbu rCi58xmevCLf/ehLh63qk0MROUGzgNk= From: Florian Weimer To: libc-alpha@sourceware.org Subject: [PATCH] Move implementation of into a C file Date: Mon, 17 Feb 2020 12:28:32 +0100 Message-ID: <87tv3pqwpr.fsf@oldenburg2.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com file_change_detection_for_stat partially initialize struct file_change_detection in some cases, when the size member alone determines the outcome of all comparisons. This results in maybe-uninitialized compiler warnings in case of sufficiently aggressive inlining. Once the implementation is moved into a separate C file, this kind of inlining is no longer possible, so the compiler warnings are gone. (Note that I could not reproduce this warning in my tests.) ----- include/file_change_detection.h | 110 +++++++--------------------------- io/Makefile | 2 +- io/Versions | 4 ++ io/file_change_detection.c | 129 ++++++++++++++++++++++++++++++++++++++++ io/tst-file_change_detection.c | 57 +++++++++--------- resolv/res_init.c | 2 +- resolv/resolv_conf.c | 6 +- 7 files changed, 185 insertions(+), 125 deletions(-) diff --git a/include/file_change_detection.h b/include/file_change_detection.h index aaed0a9b6d..f5220745a9 100644 --- a/include/file_change_detection.h +++ b/include/file_change_detection.h @@ -16,9 +16,10 @@ License along with the GNU C Library; if not, see . */ -#include +#ifndef _FILE_CHANGE_DETECTION_H +#define _FILE_CHANGE_DETECTION_H + #include -#include #include #include #include @@ -38,103 +39,32 @@ struct file_change_detection /* Returns true if *LEFT and *RIGHT describe the same version of the same file. */ -static bool __attribute__ ((unused)) -file_is_unchanged (const struct file_change_detection *left, - const struct file_change_detection *right) -{ - if (left->size < 0 || right->size < 0) - /* Negative sizes are used as markers and never match. */ - return false; - else if (left->size == 0 && right->size == 0) - /* Both files are empty or do not exist, so they have the same - content, no matter what the other fields indicate. */ - return true; - else - return left->size == right->size - && left->ino == right->ino - && left->mtime.tv_sec == right->mtime.tv_sec - && left->mtime.tv_nsec == right->mtime.tv_nsec - && left->ctime.tv_sec == right->ctime.tv_sec - && left->ctime.tv_nsec == right->ctime.tv_nsec; -} +bool __file_is_unchanged (const struct file_change_detection *left, + const struct file_change_detection *right); /* Extract file change information to *FILE from the stat buffer *ST. */ -static void __attribute__ ((unused)) -file_change_detection_for_stat (struct file_change_detection *file, - const struct stat64 *st) -{ - if (S_ISDIR (st->st_mode)) - /* Treat as empty file. */ - file->size = 0; - else if (!S_ISREG (st->st_mode)) - /* Non-regular files cannot be cached. */ - file->size = -1; - else - { - file->size = st->st_size; - file->ino = st->st_ino; - file->mtime = st->st_mtim; - file->ctime = st->st_ctim; - } -} +void __file_change_detection_for_stat (struct file_change_detection *file, + const struct stat64 *st); /* Writes file change information for PATH to *FILE. Returns true on success. For benign errors, *FILE is cleared, and true is returned. For errors indicating resource outages and the like, false is returned. */ -static bool __attribute__ ((unused)) -file_change_detection_for_path (struct file_change_detection *file, - const char *path) -{ - struct stat64 st; - if (stat64 (path, &st) != 0) - switch (errno) - { - case EACCES: - case EISDIR: - case ELOOP: - case ENOENT: - case ENOTDIR: - case EPERM: - /* Ignore errors due to file system contents. Instead, treat - the file as empty. */ - file->size = 0; - return true; - default: - /* Other errors are fatal. */ - return false; - } - else /* stat64 was successfull. */ - { - file_change_detection_for_stat (file, &st); - return true; - } -} +bool __file_change_detection_for_path (struct file_change_detection *file, + const char *path); /* Writes file change information for the stream FP to *FILE. Returns ture on success, false on failure. If FP is NULL, treat the file as non-existing. */ -static bool __attribute__ ((unused)) -file_change_detection_for_fp (struct file_change_detection *file, - FILE *fp) -{ - if (fp == NULL) - { - /* The file does not exist. */ - file->size = 0; - return true; - } - else - { - struct stat64 st; - if (fstat64 (__fileno (fp), &st) != 0) - /* If we already have a file descriptor, all errors are fatal. */ - return false; - else - { - file_change_detection_for_stat (file, &st); - return true; - } - } -} +bool __file_change_detection_for_fp (struct file_change_detection *file, + FILE *fp); + +#ifndef _ISOMAC +libc_hidden_proto (__file_is_unchanged) +libc_hidden_proto (__file_change_detection_for_stat) +libc_hidden_proto (__file_change_detection_for_path) +libc_hidden_proto (__file_change_detection_for_fp) +#endif + +#endif /* _FILE_CHANGE_DETECTION_H */ diff --git a/io/Makefile b/io/Makefile index 04c4647dc0..cf380f3516 100644 --- a/io/Makefile +++ b/io/Makefile @@ -55,7 +55,7 @@ routines := \ posix_fadvise posix_fadvise64 \ posix_fallocate posix_fallocate64 \ sendfile sendfile64 copy_file_range \ - utimensat futimens + utimensat futimens file_change_detection # These routines will be omitted from the libc shared object. # Instead the static object files will be included in a special archive diff --git a/io/Versions b/io/Versions index f7e5dbe49e..ee468055ff 100644 --- a/io/Versions +++ b/io/Versions @@ -137,5 +137,9 @@ libc { __fcntl_nocancel; __open64_nocancel; __write_nocancel; + __file_is_unchanged; + __file_change_detection_for_stat; + __file_change_detection_for_path; + __file_change_detection_for_fp; } } diff --git a/io/file_change_detection.c b/io/file_change_detection.c new file mode 100644 index 0000000000..1ac869c3ec --- /dev/null +++ b/io/file_change_detection.c @@ -0,0 +1,129 @@ +/* Detecting file changes using modification times. + Copyright (C) 2017-2020 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 + +/* Returns true if *LEFT and *RIGHT describe the same version of the + same file. */ +bool +__file_is_unchanged (const struct file_change_detection *left, + const struct file_change_detection *right) +{ + if (left->size < 0 || right->size < 0) + /* Negative sizes are used as markers and never match. */ + return false; + else if (left->size == 0 && right->size == 0) + /* Both files are empty or do not exist, so they have the same + content, no matter what the other fields indicate. */ + return true; + else + return left->size == right->size + && left->ino == right->ino + && left->mtime.tv_sec == right->mtime.tv_sec + && left->mtime.tv_nsec == right->mtime.tv_nsec + && left->ctime.tv_sec == right->ctime.tv_sec + && left->ctime.tv_nsec == right->ctime.tv_nsec; +} +libc_hidden_def (__file_is_unchanged) + +/* Extract file change information to *FILE from the stat buffer + *ST. */ +void +__file_change_detection_for_stat (struct file_change_detection *file, + const struct stat64 *st) +{ + if (S_ISDIR (st->st_mode)) + /* Treat as empty file. */ + file->size = 0; + else if (!S_ISREG (st->st_mode)) + /* Non-regular files cannot be cached. */ + file->size = -1; + else + { + file->size = st->st_size; + file->ino = st->st_ino; + file->mtime = st->st_mtim; + file->ctime = st->st_ctim; + } +} +libc_hidden_def (__file_change_detection_for_stat) + +/* Writes file change information for PATH to *FILE. Returns true on + success. For benign errors, *FILE is cleared, and true is + returned. For errors indicating resource outages and the like, + false is returned. */ +bool +__file_change_detection_for_path (struct file_change_detection *file, + const char *path) +{ + struct stat64 st; + if (stat64 (path, &st) != 0) + switch (errno) + { + case EACCES: + case EISDIR: + case ELOOP: + case ENOENT: + case ENOTDIR: + case EPERM: + /* Ignore errors due to file system contents. Instead, treat + the file as empty. */ + file->size = 0; + return true; + default: + /* Other errors are fatal. */ + return false; + } + else /* stat64 was successfull. */ + { + __file_change_detection_for_stat (file, &st); + return true; + } +} +libc_hidden_def (__file_change_detection_for_path) + +/* Writes file change information for the stream FP to *FILE. Returns + ture on success, false on failure. If FP is NULL, treat the file + as non-existing. */ +bool +__file_change_detection_for_fp (struct file_change_detection *file, + FILE *fp) +{ + if (fp == NULL) + { + /* The file does not exist. */ + file->size = 0; + return true; + } + else + { + struct stat64 st; + if (fstat64 (__fileno (fp), &st) != 0) + /* If we already have a file descriptor, all errors are fatal. */ + return false; + else + { + __file_change_detection_for_stat (file, &st); + return true; + } + } +} +libc_hidden_def (__file_change_detection_for_fp) diff --git a/io/tst-file_change_detection.c b/io/tst-file_change_detection.c index 035dd39c4d..6e00e787b1 100644 --- a/io/tst-file_change_detection.c +++ b/io/tst-file_change_detection.c @@ -16,10 +16,6 @@ License along with the GNU C Library; if not, see . */ -/* The header uses the internal __fileno symbol, which is not - available outside of libc (even to internal tests). */ -#define __fileno(fp) fileno (fp) - #include #include @@ -40,7 +36,7 @@ all_same (struct file_change_detection *array, size_t length) { if (test_verbose > 0) printf ("info: comparing %zu and %zu\n", i, j); - TEST_VERIFY (file_is_unchanged (array + i, array + j)); + TEST_VERIFY (__file_is_unchanged (array + i, array + j)); } } @@ -54,7 +50,7 @@ all_different (struct file_change_detection *array, size_t length) continue; if (test_verbose > 0) printf ("info: comparing %zu and %zu\n", i, j); - TEST_VERIFY (!file_is_unchanged (array + i, array + j)); + TEST_VERIFY (!__file_is_unchanged (array + i, array + j)); } } @@ -105,24 +101,24 @@ do_test (void) struct file_change_detection fcd[10]; int i = 0; /* Two empty files always have the same contents. */ - TEST_VERIFY (file_change_detection_for_path (&fcd[i++], path_empty1)); - TEST_VERIFY (file_change_detection_for_path (&fcd[i++], path_empty2)); + TEST_VERIFY (__file_change_detection_for_path (&fcd[i++], path_empty1)); + TEST_VERIFY (__file_change_detection_for_path (&fcd[i++], path_empty2)); /* So does a missing file (which is treated as empty). */ - TEST_VERIFY (file_change_detection_for_path (&fcd[i++], - path_does_not_exist)); + TEST_VERIFY (__file_change_detection_for_path (&fcd[i++], + path_does_not_exist)); /* And a symbolic link loop. */ - TEST_VERIFY (file_change_detection_for_path (&fcd[i++], path_loop)); + TEST_VERIFY (__file_change_detection_for_path (&fcd[i++], path_loop)); /* And a dangling symbolic link. */ - TEST_VERIFY (file_change_detection_for_path (&fcd[i++], path_dangling)); + TEST_VERIFY (__file_change_detection_for_path (&fcd[i++], path_dangling)); /* And a directory. */ - TEST_VERIFY (file_change_detection_for_path (&fcd[i++], tempdir)); + TEST_VERIFY (__file_change_detection_for_path (&fcd[i++], tempdir)); /* And a symbolic link to an empty file. */ - TEST_VERIFY (file_change_detection_for_path (&fcd[i++], path_to_empty1)); + TEST_VERIFY (__file_change_detection_for_path (&fcd[i++], path_to_empty1)); /* Likewise for access the file via a FILE *. */ - TEST_VERIFY (file_change_detection_for_fp (&fcd[i++], fp_empty1)); - TEST_VERIFY (file_change_detection_for_fp (&fcd[i++], fp_empty2)); + TEST_VERIFY (__file_change_detection_for_fp (&fcd[i++], fp_empty1)); + TEST_VERIFY (__file_change_detection_for_fp (&fcd[i++], fp_empty2)); /* And a NULL FILE * (missing file). */ - TEST_VERIFY (file_change_detection_for_fp (&fcd[i++], NULL)); + TEST_VERIFY (__file_change_detection_for_fp (&fcd[i++], NULL)); TEST_COMPARE (i, array_length (fcd)); all_same (fcd, array_length (fcd)); @@ -132,9 +128,9 @@ do_test (void) { struct file_change_detection fcd[3]; int i = 0; - TEST_VERIFY (file_change_detection_for_path (&fcd[i++], path_file1)); - TEST_VERIFY (file_change_detection_for_path (&fcd[i++], path_to_file1)); - TEST_VERIFY (file_change_detection_for_fp (&fcd[i++], fp_file1)); + TEST_VERIFY (__file_change_detection_for_path (&fcd[i++], path_file1)); + TEST_VERIFY (__file_change_detection_for_path (&fcd[i++], path_to_file1)); + TEST_VERIFY (__file_change_detection_for_fp (&fcd[i++], fp_file1)); TEST_COMPARE (i, array_length (fcd)); all_same (fcd, array_length (fcd)); } @@ -144,20 +140,20 @@ do_test (void) struct file_change_detection fcd[5]; int i = 0; /* The other files are not empty. */ - TEST_VERIFY (file_change_detection_for_path (&fcd[i++], path_empty1)); + TEST_VERIFY (__file_change_detection_for_path (&fcd[i++], path_empty1)); /* These two files have the same contents, but have different file identity. */ - TEST_VERIFY (file_change_detection_for_path (&fcd[i++], path_file1)); - TEST_VERIFY (file_change_detection_for_path (&fcd[i++], path_file2)); + TEST_VERIFY (__file_change_detection_for_path (&fcd[i++], path_file1)); + TEST_VERIFY (__file_change_detection_for_path (&fcd[i++], path_file2)); /* FIFOs are always different, even with themselves. */ - TEST_VERIFY (file_change_detection_for_path (&fcd[i++], path_fifo)); - TEST_VERIFY (file_change_detection_for_path (&fcd[i++], path_fifo)); + TEST_VERIFY (__file_change_detection_for_path (&fcd[i++], path_fifo)); + TEST_VERIFY (__file_change_detection_for_path (&fcd[i++], path_fifo)); TEST_COMPARE (i, array_length (fcd)); all_different (fcd, array_length (fcd)); /* Replacing the file with its symbolic link does not make a difference. */ - TEST_VERIFY (file_change_detection_for_path (&fcd[1], path_to_file1)); + TEST_VERIFY (__file_change_detection_for_path (&fcd[1], path_to_file1)); all_different (fcd, array_length (fcd)); } @@ -166,16 +162,17 @@ do_test (void) for (int use_stdio = 0; use_stdio < 2; ++use_stdio) { struct file_change_detection initial; - TEST_VERIFY (file_change_detection_for_path (&initial, path_file1)); + TEST_VERIFY (__file_change_detection_for_path (&initial, path_file1)); while (true) { support_write_file_string (path_file1, "line\n"); struct file_change_detection current; if (use_stdio) - TEST_VERIFY (file_change_detection_for_fp (¤t, fp_file1)); + TEST_VERIFY (__file_change_detection_for_fp (¤t, fp_file1)); else - TEST_VERIFY (file_change_detection_for_path (¤t, path_file1)); - if (!file_is_unchanged (&initial, ¤t)) + TEST_VERIFY (__file_change_detection_for_path + (¤t, path_file1)); + if (!__file_is_unchanged (&initial, ¤t)) break; /* Wait for a bit to reduce system load. */ usleep (100 * 1000); diff --git a/resolv/res_init.c b/resolv/res_init.c index 98d84f264d..ee5dfdd391 100644 --- a/resolv/res_init.c +++ b/resolv/res_init.c @@ -583,7 +583,7 @@ __resolv_conf_load (struct __res_state *preinit, if (ok && change != NULL) /* Update the file change information if the configuration was loaded successfully. */ - ok = file_change_detection_for_fp (change, fp); + ok = __file_change_detection_for_fp (change, fp); if (ok) { diff --git a/resolv/resolv_conf.c b/resolv/resolv_conf.c index 29a1f4fb94..286149ffad 100644 --- a/resolv/resolv_conf.c +++ b/resolv/resolv_conf.c @@ -121,7 +121,7 @@ struct resolv_conf * __resolv_conf_get_current (void) { struct file_change_detection initial; - if (!file_change_detection_for_path (&initial, _PATH_RESCONF)) + if (!__file_change_detection_for_path (&initial, _PATH_RESCONF)) return NULL; struct resolv_conf_global *global_copy = get_locked_global (); @@ -129,7 +129,7 @@ __resolv_conf_get_current (void) return NULL; struct resolv_conf *conf; if (global_copy->conf_current != NULL - && file_is_unchanged (&initial, &global_copy->file_resolve_conf)) + && __file_is_unchanged (&initial, &global_copy->file_resolve_conf)) /* We can reuse the cached configuration object. */ conf = global_copy->conf_current; else @@ -149,7 +149,7 @@ __resolv_conf_get_current (void) /etc/resolv.conf is temporarily replaced while the file is read (after the initial measurement), and restored to the initial version later. */ - if (file_is_unchanged (&initial, &after_load)) + if (__file_is_unchanged (&initial, &after_load)) global_copy->file_resolve_conf = after_load; else /* If there is a discrepancy, trigger a reload during the