On Wednesday, November 7, 2018 12:33:45 AM CET Bernhard Voelker wrote: > On 11/6/18 7:35 PM, Paul Eggert wrote: > > Thanks, I installed that and am closing the bug report. > > That was a real bug, i.e., not only a resource leak, wasn't it? > > If the calling user has -r+w permissions on the file, then sync previously > exited with 1 without actually syncing: > > $ install -m 0200 /dev/null /tmp/file > > $ ls -log /tmp/file > --w------- 1 0 Nov 7 00:06 /tmp/file > > $ strace -v /usr/bin/sync /tmp/file 2>&1 | tail -n6 > openat(AT_FDCWD, "/tmp/file", O_RDONLY|O_NONBLOCK) = -1 EACCES (Permission > denied) openat(AT_FDCWD, "/tmp/file", O_WRONLY|O_NONBLOCK) = 3 > close(1) = 0 > close(2) = 0 > exit_group(1) = ? > +++ exited with 1 +++ > > With the patch, fsync is called, and sync terminated with success: > > $ strace -v src/sync /tmp/file 2>&1 | tail > openat(AT_FDCWD, "/tmp/file", O_RDONLY|O_NONBLOCK) = -1 EACCES (Permission > denied) openat(AT_FDCWD, "/tmp/file", O_WRONLY|O_NONBLOCK) = 3 > fcntl(3, F_GETFL) = 0x8801 (flags > O_WRONLY|O_NONBLOCK|O_LARGEFILE) fcntl(3, F_SETFL, O_WRONLY|O_LARGEFILE) = > 0 > fsync(3) = 0 > close(3) = 0 > close(1) = 0 > close(2) = 0 > exit_group(0) = ? > +++ exited with 0 +++ > > Should we add a NEWS entry and a test - see attached?
Looks good. Thanks for the follow-up patch! Kamil > Thanks & have a nice day, > Berny
