[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified
From: |
Bernhard Voelker |
Subject: |
bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified |
Date: |
Wed, 28 Jan 2015 09:17:55 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 |
On 01/27/2015 03:58 PM, Pádraig Brady wrote:
> From 12c6f0fd7f44133a2af8950c69b2bfa46ea5d3a4 Mon Sep 17 00:00:00 2001
> From: Giuseppe Scrivano <address@hidden>
> Date: Sun, 25 Jan 2015 01:33:45 +0100
> Subject: [PATCH] sync: support syncing specified arguments
> --- a/doc/coreutils.texi
> +++ b/doc/coreutils.texi
> @@ -12043,18 +12043,40 @@ with @env{TZ}, libc, The GNU C Library Reference
> Manual}.
> @command{sync} writes any data buffered in memory out to disk. This can
> include (but is not limited to) modified superblocks, modified inodes,
> and delayed reads and writes. This must be implemented by the kernel;
> -The @command{sync} program does nothing but exercise the @code{sync} system
> -call.
> +The @command{sync} program does nothing but exercise the @code{sync},
> address@hidden, @code{fsync}, and @code{fdatasync} system calls.
I think sync's info page now deserves a "Synopsis" line ... as the command
now takes more than just --help/--version.
Maybe the first line of 'man sync'
sync - flush file system buffers
and 'info sync'
"synchronize memory and disk" (in the parent table), and
"sync data on disk with memory" (sync invocation)
should be harmonized, too?
> diff --git a/src/sync.c b/src/sync.c
> index e9f4d7e..80d1403 100644
> --- a/src/sync.c
> +++ b/src/sync.c
> @@ -37,11 +61,20 @@ usage (int status)
> emit_try_help ();
> else
> {
> - printf (_("Usage: %s [OPTION]\n"), program_name);
> + printf (_("Usage: %s [OPTION] [FILE]...\n"), program_name);
> fputs (_("\
> Force changed blocks to disk, update the super block.\n\
> \n\
> +If one or more file paths are specified, sync only them,\n\
> +use --data and --file-system to change the default behavior\n\
> +\n\
> "), stdout);
> +
> + fputs (_("\
> + --file-system sync the file systems that contain the files\n\
> + --data only sync data for files, no unneeded
> metadata\n\
> +"), stdout);
> +
'--d' should go before '--f'.
And shouldn't we also be more translator-friendly, and split the
2 options into 2 fputs calls?
The rest of the patch including the test almost LGTM:
when running against a non-accessible directory, then the correct error
diagnostic ("permission denied") is eclipsed by a non-descriptive
diagnostic:
$ src/sync --file /tmp /root
src/sync: error opening ‘/root’: Is a directory
strace output of the above:
open("/root", O_RDONLY|O_NONBLOCK) = -1 EACCES (Permission denied)
open("/root", O_WRONLY|O_NONBLOCK) = -1 EISDIR (Is a directory)
Please note that syncfs() for the /tmp argument succeeded correctly.
For completeness, here is the correct case with a good error diagnostic
for an inaccessible file:
$ src/sync --file /root/.bashrc
src/sync: error opening ‘/root/.bashrc’: Permission denied
Thanks & have a nice day,
Berny
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, (continued)
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Paul Eggert, 2015/01/25
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Pádraig Brady, 2015/01/25
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Bernhard Voelker, 2015/01/25
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Pádraig Brady, 2015/01/25
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Giuseppe Scrivano, 2015/01/26
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Pádraig Brady, 2015/01/26
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Giuseppe Scrivano, 2015/01/26
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Pádraig Brady, 2015/01/26
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Giuseppe Scrivano, 2015/01/27
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Pádraig Brady, 2015/01/27
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified,
Bernhard Voelker <=
- bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Pádraig Brady, 2015/01/28
bug#19681: [PATCH] sync: use syncfs(2) if any argument is specified, Eric Blake, 2015/01/26