zsh-workers
 help / color / mirror / code / Atom feed
* Any way to allow clobbering empty files when noclobber is set?
@ 2020-06-03  2:08 ` Martin Tournoij
  2020-06-03 12:04   ` Peter Stephenson
  0 siblings, 1 reply; 43+ messages in thread
From: Martin Tournoij @ 2020-06-03  2:08 UTC (permalink / raw)
  To: zsh-workers

Hi there,

I switched from tcsh to zsh a while ago (many years too late, I know), and
found zsh can do pretty much everything better. There's one thing I rather
miss though: the 'notempty' option in 'noclobber'. To quote tcsh(1):

> If the shell variable noclobber is set, then the file must not exist or be
> a character special file [...] If notempty is given in noclobber, `>' is
> allowed on empty files; if ask is set, an interacive confirmation is
> presented, rather than an error.

1. Is there any way to do this in zsh (especially the notempty option)? I
   know you can use >! to override it, but empty files are rarely important
   so just allowing them to be clobbered seems better UX in most cases.
   I can't find anything in zshall(1) or the interwebz.

2. Assuming the answer to 1) is "no", would any patches be accepted to add a
   setting for this? I rather miss this behaviour (I actually added these
   settings to tcsh a few years ago).

For context, the most common case (for me, anyway) is when I accidentally
create a file with a failing command:

    $ cmd -wrong-flag > foo
    stderr: you made a mistake

    # Oops, we used a wrong flag, let's correct it:
    $ cmd -correct-flag > foo
    zsh: file exists: foo

    # Ah, an empty file still got created >_< Need to remove it first, or
    # replace > with >!
    $ rm foo
    $ cmd -correct arg > foo

Thanks,
Martin


P.S. The emailing list software told me to email zsh-workers-faq@zsh.org for
a FAQ about this list, which is what I did and it came back with:

> FAQ - Frequently asked questions of the zsh-workers@zsh.org list.
>
> None available yet.

Maybe remove that part from the template, as there is no FAQ?

^ permalink raw reply	[flat|nested] 43+ messages in thread

* RE: Any way to allow clobbering empty files when noclobber is set?
  2020-06-03  2:08 ` Any way to allow clobbering empty files when noclobber is set? Martin Tournoij
@ 2020-06-03 12:04   ` Peter Stephenson
  2020-06-04  1:48     ` Daniel Shahaf
                       ` (3 more replies)
  0 siblings, 4 replies; 43+ messages in thread
From: Peter Stephenson @ 2020-06-03 12:04 UTC (permalink / raw)
  To: Martin Tournoij, zsh-workers

[-- Attachment #1: Type: text/plain, Size: 315 bytes --]

Martin Tournoij wrote:
> I switched from tcsh to zsh a while ago (many years too late, I know),
> and found zsh can do pretty much everything better. There's one thing
> I rather miss though: the 'notempty' option in 'noclobber'.

This isn't actually hard to implement.  What does everyone else think?

pws

[-- Attachment #2: clobber_empty.dif --]
[-- Type: application/vnd.ms-excel, Size: 3482 bytes --]

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-03 12:04   ` Peter Stephenson
@ 2020-06-04  1:48     ` Daniel Shahaf
  2020-06-04  2:43       ` Bart Schaefer
  2020-06-04  6:31       ` Martin Tournoij
  2020-06-04  2:13     ` Vin Shelton
                       ` (2 subsequent siblings)
  3 siblings, 2 replies; 43+ messages in thread
From: Daniel Shahaf @ 2020-06-04  1:48 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Martin Tournoij, zsh-workers

Peter Stephenson wrote on Wed, 03 Jun 2020 12:04 +0000:
> Martin Tournoij wrote:
> > I switched from tcsh to zsh a while ago (many years too late, I know),
> > and found zsh can do pretty much everything better. There's one thing
> > I rather miss though: the 'notempty' option in 'noclobber'.  
> 
> This isn't actually hard to implement.  What does everyone else think?

I certainly have several cases in my $HISTFILE where I repeated
a command with «>» changed to «>!»,¹ in the pattern Martin described.

I suppose that pattern is common enough to offset the standard "Not
another option!" concern.

However, I wonder whether there are other ways to peel this orange.  For
example, I don't recall _other_ cases where I might have found the
proposed behaviour useful — that is, cases where I used «>» to create
a file, got an error because the file had been created as empty before
I executed my command, and would have preferred for shell to just
proceed without flagging an error — so, thinking out loud, how about an
option that does the following:

    Given «foo > bar», if «foo» was run and exited non-zero and
    NO_CLOBBER in effect [which implies that «bar» didn't exist before
    foo was run], check whether «bar» is zero-size and, if so, unlink it.

?

[To be clear, no objection to CLOBBER_EMPTY as posted; just brainstorming
alternatives.]

Peter Stephenson wrote on Wed, 03 Jun 2020 12:04 +0000:
> +++ b/Doc/Zsh/options.yo
> @@ -1168,6 +1168,19 @@ If the option is not set, and the option tt(APPEND_CREATE) is also
> +item(tt(CLOBBER_EMPTY))(
> +This option is only used if the option tt(CLOBBER) is not set: note that
> +it is set by default.

The referent of the pronoun "it" is ambiguous.

Cheers,

Daniel

¹ The first five of them [I didn't check further] are a pretty varied
  bunch: some of them are a single simple command with an output
  redirection; some of them have the redirection inside a function [or
  a «strace zsh -c»]; and one of them uses the $NULLCMD syntax with
  literally nothing on the line other than the redirection operator and
  its file target.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-03 12:04   ` Peter Stephenson
  2020-06-04  1:48     ` Daniel Shahaf
@ 2020-06-04  2:13     ` Vin Shelton
  2020-06-04  2:35       ` Bart Schaefer
  2020-06-04  2:36       ` Daniel Shahaf
  2020-06-04  5:06     ` Bart Schaefer
  2020-06-04 12:15     ` Peter Stephenson
  3 siblings, 2 replies; 43+ messages in thread
From: Vin Shelton @ 2020-06-04  2:13 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Martin Tournoij, zsh-workers

[-- Attachment #1: Type: text/plain, Size: 609 bytes --]

If we want to support this feature, why don't we change the meaning of
NO_CLOBBER to mean only non-empty files?  What is the significance of a
0-length file as opposed to a file with contents?

  - Vin

On Wed, Jun 3, 2020 at 8:05 AM Peter Stephenson <p.stephenson@samsung.com>
wrote:

> Martin Tournoij wrote:
> > I switched from tcsh to zsh a while ago (many years too late, I know),
> > and found zsh can do pretty much everything better. There's one thing
> > I rather miss though: the 'notempty' option in 'noclobber'.
>
> This isn't actually hard to implement.  What does everyone else think?
>
> pws
>

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-04  2:13     ` Vin Shelton
@ 2020-06-04  2:35       ` Bart Schaefer
  2020-06-04  2:36       ` Daniel Shahaf
  1 sibling, 0 replies; 43+ messages in thread
From: Bart Schaefer @ 2020-06-04  2:35 UTC (permalink / raw)
  To: Vin Shelton; +Cc: Peter Stephenson, Martin Tournoij, zsh-workers

On Wed, Jun 3, 2020 at 7:14 PM Vin Shelton <acs@alumni.princeton.edu> wrote:
>
> If we want to support this feature, why don't we change the meaning of
> NO_CLOBBER to mean only non-empty files?  What is the significance of a
> 0-length file as opposed to a file with contents?

Timestamps.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-04  2:13     ` Vin Shelton
  2020-06-04  2:35       ` Bart Schaefer
@ 2020-06-04  2:36       ` Daniel Shahaf
  2020-06-04 11:57         ` Vin Shelton
  1 sibling, 1 reply; 43+ messages in thread
From: Daniel Shahaf @ 2020-06-04  2:36 UTC (permalink / raw)
  To: Vin Shelton; +Cc: Peter Stephenson, Martin Tournoij, zsh-workers

Vin Shelton wrote on Wed, 03 Jun 2020 22:13 -0400:
> What is the significance of a 0-length file as opposed to a file with
> contents?

There _are_ some cases where a 0-length file is treated differently to
a non-existing one:

- .vimrc
- «cat foo» (also #include's in C)
- GNU make(1)'s lookup order: GNUmakefile / Makefile / makefile
- /etc/nologin

> If we want to support this feature, why don't we change the meaning of
> NO_CLOBBER to mean only non-empty files?

Because users might be relying on the semantics that the documentation
promises.

For example, a user who does «echo foo > /etc/nologin» and doesn't get
an error may infer that it's fine to delete the file once they finish
whatever change they're working on.  Keeping the error would alert that
user that they shouldn't remove the file when they're done (and, most
likely, should pause to coordinate with whoever created /etc/nologin
before them).

Cheers,

Daniel


>   - Vin
> 
> On Wed, Jun 3, 2020 at 8:05 AM Peter Stephenson <p.stephenson@samsung.com>
> wrote:
> 
> > Martin Tournoij wrote:
> > > I switched from tcsh to zsh a while ago (many years too late, I know),
> > > and found zsh can do pretty much everything better. There's one thing
> > > I rather miss though: the 'notempty' option in 'noclobber'.
> >
> > This isn't actually hard to implement.  What does everyone else think?
> >
> > pws
> >


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-04  1:48     ` Daniel Shahaf
@ 2020-06-04  2:43       ` Bart Schaefer
  2020-06-04  4:06         ` Daniel Shahaf
  2020-06-04  6:31       ` Martin Tournoij
  1 sibling, 1 reply; 43+ messages in thread
From: Bart Schaefer @ 2020-06-04  2:43 UTC (permalink / raw)
  To: zsh-workers; +Cc: Martin Tournoij

On Wed, Jun 3, 2020 at 6:49 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> I certainly have several cases in my $HISTFILE where I repeated
> a command with «>» changed to «>!»,¹ in the pattern Martin described.

Are you sure you haven't set the HIST_ALLOW_CLOBBER option?

> ... thinking out loud, how about an
> option that does the following:
>
>     Given «foo > bar», if «foo» was run and exited non-zero and
>     NO_CLOBBER in effect [which implies that «bar» didn't exist before
>     foo was run], check whether «bar» is zero-size and, if so, unlink it.

How does that user experience differ from HIST_ALLOW_CLOBBER?  In
either of those cases, you have to retrieve the command from history
and execute it again ... and silently unlinking a file is not very
friendly.  The /etc/nologin example in your next message comes to
mind.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-04  2:43       ` Bart Schaefer
@ 2020-06-04  4:06         ` Daniel Shahaf
  2020-06-04  5:00           ` Bart Schaefer
  0 siblings, 1 reply; 43+ messages in thread
From: Daniel Shahaf @ 2020-06-04  4:06 UTC (permalink / raw)
  To: Bart Schaefer, zsh-workers; +Cc: Martin Tournoij

Bart Schaefer wrote on Thu, 04 Jun 2020 02:43 +00:00:
> On Wed, Jun 3, 2020 at 6:49 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> >
> > I certainly have several cases in my $HISTFILE where I repeated
> > a command with «>» changed to «>!»,¹ in the pattern Martin described.
> 
> Are you sure you haven't set the HIST_ALLOW_CLOBBER option?
> 

I am.  I really do have consecutive pairs of lines where the second line
is equal to the first one with s/>/>!/.

> > ... thinking out loud, how about an
> > option that does the following:
> >
> >     Given «foo > bar», if «foo» was run and exited non-zero and
> >     NO_CLOBBER in effect [which implies that «bar» didn't exist before
> >     foo was run], check whether «bar» is zero-size and, if so, unlink it.

[ Let's call this UNLINK_EMPTY_AFTER_FAILURE for the sake of discussion. ]

> 
> How does that user experience differ from HIST_ALLOW_CLOBBER?  In
> either of those cases, you have to retrieve the command from history
> and execute it again ...

Well, for one, because that proposal only takes effect when foo exited
non-zero, created bar, and bar is zero-sized?  HIST_ALLOW_CLOBBER
applies even when some of these conditions don't hold.

> and silently unlinking a file is not very friendly.  The /etc/nologin
> example in your next message comes to mind.

Doesn't this argument also apply to anyone who might set CLOBBER_EMPTY
and then lose the timestamps on empty files, as in your next message?
That's why these options are opt-in.

Seriously, though, it's perfectly possible that CLOBBER_EMPTY is the
better solution here.  I was only brainstorming.

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-04  4:06         ` Daniel Shahaf
@ 2020-06-04  5:00           ` Bart Schaefer
  2020-06-05  3:10             ` Daniel Shahaf
  0 siblings, 1 reply; 43+ messages in thread
From: Bart Schaefer @ 2020-06-04  5:00 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers, Martin Tournoij

On Wed, Jun 3, 2020 at 9:06 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:>
>
> [ Let's call this UNLINK_EMPTY_AFTER_FAILURE for the sake of discussion. ]
>
> Bart Schaefer wrote on Thu, 04 Jun 2020 02:43 +00:00:
> >
> > How does that user experience differ from HIST_ALLOW_CLOBBER?
>
> Well, for one, because that proposal only takes effect when foo exited
> non-zero, created bar, and bar is zero-sized?

Fair enough, although (silly example) "false > empty" would remove the
file?  Wouldn't you at least want the reason for the unlink to be that
the redirection failed, rather than that the command exited non-zero?

What if the redirection target is a symbolic link?  What if it isn't a
plain file?

> > and silently unlinking a file is not very friendly.  The /etc/nologin
> > example in your next message comes to mind.
>
> Doesn't this argument also apply to anyone who might set CLOBBER_EMPTY
> and then lose the timestamps on empty files, as in your next message?

Yes, but to a lesser degree, because the file is at least still
present if it's presence means something.

> Seriously, though, it's perfectly possible that CLOBBER_EMPTY is the
> better solution here.  I was only brainstorming.

Sure, you just got me curious about thinking through it.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-03 12:04   ` Peter Stephenson
  2020-06-04  1:48     ` Daniel Shahaf
  2020-06-04  2:13     ` Vin Shelton
@ 2020-06-04  5:06     ` Bart Schaefer
  2020-06-04  5:41       ` Roman Perepelitsa
                         ` (2 more replies)
  2020-06-04 12:15     ` Peter Stephenson
  3 siblings, 3 replies; 43+ messages in thread
From: Bart Schaefer @ 2020-06-04  5:06 UTC (permalink / raw)
  To: zsh-workers; +Cc: Martin Tournoij

On Wed, Jun 3, 2020 at 5:05 AM Peter Stephenson
<p.stephenson@samsung.com> wrote:
>
> Martin Tournoij wrote:
> > I rather miss though: the 'notempty' option in 'noclobber'.
>
> This isn't actually hard to implement.  What does everyone else think?

Gmail thinks ".dif" is a spreadsheet, and MacOS thinks it's a video.

More seriously ... what, if any, effect ought this have on "command >>
nonexistent" ?

I'm not familiar with the tcsh variation of this.  Does it matter if
the file is a symlink?

Should this be done with open()-then-fstat() as is done for
non-regular files instead of stat()-then-open(), to avoid race
conditions?

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-04  5:06     ` Bart Schaefer
@ 2020-06-04  5:41       ` Roman Perepelitsa
  2020-06-05  2:07         ` Daniel Shahaf
  2020-06-04  6:47       ` Martin Tournoij
  2020-06-04  9:42       ` Peter Stephenson
  2 siblings, 1 reply; 43+ messages in thread
From: Roman Perepelitsa @ 2020-06-04  5:41 UTC (permalink / raw)
  To: zsh-workers; +Cc: Martin Tournoij

The regular noclobber can use O_CREAT | O_EXCL to avoid races. How
would any of the proposed modifications work? That is, how to
implement unlink-if-empty without ever unlinking non-empty files? Or
how to implement clobber-if-empty without ever clobbering non-empty
files?

Roman.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-04  1:48     ` Daniel Shahaf
  2020-06-04  2:43       ` Bart Schaefer
@ 2020-06-04  6:31       ` Martin Tournoij
  2020-06-05  2:22         ` Daniel Shahaf
  1 sibling, 1 reply; 43+ messages in thread
From: Martin Tournoij @ 2020-06-04  6:31 UTC (permalink / raw)
  To: Daniel Shahaf, Peter Stephenson; +Cc: zsh-workers

On Thu, Jun 4, 2020, at 09:48, Daniel Shahaf wrote:
> However, I wonder whether there are other ways to peel this orange.  For
> example, I don't recall _other_ cases where I might have found the
> proposed behaviour useful — that is, cases where I used «>» to create
> a file, got an error because the file had been created as empty before
> I executed my command, and would have preferred for shell to just
> proceed without flagging an error — so, thinking out loud, how about an
> option that does the following:
> 
>     Given «foo > bar», if «foo» was run and exited non-zero and
>     NO_CLOBBER in effect [which implies that «bar» didn't exist before
>     foo was run], check whether «bar» is zero-size and, if so, unlink it.
> 
> ?
> 
> [To be clear, no objection to CLOBBER_EMPTY as posted; just brainstorming
> alternatives.]

The thinking was that it's the "least magical" behaviour I could think of:
if I tell zsh to "NO_CLOBBER" then I'm telling it to "don't make me lose
data when I'm being stupid". I think allowing it to overwrite empty files
fits rather nicely with the intent of NO_CLOBBER.

This seems less magic to me than "> creates a file, except when [...]".

That being said, you solution would obviously also be workable, and don't
overly care one way or the other, but this was my thinking behind it.

Also, I'm almost disappointed Peter wrote a patch already, as I was looking
forward to doing it myself heh 😅

Cheerio,
Martin

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-04  5:06     ` Bart Schaefer
  2020-06-04  5:41       ` Roman Perepelitsa
@ 2020-06-04  6:47       ` Martin Tournoij
  2020-06-04  9:42       ` Peter Stephenson
  2 siblings, 0 replies; 43+ messages in thread
From: Martin Tournoij @ 2020-06-04  6:47 UTC (permalink / raw)
  To: zsh-workers

On Thu, Jun 4, 2020, at 13:06, Bart Schaefer wrote:
> More seriously ... what, if any, effect ought this have on "command >>
> nonexistent" ?

Probably none?

> I'm not familiar with the tcsh variation of this.  Does it matter if
> the file is a symlink?
> 
> Should this be done with open()-then-fstat() as is done for
> non-regular files instead of stat()-then-open(), to avoid race
> conditions?

tcsh does a stat() to check the file permissions and size, and then
xcreat()s (some wrapper around creat()?) it if it all looks good; see:

https://github.com/tcsh-org/tcsh/commit/cc952ed342f6460a33fa77111c4edfcd915ce9b5#diff-1c39f18ef0f96621a9b743b6e4400fabR984

You're right that this is subject to race conditions since there might be
data written to the file in between the 2 calls. I'm not entirely sure how
realistic this scenario is though?

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-04  5:06     ` Bart Schaefer
  2020-06-04  5:41       ` Roman Perepelitsa
  2020-06-04  6:47       ` Martin Tournoij
@ 2020-06-04  9:42       ` Peter Stephenson
  2020-06-04 12:20         ` Roman Perepelitsa
  2 siblings, 1 reply; 43+ messages in thread
From: Peter Stephenson @ 2020-06-04  9:42 UTC (permalink / raw)
  To: zsh-workers; +Cc: Martin Tournoij


> On 04 June 2020 at 06:06 Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Wed, Jun 3, 2020 at 5:05 AM Peter Stephenson
> <p.stephenson@samsung.com> wrote:
> >
> > Martin Tournoij wrote:
> > > I rather miss though: the 'notempty' option in 'noclobber'.
> >
> > This isn't actually hard to implement.  What does everyone else think?
> 
> Gmail thinks ".dif" is a spreadsheet, and MacOS thinks it's a video.

And, obviously, as a mere user, you're not allowed to treat it any
differently without actually downloading it...
 
> More seriously ... what, if any, effect ought this have on "command >>
> nonexistent" ?

I couldn't see any direct relevance, but there may be some overlap
I've missed.
 
> I'm not familiar with the tcsh variation of this.  Does it matter if
> the file is a symlink?
> 
> Should this be done with open()-then-fstat() as is done for
> non-regular files instead of stat()-then-open(), to avoid race
> conditions?

I'm not entirely sure if we get quite the same behaviour if we do
that.  At the least we'd need to open the existing file without
truncating (then either close it or leave it open for writing
if it was already empty).  Unless there's a security aspect to this,
with there already being an endless number of things to go wrong if
files are actively changing under you, I'd be inclined just to keep
it simple.  Happy to hear a knockout argument the other way.

pws

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-04  2:36       ` Daniel Shahaf
@ 2020-06-04 11:57         ` Vin Shelton
  0 siblings, 0 replies; 43+ messages in thread
From: Vin Shelton @ 2020-06-04 11:57 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Peter Stephenson, Martin Tournoij, zsh-workers

[-- Attachment #1: Type: text/plain, Size: 1848 bytes --]

IMO, there is little to no reason to complexify the code and add to the
users' cognitive burden by adding a new option.  That written, NO_CLOBBER
is a feature I don't use, so I'll contentedly end my part of this
conversation now.

  - Vin


On Wed, Jun 3, 2020 at 10:36 PM Daniel Shahaf <d.s@daniel.shahaf.name>
wrote:

> Vin Shelton wrote on Wed, 03 Jun 2020 22:13 -0400:
> > What is the significance of a 0-length file as opposed to a file with
> > contents?
>
> There _are_ some cases where a 0-length file is treated differently to
> a non-existing one:
>
> - .vimrc
> - «cat foo» (also #include's in C)
> - GNU make(1)'s lookup order: GNUmakefile / Makefile / makefile
> - /etc/nologin
>
> > If we want to support this feature, why don't we change the meaning of
> > NO_CLOBBER to mean only non-empty files?
>
> Because users might be relying on the semantics that the documentation
> promises.
>
> For example, a user who does «echo foo > /etc/nologin» and doesn't get
> an error may infer that it's fine to delete the file once they finish
> whatever change they're working on.  Keeping the error would alert that
> user that they shouldn't remove the file when they're done (and, most
> likely, should pause to coordinate with whoever created /etc/nologin
> before them).
>
> Cheers,
>
> Daniel
>
>
> >   - Vin
> >
> > On Wed, Jun 3, 2020 at 8:05 AM Peter Stephenson <
> p.stephenson@samsung.com>
> > wrote:
> >
> > > Martin Tournoij wrote:
> > > > I switched from tcsh to zsh a while ago (many years too late, I
> know),
> > > > and found zsh can do pretty much everything better. There's one thing
> > > > I rather miss though: the 'notempty' option in 'noclobber'.
> > >
> > > This isn't actually hard to implement.  What does everyone else think?
> > >
> > > pws
> > >
>
>

^ permalink raw reply	[flat|nested] 43+ messages in thread

* RE: Any way to allow clobbering empty files when noclobber is set?
  2020-06-03 12:04   ` Peter Stephenson
                       ` (2 preceding siblings ...)
  2020-06-04  5:06     ` Bart Schaefer
@ 2020-06-04 12:15     ` Peter Stephenson
  2020-06-04 20:35       ` Bart Schaefer
  3 siblings, 1 reply; 43+ messages in thread
From: Peter Stephenson @ 2020-06-04 12:15 UTC (permalink / raw)
  To: Martin Tournoij, zsh-workers

> On 03 June 2020 at 13:04 Peter Stephenson <p.stephenson@samsung.com> wrote:
> Martin Tournoij wrote:
> > I switched from tcsh to zsh a while ago (many years too late, I know),
> > and found zsh can do pretty much everything better. There's one thing
> > I rather miss though: the 'notempty' option in 'noclobber'.
> 
> This isn't actually hard to implement.

One thing I missed is that we already open the file and run fstat to
check if it's regular.  We can simply check if it's empty at the
same point, so there's no additional operation except for the
clobbering open if that test succeeded.

This is pasted into a webmail client as an experiment, it could do
anything...

pws

diff --git a/Doc/Zsh/options.yo b/Doc/Zsh/options.yo
index 2b7637ff4..a42daa56f 100644
--- a/Doc/Zsh/options.yo
+++ b/Doc/Zsh/options.yo
@@ -1168,6 +1168,19 @@ If the option is not set, and the option tt(APPEND_CREATE) is also
 not set, `tt(>>!)' or `tt(>>|)' must be used to create a file.
 If either option is set, `tt(>>)' may be used.
 )
+pindex(CLOBBER_EMPTY)
+pindex(NO_CLOBBER_EMPTY)
+pindex(CLOBBEREMPTY)
+pindex(NOCLOBBEREMPTY)
+cindex(clobbering, of empty files)
+cindex(file clobbering, of empty files)
+item(tt(CLOBBER_EMPTY))(
+This option is only used if the option tt(CLOBBER) is not set: note that
+it is set by default.
+
+If this option is set, then regular files of zero length may be
+ovewritten (`clobbered').
+)
 pindex(CORRECT)
 pindex(NO_CORRECT)
 pindex(NOCORRECT)
diff --git a/Src/exec.c b/Src/exec.c
index 29f4fc5ca..8e9443838 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2143,14 +2143,15 @@ clobber_open(struct redir *f)
 {
     struct stat buf;
     int fd, oerrno;
+    char *ufname = unmeta(f->name);
 
     /* If clobbering, just open. */
     if (isset(CLOBBER) || IS_CLOBBER_REDIR(f->type))
-	return open(unmeta(f->name),
+	return open(ufname,
 		O_WRONLY | O_CREAT | O_TRUNC | O_NOCTTY, 0666);
 
     /* If not clobbering, attempt to create file exclusively. */
-    if ((fd = open(unmeta(f->name),
+    if ((fd = open(ufname,
 		   O_WRONLY | O_CREAT | O_EXCL | O_NOCTTY, 0666)) >= 0)
 	return fd;
 
@@ -2158,11 +2159,25 @@ clobber_open(struct redir *f)
      * Try opening, and if it's a regular file then close it again    *
      * because we weren't supposed to open it.                        */
     oerrno = errno;
-    if ((fd = open(unmeta(f->name), O_WRONLY | O_NOCTTY)) != -1) {
-	if(!fstat(fd, &buf) && !S_ISREG(buf.st_mode))
-	    return fd;
+    if ((fd = open(ufname, O_WRONLY | O_NOCTTY)) != -1) {
+	if(!fstat(fd, &buf)) {
+	    if (!S_ISREG(buf.st_mode))
+		return fd;
+	    /*
+	     * If zero-length, we'll check CLOBBER_EMPTY.
+	     * As we're clobbering, not re-using, in that case
+	     * we'll open again.
+	     */
+	    if (isset(CLOBBEREMPTY) && buf.st_size == 0)
+	    {
+		close(fd);
+		return open(ufname, O_WRONLY | O_CREAT | O_TRUNC | O_NOCTTY,
+			    0666);
+	    }
+	}
 	close(fd);
     }
+
     errno = oerrno;
     return -1;
 }
diff --git a/Src/options.c b/Src/options.c
index 7586d21d2..fba021e7d 100644
--- a/Src/options.c
+++ b/Src/options.c
@@ -114,6 +114,7 @@ static struct optname optns[] = {
 {{NULL, "checkjobs",	      OPT_EMULATE|OPT_ZSH},	 CHECKJOBS},
 {{NULL, "checkrunningjobs",   OPT_EMULATE|OPT_ZSH},	 CHECKRUNNINGJOBS},
 {{NULL, "clobber",	      OPT_EMULATE|OPT_ALL},	 CLOBBER},
+{{NULL, "clobberempty",	      0},			 CLOBBEREMPTY},
 {{NULL, "combiningchars",     0},			 COMBININGCHARS},
 {{NULL, "completealiases",    0},			 COMPLETEALIASES},
 {{NULL, "completeinword",     0},			 COMPLETEINWORD},
diff --git a/Src/zsh.h b/Src/zsh.h
index 1f2d774a1..ed123f2b9 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -2378,6 +2378,7 @@ enum {
     CHECKJOBS,
     CHECKRUNNINGJOBS,
     CLOBBER,
+    CLOBBEREMPTY,
     APPENDCREATE,
     COMBININGCHARS,
     COMPLETEALIASES,
diff --git a/Test/A04redirect.ztst b/Test/A04redirect.ztst
index d60519064..993138e7d 100644
--- a/Test/A04redirect.ztst
+++ b/Test/A04redirect.ztst
@@ -708,3 +708,17 @@
   cat <&$testfd
 0:Regression test for here document with fd declarator
 >  This is, in some sense, a here document.
+
+  (setopt noclobber clobberempty
+  rm -f foo
+  touch foo
+  print Works >foo
+  cat foo
+  print Works not >foo
+  # Make sure the file was not harmed
+  cat foo
+  )
+0:CLOBBER_EMPTY
+>Works
+>Works
+?(eval):6: file exists: foo

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-04  9:42       ` Peter Stephenson
@ 2020-06-04 12:20         ` Roman Perepelitsa
  2020-06-04 12:26           ` Peter Stephenson
  0 siblings, 1 reply; 43+ messages in thread
From: Roman Perepelitsa @ 2020-06-04 12:20 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list, Martin Tournoij

On Thu, Jun 4, 2020 at 11:43 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>  At the least we'd need to open the existing file without
> truncating (then either close it or leave it open for writing
> if it was already empty).

If you mean opening the file with O_APPEND, this could result in
unexpected file content. Suppose N processes concurrently write "hello
world" to /foo/bar with the new no_clobber_non_empty option. The final
content of the file can be up to N * strlen("hello world") bytes long
with mangled content. If writing were done with the regular
no_clobber, there would be a guarantee that at any point readers of
the file would either not see the file, or would see content that is a
prefix of "hello world".

I should also note that the existing no_clobber option can be used to
implement file locks.

    emulate zsh -o no_clobber
    if ! print -n >/var/lock; then
     print -ru2 'cannot acquire lock'
     exit 1
    fi
    do_exclusive_stuff
    unlink /var/lock

Regardless of what gets decided w.r.t. no-clobber extensions, I hope
this would keep working.

Roman.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-04 12:20         ` Roman Perepelitsa
@ 2020-06-04 12:26           ` Peter Stephenson
  0 siblings, 0 replies; 43+ messages in thread
From: Peter Stephenson @ 2020-06-04 12:26 UTC (permalink / raw)
  To: Zsh hackers list; +Cc: Martin Tournoij


> On 04 June 2020 at 13:20 Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote:
> On Thu, Jun 4, 2020 at 11:43 AM Peter Stephenson
> <p.w.stephenson@ntlworld.com> wrote:
> >  At the least we'd need to open the existing file without
> > truncating (then either close it or leave it open for writing
> > if it was already empty).
> 
> If you mean opening the file with O_APPEND, this could result in
> unexpected file content.
>
> I should also note that the existing no_clobber option can be used to
> implement file locks.
> 
> Regardless of what gets decided w.r.t. no-clobber extensions, I hope
> this would keep working.

Indeed, this is the sort of thing backing up my thinking that we keep
the clobbering open function call exactly as it now is if we add the
extension for empty files.

pws

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-04 12:15     ` Peter Stephenson
@ 2020-06-04 20:35       ` Bart Schaefer
  2020-06-05  1:59         ` Daniel Shahaf
  0 siblings, 1 reply; 43+ messages in thread
From: Bart Schaefer @ 2020-06-04 20:35 UTC (permalink / raw)
  To: zsh-workers; +Cc: Martin Tournoij

On Thu, Jun 4, 2020 at 5:16 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> One thing I missed is that we already open the file and run fstat to
> check if it's regular.  We can simply check if it's empty at the
> same point

Exactly where my earlier question came from.

> +           if (isset(CLOBBEREMPTY) && buf.st_size == 0)
> +           {
> +               close(fd);
> +               return open(ufname, O_WRONLY | O_CREAT | O_TRUNC | O_NOCTTY,
> +                           0666);
> +           }

Considering the concurrent-openers situation that Roman mentioned, I'm
debating whether there is any benefit to doing:

  int newfd = open(ufname, O_WRONLY | O_CREAT | O_TRUNC | O_NOCTTY, 0666);
  close(fd);
  return newfd;

I have a vague sense that keeping the descriptor open until the new
file is created might prevent some races, but I can't recite an
example.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-04 20:35       ` Bart Schaefer
@ 2020-06-05  1:59         ` Daniel Shahaf
  0 siblings, 0 replies; 43+ messages in thread
From: Daniel Shahaf @ 2020-06-05  1:59 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: zsh-workers, Martin Tournoij

Bart Schaefer wrote on Thu, 04 Jun 2020 13:35 -0700:
> On Thu, Jun 4, 2020 at 5:16 AM Peter Stephenson
> <p.w.stephenson@ntlworld.com> wrote:
> >
> > One thing I missed is that we already open the file and run fstat to
> > check if it's regular.  We can simply check if it's empty at the
> > same point  
> 
> Exactly where my earlier question came from.
> 
> > +           if (isset(CLOBBEREMPTY) && buf.st_size == 0)
> > +           {
> > +               close(fd);
> > +               return open(ufname, O_WRONLY | O_CREAT | O_TRUNC | O_NOCTTY,
> > +                           0666);
> > +           }  
> 
> Considering the concurrent-openers situation that Roman mentioned, I'm
> debating whether there is any benefit to doing:
> 
>   int newfd = open(ufname, O_WRONLY | O_CREAT | O_TRUNC | O_NOCTTY, 0666);
>   close(fd);
>   return newfd;
> 
> I have a vague sense that keeping the descriptor open until the new
> file is created might prevent some races, but I can't recite an
> example.

This wouldn't prevent races, because the filename might be unlinked
between the first and second open(2) calls.

I think the right fix here is to simply change the body of the quoted
if statement to «return fd».  What do we gain by re-opening the file
with the O_TRUNC flag set when we already know the file is
zero-length?  It seems to me we're just introducing a race condition
for no reason.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-04  5:41       ` Roman Perepelitsa
@ 2020-06-05  2:07         ` Daniel Shahaf
  2020-06-05  4:38           ` Roman Perepelitsa
       [not found]           ` <1941572212.466119.1591360860372@mail2.virginmedia.com>
  0 siblings, 2 replies; 43+ messages in thread
From: Daniel Shahaf @ 2020-06-05  2:07 UTC (permalink / raw)
  To: Roman Perepelitsa; +Cc: zsh-workers, Martin Tournoij

Roman Perepelitsa wrote on Thu, 04 Jun 2020 07:41 +0200:
> The regular noclobber can use O_CREAT | O_EXCL to avoid races. How
> would any of the proposed modifications work? That is, how to
> implement unlink-if-empty without ever unlinking non-empty files?

I don't think that's possible: it'd require an atomic
stat-and-conditionally-unlink operation, and I don't think there's
a syscall for this.  fstatat(2)-then-unlinkat(2) gets close, but it's
still racy.

Good point.

> Or how to implement clobber-if-empty without ever clobbering non-empty
> files?

By using a non-clobbering open(), then calling fstat() on the open fd.
If it's zero length, just carry on.  If it's not zero length, close()
it and report an error.

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-04  6:31       ` Martin Tournoij
@ 2020-06-05  2:22         ` Daniel Shahaf
  0 siblings, 0 replies; 43+ messages in thread
From: Daniel Shahaf @ 2020-06-05  2:22 UTC (permalink / raw)
  To: Martin Tournoij; +Cc: Peter Stephenson, zsh-workers

Martin Tournoij wrote on Thu, 04 Jun 2020 14:31 +0800:
> The thinking was that it's the "least magical" behaviour I could think of:
> if I tell zsh to "NO_CLOBBER" then I'm telling it to "don't make me lose
> data when I'm being stupid". I think allowing it to overwrite empty files
> fits rather nicely with the intent of NO_CLOBBER.
> 
> This seems less magic to me than "> creates a file, except when [...]".
> 
> That being said, you solution would obviously also be workable, and don't
> overly care one way or the other, but this was my thinking behind it.
> 

*nod*

I was going to give as an example commands such as «tar -cf
foo /does/not/exist», «curl -o foo https://will/respond/404/not/found»,
and so on, but it turns out they actually do create a zero-length file
and return non-zero.

The evidence does seem to argue in favour of the CLOBBER_EMPTY approach…

> Also, I'm almost disappointed Peter wrote a patch already, as I was looking
> forward to doing it myself heh 😅

Our apologies, then ☺

Perhaps you'd be interested in writing some other patch?  See Etc/BUGS
and Test/list-XFails for known issues.  workers/45843#1, for example,
is a recently-reported segfault.

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-04  5:00           ` Bart Schaefer
@ 2020-06-05  3:10             ` Daniel Shahaf
  2020-06-05  3:18               ` Daniel Shahaf
  2020-06-06  1:07               ` Bart Schaefer
  0 siblings, 2 replies; 43+ messages in thread
From: Daniel Shahaf @ 2020-06-05  3:10 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: zsh-workers, Martin Tournoij

Bart Schaefer wrote on Wed, 03 Jun 2020 22:00 -0700:
> On Wed, Jun 3, 2020 at 9:06 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:>
> >
> > [ Let's call this UNLINK_EMPTY_AFTER_FAILURE for the sake of discussion. ]
> >
> > Bart Schaefer wrote on Thu, 04 Jun 2020 02:43 +00:00:  
> > >
> > > How does that user experience differ from HIST_ALLOW_CLOBBER?  
> >
> > Well, for one, because that proposal only takes effect when foo exited
> > non-zero, created bar, and bar is zero-sized?  
> 
> Fair enough, although (silly example) "false > empty" would remove the
> file?

Depends on whether ./empty was created by the redirection or not.  Files
would only be removed if they were created by the redirection operator:

    % cd "$(mktemp -d)"
    % ls
    % false > empty
    % ls
    % touch empty
    % false > empty
    % ls
    empty
    % 

> Wouldn't you at least want the reason for the unlink to be that the
> redirection failed, rather than that the command exited non-zero?
> 

I was thinking that if the command exited non-zero and didn't write
anything to stdout, then presumably the file was expected to contain
something and is of no use otherwise.  This rule DTRT's for diff(1), for
example, despite that command exiting non-zero even in some non-error cases.

Besides, if the redirection failed — I assume you mean the open(2)
failed — then isn't the unlink likely to fail as well?

> What if the redirection target is a symbolic link?  What if it isn't a
> plain file?

Again, the sequence of events is:

1. bar doesn't exist.
2. Run «foo > bar».  As part of this, we create bar as a zero-length plain file.
3. The exit code is non-zero.
4. We stat() bar to see if it's zero-length.

If in step #4 we find that bar is not even a plain file, we'll infer
that some other process has to have unlinked the plain file we created
in step 2, and we'll leave bar alone.

> > > and silently unlinking a file is not very friendly.  The /etc/nologin
> > > example in your next message comes to mind.  
> >
> > Doesn't this argument also apply to anyone who might set CLOBBER_EMPTY
> > and then lose the timestamps on empty files, as in your next message?  
> 
> Yes, but to a lesser degree, because the file is at least still
> present if it's presence means something.

Okay.  Is there a case where leaving around an empty file is worse than
removing it?  Consider, for example, «curl -o- http://example.com/pre-repvrop-change > hooks/pre-revprop-change»
in a Subversion repository.  The URL is misspelled, so the command will
create an empty pre-revprop-change file, which will allow non-undoable
changes to be made.  Furthermore, if the command had succeeded, only
some such changes would have been allowed; and if the file had not
existed, no such changes would have been allowed.  Given these
semantics, a sysadmin might well prefer to have _no_ pre-revprop-change
file rather than to have an empty one.

Another example: if I do «diff lorem ipsum > bar» and some other process
periodically polls whether bar has been created, the waiting process
might take an empty file to mean there's no difference between lorem and
ipsum.  In this situation, too, it might be better to unlink bar than to
leave it around empty.  (This could be made less contrived by changing
the scenario to someone copying a command from the output of `make -n`
and pasting it at the interactive prompt.  make(1) targets is another
case where empty files and non-existing files behave differently.)

Or perhaps something along the lines of «tar cf - foo > foo.tar; xz
foo.tar» in a single command line.  (Not a good example, though, since
here the user could've used «&&».)

Cheers,

Daniel

P.S.  In the pre-revprop-change example, the sysadmin should have
downloaded the file to a temporary name and then atomically renamed it
into place.  Some syntactic sugar for this pattern this might be nice…

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-05  3:10             ` Daniel Shahaf
@ 2020-06-05  3:18               ` Daniel Shahaf
  2020-06-06  1:07               ` Bart Schaefer
  1 sibling, 0 replies; 43+ messages in thread
From: Daniel Shahaf @ 2020-06-05  3:18 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: zsh-workers, Martin Tournoij

Daniel Shahaf wrote on Fri, 05 Jun 2020 03:10 +0000:
> P.S.  In the pre-revprop-change example, the sysadmin should have
> downloaded the file to a temporary name and then atomically renamed it
> into place.  Some syntactic sugar for this pattern this might be nice…

The pattern I'm referring to is just the "Spool some data to disk to a
temporary name, and then atomically rename(2) it into place" part, not
the "download" part.

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-05  2:07         ` Daniel Shahaf
@ 2020-06-05  4:38           ` Roman Perepelitsa
  2020-06-06  1:41             ` Bart Schaefer
       [not found]           ` <1941572212.466119.1591360860372@mail2.virginmedia.com>
  1 sibling, 1 reply; 43+ messages in thread
From: Roman Perepelitsa @ 2020-06-05  4:38 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers, Martin Tournoij

On Fri, Jun 5, 2020 at 4:07 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> Roman Perepelitsa wrote on Thu, 04 Jun 2020 07:41 +0200:
>
> > Or how to implement clobber-if-empty without ever clobbering non-empty
> > files?
>
> By using a non-clobbering open(), then calling fstat() on the open fd.
> If it's zero length, just carry on.  If it's not zero length, close()
> it and report an error.

Consider the following program:

    write() print -rn -- $1
    rm -f foo
    write hello >foo &
    write bye >foo &
    wait

With regular no_clobber it has the following guarantees:

1. `write` is executed exactly once
2. once `wait` completes, `foo` contains either "hello" or "bye"

Is there a way to provide these guarantees with clobber_empty? (2) is
especially important. Seeing "byelo" or "hellobye" in `foo` would be
quite confusing. It would look like clobbering.

Roman.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-05  3:10             ` Daniel Shahaf
  2020-06-05  3:18               ` Daniel Shahaf
@ 2020-06-06  1:07               ` Bart Schaefer
  2020-06-06  4:48                 ` Daniel Shahaf
  1 sibling, 1 reply; 43+ messages in thread
From: Bart Schaefer @ 2020-06-06  1:07 UTC (permalink / raw)
  To: zsh-workers; +Cc: Martin Tournoij

(Martin, if you are no longer interested in these side discussions, we
can stop Cc'ing you.)

On Thu, Jun 4, 2020 at 8:10 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> Bart Schaefer wrote on Wed, 03 Jun 2020 22:00 -0700:
> > Fair enough, although (silly example) "false > empty" would remove the
> > file?
>
> Depends on whether ./empty was created by the redirection or not.

Hm.  That means that the following must all be preserved (in the
parent shell, so fails for "exec command > file") all the way from the
redirection event to the completion of the command:
 - whether the descriptor resulted from redirection
 - what kind of redirection operator was used
 - whether the appropriate clobber-related option was set at the time
 - how to identify the file we opened, in case something else renamed
or removed it, and created another one of the same name in the
meantime

That seems at least impractical, especially the last one.  And what
happens in the case of rename?

> Besides, if the redirection failed — I assume you mean the open(2)
> failed — then isn't the unlink likely to fail as well?

Open could fail because of file permissions.  Unlink depends only on
directory permissions.

> Okay.  Is there a case where leaving around an empty file is worse than
> removing it?

IMO your examples are all too specific to warrant a generalized change
in behavior of redirections.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-05  4:38           ` Roman Perepelitsa
@ 2020-06-06  1:41             ` Bart Schaefer
  2020-06-06  4:55               ` Daniel Shahaf
  0 siblings, 1 reply; 43+ messages in thread
From: Bart Schaefer @ 2020-06-06  1:41 UTC (permalink / raw)
  To: zsh-workers; +Cc: Martin Tournoij

On Thu, Jun 4, 2020 at 9:39 PM Roman Perepelitsa
<roman.perepelitsa@gmail.com> wrote:
>
>     write() print -rn -- $1
>     rm -f foo
>     write hello >foo &
>     write bye >foo &
>     wait
>
> With regular no_clobber it has the following guarantees:
>
> 1. `write` is executed exactly once
> 2. once `wait` completes, `foo` contains either "hello" or "bye"

I don't think that's the intended typical usage of noclobber.  It's
not set by default, and it can't have any effect outside the local
shell.  It's meant to keep you from making silly mistakes when
interacting with the interpreter, not as a concurrent programming
tool.

Nevertheless:

> Is there a way to provide these guarantees with clobber_empty?

With clobber, you have #2, but not #1, so I think we should focus on
whether clobber_empty can guarantee #2.  If you want #1, there are
other ways (but I suspect it arises from the implementation of #2 even
so).

I think the answer is that #2 can be guaranteed if and only if
fcntl()-based locking can be applied, or some equivalent kernel
mechanism such that no more than closing of the (last dup of the)
descriptor is necessary to release the lock.  The steps would have to
be:

1. Open the file for write without truncate.
2. Attempt lock, non-blocking, and immediately close/fail if not locked.
3. Check for zero size, and immediately unlock/close/fail when nonzero.
4. Return the locked descriptor.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-06  1:07               ` Bart Schaefer
@ 2020-06-06  4:48                 ` Daniel Shahaf
  2020-06-06  7:04                   ` Bart Schaefer
  0 siblings, 1 reply; 43+ messages in thread
From: Daniel Shahaf @ 2020-06-06  4:48 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: zsh-workers, Martin Tournoij

Bart Schaefer wrote on Fri, 05 Jun 2020 18:07 -0700:
> (Martin, if you are no longer interested in these side discussions, we
> can stop Cc'ing you.)
> 
> On Thu, Jun 4, 2020 at 8:10 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> >
> > Bart Schaefer wrote on Wed, 03 Jun 2020 22:00 -0700:  
> > > Fair enough, although (silly example) "false > empty" would remove the
> > > file?  
> >
> > Depends on whether ./empty was created by the redirection or not.  
> 
> Hm.  That means that the following must all be preserved (in the
> parent shell, so fails for "exec command > file") all the way from the
> redirection event to the completion of the command:
>  - whether the descriptor resulted from redirection
>  - what kind of redirection operator was used
>  - whether the appropriate clobber-related option was set at the time
>  - how to identify the file we opened, in case something else renamed
> or removed it, and created another one of the same name in the
> meantime
> 
> That seems at least impractical, especially the last one.

It'll just be a couple of local variables on the C stack, won't it?

For the last bullet, I suppose we could save the inode number (and
device number), but as I wrote in workers/45977, I don't see a way to
avoid a race condition.

> And what happens in the case of rename?

If the file has been renamed before we get around to unlink it, then we
won't unlink it.  We won't have a choice.

> > Besides, if the redirection failed — I assume you mean the open(2)
> > failed — then isn't the unlink likely to fail as well?  
> 
> Open could fail because of file permissions.  Unlink depends only on
> directory permissions.
> 

Well, yes, but usually, if you don't have permission to read a file,
in practice you won't have permission to delete it either.

> > Okay.  Is there a case where leaving around an empty file is worse than
> > removing it?  
> 
> IMO your examples are all too specific to warrant a generalized change
> in behavior of redirections.

Fair enough.  As I said, just brainstorming.

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-06  1:41             ` Bart Schaefer
@ 2020-06-06  4:55               ` Daniel Shahaf
  2020-06-06  6:25                 ` Roman Perepelitsa
  2020-06-06  7:08                 ` Bart Schaefer
  0 siblings, 2 replies; 43+ messages in thread
From: Daniel Shahaf @ 2020-06-06  4:55 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: zsh-workers, Martin Tournoij

Bart Schaefer wrote on Fri, 05 Jun 2020 18:41 -0700:
> On Thu, Jun 4, 2020 at 9:39 PM Roman Perepelitsa
> <roman.perepelitsa@gmail.com> wrote:
> >
> >     write() print -rn -- $1
> >     rm -f foo
> >     write hello >foo &
> >     write bye >foo &
> >     wait
> >
> > With regular no_clobber it has the following guarantees:
> >
> > 1. `write` is executed exactly once
> > 2. once `wait` completes, `foo` contains either "hello" or "bye"  
> 
> I don't think that's the intended typical usage of noclobber.  It's
> not set by default, and it can't have any effect outside the local
> shell.  It's meant to keep you from making silly mistakes when
> interacting with the interpreter, not as a concurrent programming
> tool.

NO_CLOBBER causes open() to be called with the O_EXCL bit, which does
affect other processes as well.

> Nevertheless:
> 
> > Is there a way to provide these guarantees with clobber_empty?  
> 

I don't understand the question, really.  If you can get these
semantics with NO_CLOBBER, why does it matter whether or not
CLOBBER_EMPTY can provide them or not?  You can always leave it off (or
locally unset it).

Or use mkdir without the -p flag as a poor man's mutex.

> With clobber, you have #2, but not #1,

Doesn't O_EXCL achieve #1?

> so I think we should focus on
> whether clobber_empty can guarantee #2.  If you want #1, there are
> other ways (but I suspect it arises from the implementation of #2 even
> so).
> 
> I think the answer is that #2 can be guaranteed if and only if
> fcntl()-based locking can be applied, or some equivalent kernel
> mechanism such that no more than closing of the (last dup of the)
> descriptor is necessary to release the lock.  The steps would have to
> be:
> 
> 1. Open the file for write without truncate.
> 2. Attempt lock, non-blocking, and immediately close/fail if not locked.
> 3. Check for zero size, and immediately unlock/close/fail when nonzero.
> 4. Return the locked descriptor.

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-06  4:55               ` Daniel Shahaf
@ 2020-06-06  6:25                 ` Roman Perepelitsa
  2020-06-06  7:08                 ` Bart Schaefer
  1 sibling, 0 replies; 43+ messages in thread
From: Roman Perepelitsa @ 2020-06-06  6:25 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Bart Schaefer, zsh-workers, Martin Tournoij

On Sat, Jun 6, 2020 at 6:56 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> > On Thu, Jun 4, 2020 at 9:39 PM Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote:
> >
> > Is there a way to provide these guarantees with clobber_empty?
>
> I don't understand the question, really.

If someone writes "hello" and "bye" to the same file with no_clobber
and clobber_empty, and no error occurs (apart from no_clobber kicking
in and refusing to clobber), what can be the content of the file in a
conformant zsh implementation? I think providing an answer to this
question is important. Without it it's unclear what no_clobber with
clobber_empty means.

If writes happen sequentially, the file will contain either "hello" or
"bye". It cannot contain anything else. If the same guarantee could be
provided for concurrent writes, that would be ideal. I don't think
it's possible on all systems that zsh targets but maybe I'm wrong. On
some systems there might be no better answer than "the content of the
file is unspecified". This can be counterintuitive. Seeing "byelo" or
"hellobye" in the file one could reasonably assume that "hello" was
clobbered by "bye" despite options that on the surface disallow
clobbering non-empty files.

On Sat, Jun 6, 2020 at 3:42 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
> I think the answer is that #2 can be guaranteed if and only if
> fcntl()-based locking can be applied, or some equivalent kernel
> mechanism such that no more than closing of the (last dup of the)
> descriptor is necessary to release the lock.

Nod. It would also guarantee #1.

Roman.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-06  4:48                 ` Daniel Shahaf
@ 2020-06-06  7:04                   ` Bart Schaefer
  0 siblings, 0 replies; 43+ messages in thread
From: Bart Schaefer @ 2020-06-06  7:04 UTC (permalink / raw)
  To: zsh-workers; +Cc: Martin Tournoij

On Fri, Jun 5, 2020 at 9:48 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> Bart Schaefer wrote on Fri, 05 Jun 2020 18:07 -0700:
> >  - how to identify the file we opened, in case something else renamed
> > or removed it, and created another one of the same name in the
> > meantime
> >
> > That seems at least impractical, especially the last one.
>
> It'll just be a couple of local variables on the C stack, won't it?

That would only work for jobs that run synchronously.  It has to be in
the fdtable or the job table or something for background jobs and
pipelines.

> > Open could fail because of file permissions.  Unlink depends only on
> > directory permissions.
>
> Well, yes, but usually, if you don't have permission to read a file,
> in practice you won't have permission to delete it either.

I think you mean "write" there, but actually you might be able to
write to an empty file that you can't delete, but not create a file
that you can't delete.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-06  4:55               ` Daniel Shahaf
  2020-06-06  6:25                 ` Roman Perepelitsa
@ 2020-06-06  7:08                 ` Bart Schaefer
  2020-06-06  8:03                   ` Daniel Shahaf
  1 sibling, 1 reply; 43+ messages in thread
From: Bart Schaefer @ 2020-06-06  7:08 UTC (permalink / raw)
  To: zsh-workers; +Cc: Martin Tournoij

On Fri, Jun 5, 2020 at 9:55 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> Bart Schaefer wrote on Fri, 05 Jun 2020 18:41 -0700:
> >
> > I don't think that's the intended typical usage of noclobber.  It's
> > not set by default, and it can't have any effect outside the local
> > shell.
>
> NO_CLOBBER causes open() to be called with the O_EXCL bit, which does
> affect other processes as well.

??  All that O_EXCL guarantees is that no other process is able to
create the file.  Once it exists, another file can open it and
truncate it and write to it, unless prevented by umask.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-06  7:08                 ` Bart Schaefer
@ 2020-06-06  8:03                   ` Daniel Shahaf
  0 siblings, 0 replies; 43+ messages in thread
From: Daniel Shahaf @ 2020-06-06  8:03 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: zsh-workers, Martin Tournoij

Bart Schaefer wrote on Sat, 06 Jun 2020 00:08 -0700:
> On Fri, Jun 5, 2020 at 9:55 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> >
> > Bart Schaefer wrote on Fri, 05 Jun 2020 18:41 -0700:  
> > >
> > > I don't think that's the intended typical usage of noclobber.  It's
> > > not set by default, and it can't have any effect outside the local
> > > shell.  
> >
> > NO_CLOBBER causes open() to be called with the O_EXCL bit, which does
> > affect other processes as well.  
> 
> ??  All that O_EXCL guarantees is that no other process is able to
> create the file.  Once it exists, another file can open it and
> truncate it and write to it, unless prevented by umask.

But in Roman's example, _both_ backgrouned processes use O_EXCL, so
whichever process loses the race will get an error when it calls
open(2).  Yes, attempts to open() the file without O_EXCL will succeed,
but when a file is used as a mutex, everyone who tries to open the file
will try to use O_EXCL.

Makes sense?

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
       [not found]             ` <e7f7dfe2-eb4a-457b-85fb-091935a74c0e@www.fastmail.com>
@ 2020-06-06 11:57               ` Peter Stephenson
  2020-06-06 12:48                 ` Roman Perepelitsa
  2020-06-06 15:09                 ` Bart Schaefer
  0 siblings, 2 replies; 43+ messages in thread
From: Peter Stephenson @ 2020-06-06 11:57 UTC (permalink / raw)
  To: Zsh hackers list

On Sat, 2020-06-06 at 00:53 +0000, Daniel Shahaf wrote:
> Peter Stephenson wrote on Fri, 05 Jun 2020 12:41 +00:00:
> > > On 05 June 2020 at 03:07 Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > > Roman Perepelitsa wrote on Thu, 04 Jun 2020 07:41 +0200:
> > > > The regular noclobber can use O_CREAT | O_EXCL to avoid races. How
> > > > would any of the proposed modifications work? That is, how to
> > > > implement unlink-if-empty without ever unlinking non-empty files?
> > > 
> > > I don't think that's possible: it'd require an atomic
> > > stat-and-conditionally-unlink operation, and I don't think there's
> > > a syscall for this.  fstatat(2)-then-unlinkat(2) gets close, but it's
> > > still racy.
> > > 
> > > Good point.
> > 
> > I think we'd just have to document that this option doesn't provide
> > guarantees against asynchronous file access.  It wasn't requested
> > in order to implement standard behaviour, and we're not changing
> > the existing behaviour without the new option (I think we're mostly
> > agreed), and zsh already has so many options to sanitise if you do
> > want standard behaviour I don't think adding another is a big deal
> > --- assuming, of course, it actually is generally useful.
> > 
> > For what it's worth, I don't think it would have occurred to me
> > that there might be a race-free way of ensuring a file was empty on
> > opening, but the discussion has been interesting anyway.
> E
> [ Was this intentionally offlist?  Quoting without trimming; feel free
> to re-Cc the list on reply. ]

This was indeed intended to go to everyone.

> I'm not sure I understand.  As said on list, CLOBBER_EMPTY _can_ easily
> be implemented without races, using open()-then-fstat(); whereas
> UNLINK_EMPTY_AFTER_FAILURE cannot be implemented without races, because
> there's no fstatat()-then-unlinkat() syscall that's atomic against
> concurrent unlink()/creat() calls affecting the directory in question.
> Thus, I don't quite follow what the rationale is for _not_ implementing
> CLOBBER_EMPTY atomically.
> 
> Would you elaborate on the rationale, please?

My understanding of the semantics (feel free to put me right, of
course), is that as any use of fstat() would be on a non-clobbering
open, you can't stop anyone else writing to the file (the same file,
rahter than a newly created one with the same one) at any point.  Worse,
you're now claiming you've clobbered that open file, and you haven't.
So now you have Roman's nightmare scneario where you're claiming you've
opened a new file because the old one was empty, but actually two
processes have written to it.  Not only have you not fixed a race,
you've created a new one.

Assuming both processes are actually doing clobbering opens (O_CREAT |
O_EXCL), you can't get into that position.  You can't guarantee that
someone else hasn't written to a file of that name, but you can
guarantee that two so-called clobbering opens actually are that.

This is without using file locking --- I trust we don't want to go down
that route.  I mention it because (this is a key point, so if I've got
it wrong the whole argument falls apart, but I think Bart is saying the
same thing) just using normal system calls without locking there's no
way of stopping multiple processes opening existing files for writing.

pws


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-06 11:57               ` Peter Stephenson
@ 2020-06-06 12:48                 ` Roman Perepelitsa
  2020-06-06 15:24                   ` Bart Schaefer
  2020-06-06 15:09                 ` Bart Schaefer
  1 sibling, 1 reply; 43+ messages in thread
From: Roman Perepelitsa @ 2020-06-06 12:48 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

On Sat, Jun 6, 2020 at 1:58 PM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> My understanding of the semantics (feel free to put me right, of
> course), is that as any use of fstat() would be on a non-clobbering
> open, you can't stop anyone else writing to the file (the same file,
> rahter than a newly created one with the same one) at any point.  Worse,
> you're now claiming you've clobbered that open file, and you haven't.
> So now you have Roman's nightmare scneario where you're claiming you've
> opened a new file because the old one was empty, but actually two
> processes have written to it.  Not only have you not fixed a race,
> you've created a new one.

Yep. Having unspecified file content that is the result of interleaved
writes seems pretty bad.

> Assuming both processes are actually doing clobbering opens (O_CREAT |
> O_EXCL), you can't get into that position.  You can't guarantee that
> someone else hasn't written to a file of that name, but you can
> guarantee that two so-called clobbering opens actually are that.

Let me put this in code:

  { print hello >file } &
  hello_pid=$!

  { print bye >file } &
  bye_pid=$!

  { print -n >file } &
  empty_pid=$!

  wait $hello_pid
  print "print hello => $?"

  wait $bye_pid
  print "print bye => $?"

  wait $empty_pid
  print "print -n => $?"

  print "file => $(<file)"

This program concurrently writes "hello", "bye" and "" (empty) to the
same file. Any implementation of clobber_empty that uses just POSIX C
API will allow multiple writes to succeed, resulting in this output:

  print hello => 0
  print bye => 0

This output implies that one command has overwritten the output of
another and the user has received no indication to this effect.

In addition, no implementation restricted to POSIX C API can guarantee
that the last line of the output is one of these:

  file => hello
  file => bye

Different implementations will violate this expectation in different
ways. Some will allow abominations such as "byelo" or "hellobye". If I
understand Peter's point correctly, his preferred implementation will
guarantee that the file exists and contains either "hello", "bye" or
"" (empty). The possibility of an empty file is unfortunate. In
addition, this implementation requires unlinking files (when the file
exists prior to a write and is empty), which may be prohibited while
writes to the file are allowed. Unlinking also introduces the
possibility of losing files, together with their permissions and
ownership, which could've been informative. This happens when one of
the writes gets interrupted or fails to create a file after unlinking.

All these issues can be fixed through the use of advisory locks on
systems that support them.

Roman.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-06 11:57               ` Peter Stephenson
  2020-06-06 12:48                 ` Roman Perepelitsa
@ 2020-06-06 15:09                 ` Bart Schaefer
  1 sibling, 0 replies; 43+ messages in thread
From: Bart Schaefer @ 2020-06-06 15:09 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

On Sat, Jun 6, 2020 at 4:58 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> Assuming both processes are actually doing clobbering opens (O_CREAT |
> O_EXCL), you can't get into that position.

Just to clarify:  O_CREAT|O_EXCL is actually the NO_clobber-ing open.
The clobbering open is just to open it for writing if allowed to. and
start scribbling over whatever is there.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-06 12:48                 ` Roman Perepelitsa
@ 2020-06-06 15:24                   ` Bart Schaefer
  2020-06-06 16:24                     ` Peter Stephenson
  0 siblings, 1 reply; 43+ messages in thread
From: Bart Schaefer @ 2020-06-06 15:24 UTC (permalink / raw)
  To: Roman Perepelitsa; +Cc: Peter Stephenson, Zsh hackers list

On Sat, Jun 6, 2020 at 5:49 AM Roman Perepelitsa
<roman.perepelitsa@gmail.com> wrote:
>
> Having unspecified file content that is the result of interleaved
> writes seems pretty bad.
>
> [... code example ...]
>
> This output implies that one command has overwritten the output of
> another and the user has received no indication to this effect.

This returns to my point that this isn't the intended use case for
either NO_CLOBBER or CLOBBER_EMPTY.  That code example always yields
unspecified file contents in a POSIX shell if the empty file already
exists and is writable (the only case you couldn't have is the file
empty at the end).

If you're going to use zsh + noclobber for concurrent programming,
then the implementation of clobberempty is entirely irrelevant,
because your correct program won't use it.

If you're attempting to use zsh + noclobber defensively, that won't
work either because any other process not written in zsh won't pay
attention to it.

Consequently we should be looking at this as an entirely interactive
user DWIM feature, and choosing how to implement (or not to) based on
that.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-06 15:24                   ` Bart Schaefer
@ 2020-06-06 16:24                     ` Peter Stephenson
  2020-06-07 11:55                       ` Daniel Shahaf
  0 siblings, 1 reply; 43+ messages in thread
From: Peter Stephenson @ 2020-06-06 16:24 UTC (permalink / raw)
  To: zsh-workers

On Sat, 2020-06-06 at 08:24 -0700, Bart Schaefer wrote:
> Just to clarify:  O_CREAT|O_EXCL is actually the NO_clobber-ing open.
> The clobbering open is just to open it for writing if allowed to. and
> start scribbling over whatever is there.

You're right; in the hypothesised case, the file already exists and
this pair of options is documented to fail in that case.

I think the O_TRUNC is therefore the crucial one in our normal clobbering
open, right?

    /* If clobbering, just open. */
    if (isset(CLOBBER) || IS_CLOBBER_REDIR(f->type))
	return open(unmeta(f->name),
		O_WRONLY | O_CREAT | O_TRUNC | O_NOCTTY, 0666);

We'll truncate at that point, then start writing.  That's the best
we can do.  If anyone else is also writing to at *at that point*
--- out of luck, only file locking will help.

> Consequently we should be looking at this as an entirely interactive
> user DWIM feature, and choosing how to implement (or not to) based on
> that.

Yes, I certainly agree with that.

pws




^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-06 16:24                     ` Peter Stephenson
@ 2020-06-07 11:55                       ` Daniel Shahaf
  2020-06-07 17:00                         ` Peter Stephenson
  2020-06-07 17:20                         ` Bart Schaefer
  0 siblings, 2 replies; 43+ messages in thread
From: Daniel Shahaf @ 2020-06-07 11:55 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: zsh-workers

Peter Stephenson wrote on Sat, 06 Jun 2020 17:24 +0100:
> On Sat, 2020-06-06 at 08:24 -0700, Bart Schaefer wrote:
> > Just to clarify:  O_CREAT|O_EXCL is actually the NO_clobber-ing open.
> > The clobbering open is just to open it for writing if allowed to. and
> > start scribbling over whatever is there.  
> 
> You're right; in the hypothesised case, the file already exists and
> this pair of options is documented to fail in that case.
> 
> I think the O_TRUNC is therefore the crucial one in our normal clobbering
> open, right?
> 
>     /* If clobbering, just open. */
>     if (isset(CLOBBER) || IS_CLOBBER_REDIR(f->type))
> 	return open(unmeta(f->name),
> 		O_WRONLY | O_CREAT | O_TRUNC | O_NOCTTY, 0666);
> 
> We'll truncate at that point, then start writing.  That's the best
> we can do.  If anyone else is also writing to at *at that point*
> --- out of luck, only file locking will help.

In the clobbering open, we use O_CREAT|O_TRUNC, which means: if it
doesn't exist, create it; if it already exists, truncate it.  If some
other process calls open() on the same file immediately after our
open() [or subsequent write()] call returns, there's nothing we can do
about that.

In a regular NO_CLOBBER open, we use O_EXCL, which causes open() to fail
if the file already exists.  In that case, too, if some other process
tries to open the file *without O_EXCL* at any point after our open()
returns successfully, the other process's open() will succeed and may
overwrite our changes, and there's nothing we can do about that.
(On the other hand, if two processes use open(O_EXCL) on the same
filename, at least one of them will get an error.)

As to CLOBBER_EMPTY, the situation is similar.  We can call open() with
O_CREAT but neither O_EXCL nor O_TRUNC and then fstat() the resulting
file.  The same race conditions as in the previous paragraphs exist
here too — for example, someone could open() and write() to the file
after we fstat() it — but we neither can nor need to do any better
about races in this case than in the others.

I agree that we don't want to do file locking here.  Anyone who wants
that can use «zsystem flock».

My point here is really just the one I already made in 45976, and
wasn't answered there: can't we avoid the close()-then-open() sequence
that 45968 does?  That one seems to be an _avoidable_ race condition,
unlike the above ones.

> > Consequently we should be looking at this as an entirely interactive
> > user DWIM feature, and choosing how to implement (or not to) based on
> > that.  
> 
> Yes, I certainly agree with that.
> 
> pws

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-07 11:55                       ` Daniel Shahaf
@ 2020-06-07 17:00                         ` Peter Stephenson
  2020-06-08  3:27                           ` Daniel Shahaf
  2020-06-07 17:20                         ` Bart Schaefer
  1 sibling, 1 reply; 43+ messages in thread
From: Peter Stephenson @ 2020-06-07 17:00 UTC (permalink / raw)
  To: zsh-workers

On Sun, 2020-06-07 at 11:55 +0000, Daniel Shahaf wrote:
> My point here is really just the one I already made in 45976, and
> wasn't answered there: can't we avoid the close()-then-open() sequence
> that 45968 does?  That one seems to be an _avoidable_ race condition,
> unlike the above ones.

I think what it boils down to here is either you test using fstat() the
the file is empty, or you re-open using O_TRUNC, both of which ensure
the file is empty at that point.  Then at some later point, the file
will be written to.  Between the two it's there but empty, which is
unavoidable.  So my last change, closing and opening with O_TRUNC,
doesn't really gain anything over leaving it open after checking the
size with fstat(), I don't think, in which case the extra open() is
redundant and best removed (and that puts us in the fortuitous position
where we haven't actually added any system calls in adding
CLOBBER_EMPTY).  But I don't think taking it out actually removes a
race.

pws


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-07 11:55                       ` Daniel Shahaf
  2020-06-07 17:00                         ` Peter Stephenson
@ 2020-06-07 17:20                         ` Bart Schaefer
  1 sibling, 0 replies; 43+ messages in thread
From: Bart Schaefer @ 2020-06-07 17:20 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Peter Stephenson, zsh-workers

On Sun, Jun 7, 2020 at 4:55 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> In the clobbering open, we use O_CREAT|O_TRUNC, which means: if it
> doesn't exist, create it; if it already exists, truncate it.  If some
> other process calls open() on the same file immediately after our
> open() [or subsequent write()] call returns, there's nothing we can do
> about that.

Also if another process already had the existing file open, whatever
it was doing will be lost.  (Aside:  If it's an NFS file, and the
other process continues writing, this often results in a block of NUL
bytes where the NFS server fills in between the two write positions.)

> As to CLOBBER_EMPTY, the situation is similar.  We can call open() with
> O_CREAT but neither O_EXCL nor O_TRUNC and then fstat() the resulting
> file.  The same race conditions as in the previous paragraphs exist
> here too — for example, someone could open() and write() to the file
> after we fstat() it — but we neither can nor need to do any better
> about races in this case than in the others.

This in fact avoids (or at least does a better job of avoiding) the
problem with two different write positions if some other process has
already opened the file for write.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-07 17:00                         ` Peter Stephenson
@ 2020-06-08  3:27                           ` Daniel Shahaf
  2020-06-08  9:30                             ` Peter Stephenson
  0 siblings, 1 reply; 43+ messages in thread
From: Daniel Shahaf @ 2020-06-08  3:27 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: zsh-workers

Peter Stephenson wrote on Sun, 07 Jun 2020 18:00 +0100:
> On Sun, 2020-06-07 at 11:55 +0000, Daniel Shahaf wrote:
> > My point here is really just the one I already made in 45976, and
> > wasn't answered there: can't we avoid the close()-then-open() sequence
> > that 45968 does?  That one seems to be an _avoidable_ race condition,
> > unlike the above ones.  
> 
> I think what it boils down to here is either you test using fstat() the
> the file is empty, or you re-open using O_TRUNC, both of which ensure
> the file is empty at that point.  Then at some later point, the file
> will be written to.  Between the two it's there but empty, which is
> unavoidable.

*nod*

> So my last change, closing and opening with O_TRUNC,
> doesn't really gain anything over leaving it open after checking the
> size with fstat(), I don't think, in which case the extra open() is
> redundant and best removed (and that puts us in the fortuitous position
> where we haven't actually added any system calls in adding
> CLOBBER_EMPTY).  But I don't think taking it out actually removes a
> race.

The race is:

1. zsh runs «: > existing-empty-file».
2. zsh calls open(O_EXCL) which fails, then open() and fstat(), which both succeeds.
3. Some other process writes data to the file.
4. zsh opens the file with O_TRUNC.
5. bin_true() returns, not having written anything to the file.
6. zsh closes the file.

Step #4 will have deleted the other process's data.  If we remove the
close()/open() pair, the step #4 data will survive.

It's an edge case, yes.

Cheers,

Daniel

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: Any way to allow clobbering empty files when noclobber is set?
  2020-06-08  3:27                           ` Daniel Shahaf
@ 2020-06-08  9:30                             ` Peter Stephenson
  0 siblings, 0 replies; 43+ messages in thread
From: Peter Stephenson @ 2020-06-08  9:30 UTC (permalink / raw)
  To: zsh-workers

Here's an updated patch, which I think incorporates the
current thinking, which seems pretty much to have iterated
to a conclusion.  The effective change is in fact now
trivial --- an "if" with a return.

Webmail again, as it seemed pretty non-invasive last time...

pws

diff --git a/Doc/Zsh/options.yo b/Doc/Zsh/options.yo
index 2b7637ff4..6da68308f 100644
--- a/Doc/Zsh/options.yo
+++ b/Doc/Zsh/options.yo
@@ -1168,6 +1168,22 @@ If the option is not set, and the option tt(APPEND_CREATE) is also
 not set, `tt(>>!)' or `tt(>>|)' must be used to create a file.
 If either option is set, `tt(>>)' may be used.
 )
+pindex(CLOBBER_EMPTY)
+pindex(NO_CLOBBER_EMPTY)
+pindex(CLOBBEREMPTY)
+pindex(NOCLOBBEREMPTY)
+cindex(clobbering, of empty files)
+cindex(file clobbering, of empty files)
+item(tt(CLOBBER_EMPTY))(
+This option is only used if the option tt(CLOBBER) is not set: note that
+it is set by default.
+
+If this option is set, then regular files of zero length may be
+ovewritten (`clobbered').  Note that it is possible another process
+has written to the file between this test and use of the file by
+the current process.  This option should therefore not be used in
+cases where files to be clobbered may be written to asynchronously.
+)
 pindex(CORRECT)
 pindex(NO_CORRECT)
 pindex(NOCORRECT)
diff --git a/Src/exec.c b/Src/exec.c
index 29f4fc5ca..7b087f3b0 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2143,14 +2143,15 @@ clobber_open(struct redir *f)
 {
     struct stat buf;
     int fd, oerrno;
+    char *ufname = unmeta(f->name);
 
     /* If clobbering, just open. */
     if (isset(CLOBBER) || IS_CLOBBER_REDIR(f->type))
-	return open(unmeta(f->name),
+	return open(ufname,
 		O_WRONLY | O_CREAT | O_TRUNC | O_NOCTTY, 0666);
 
     /* If not clobbering, attempt to create file exclusively. */
-    if ((fd = open(unmeta(f->name),
+    if ((fd = open(ufname,
 		   O_WRONLY | O_CREAT | O_EXCL | O_NOCTTY, 0666)) >= 0)
 	return fd;
 
@@ -2158,11 +2159,27 @@ clobber_open(struct redir *f)
      * Try opening, and if it's a regular file then close it again    *
      * because we weren't supposed to open it.                        */
     oerrno = errno;
-    if ((fd = open(unmeta(f->name), O_WRONLY | O_NOCTTY)) != -1) {
-	if(!fstat(fd, &buf) && !S_ISREG(buf.st_mode))
-	    return fd;
+    if ((fd = open(ufname, O_WRONLY | O_NOCTTY)) != -1) {
+	if(!fstat(fd, &buf)) {
+	    if (!S_ISREG(buf.st_mode))
+		return fd;
+	    /*
+	     * If CLOBBER_EMPTY is in effect and the file is empty,
+	     * we are allowed to re-use it.
+	     *
+	     * Note: there is an intrinsic race here because another
+	     * process can write to this file at any time.  The only fix
+	     * would be file locking, which we wish to avoid in basic
+	     * file operations at this level.  This would not be
+	     * fixed. just additionally complicated, by re-opening the
+	     * file and truncating.
+	     */
+	    if (isset(CLOBBEREMPTY) && buf.st_size == 0)
+		return fd;
+	}
 	close(fd);
     }
+
     errno = oerrno;
     return -1;
 }
diff --git a/Src/options.c b/Src/options.c
index 7586d21d2..fba021e7d 100644
--- a/Src/options.c
+++ b/Src/options.c
@@ -114,6 +114,7 @@ static struct optname optns[] = {
 {{NULL, "checkjobs",	      OPT_EMULATE|OPT_ZSH},	 CHECKJOBS},
 {{NULL, "checkrunningjobs",   OPT_EMULATE|OPT_ZSH},	 CHECKRUNNINGJOBS},
 {{NULL, "clobber",	      OPT_EMULATE|OPT_ALL},	 CLOBBER},
+{{NULL, "clobberempty",	      0},			 CLOBBEREMPTY},
 {{NULL, "combiningchars",     0},			 COMBININGCHARS},
 {{NULL, "completealiases",    0},			 COMPLETEALIASES},
 {{NULL, "completeinword",     0},			 COMPLETEINWORD},
diff --git a/Src/zsh.h b/Src/zsh.h
index 1f2d774a1..ed123f2b9 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -2378,6 +2378,7 @@ enum {
     CHECKJOBS,
     CHECKRUNNINGJOBS,
     CLOBBER,
+    CLOBBEREMPTY,
     APPENDCREATE,
     COMBININGCHARS,
     COMPLETEALIASES,
diff --git a/Test/A04redirect.ztst b/Test/A04redirect.ztst
index d60519064..993138e7d 100644
--- a/Test/A04redirect.ztst
+++ b/Test/A04redirect.ztst
@@ -708,3 +708,17 @@
   cat <&$testfd
 0:Regression test for here document with fd declarator
 >  This is, in some sense, a here document.
+
+  (setopt noclobber clobberempty
+  rm -f foo
+  touch foo
+  print Works >foo
+  cat foo
+  print Works not >foo
+  # Make sure the file was not harmed
+  cat foo
+  )
+0:CLOBBER_EMPTY
+>Works
+>Works
+?(eval):6: file exists: foo

^ permalink raw reply	[flat|nested] 43+ messages in thread

end of thread, other threads:[~2020-06-08  9:31 UTC | newest]

Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20200603020919eucas1p13e26ebcbb335784d14bfb97b137f385a@eucas1p1.samsung.com>
2020-06-03  2:08 ` Any way to allow clobbering empty files when noclobber is set? Martin Tournoij
2020-06-03 12:04   ` Peter Stephenson
2020-06-04  1:48     ` Daniel Shahaf
2020-06-04  2:43       ` Bart Schaefer
2020-06-04  4:06         ` Daniel Shahaf
2020-06-04  5:00           ` Bart Schaefer
2020-06-05  3:10             ` Daniel Shahaf
2020-06-05  3:18               ` Daniel Shahaf
2020-06-06  1:07               ` Bart Schaefer
2020-06-06  4:48                 ` Daniel Shahaf
2020-06-06  7:04                   ` Bart Schaefer
2020-06-04  6:31       ` Martin Tournoij
2020-06-05  2:22         ` Daniel Shahaf
2020-06-04  2:13     ` Vin Shelton
2020-06-04  2:35       ` Bart Schaefer
2020-06-04  2:36       ` Daniel Shahaf
2020-06-04 11:57         ` Vin Shelton
2020-06-04  5:06     ` Bart Schaefer
2020-06-04  5:41       ` Roman Perepelitsa
2020-06-05  2:07         ` Daniel Shahaf
2020-06-05  4:38           ` Roman Perepelitsa
2020-06-06  1:41             ` Bart Schaefer
2020-06-06  4:55               ` Daniel Shahaf
2020-06-06  6:25                 ` Roman Perepelitsa
2020-06-06  7:08                 ` Bart Schaefer
2020-06-06  8:03                   ` Daniel Shahaf
     [not found]           ` <1941572212.466119.1591360860372@mail2.virginmedia.com>
     [not found]             ` <e7f7dfe2-eb4a-457b-85fb-091935a74c0e@www.fastmail.com>
2020-06-06 11:57               ` Peter Stephenson
2020-06-06 12:48                 ` Roman Perepelitsa
2020-06-06 15:24                   ` Bart Schaefer
2020-06-06 16:24                     ` Peter Stephenson
2020-06-07 11:55                       ` Daniel Shahaf
2020-06-07 17:00                         ` Peter Stephenson
2020-06-08  3:27                           ` Daniel Shahaf
2020-06-08  9:30                             ` Peter Stephenson
2020-06-07 17:20                         ` Bart Schaefer
2020-06-06 15:09                 ` Bart Schaefer
2020-06-04  6:47       ` Martin Tournoij
2020-06-04  9:42       ` Peter Stephenson
2020-06-04 12:20         ` Roman Perepelitsa
2020-06-04 12:26           ` Peter Stephenson
2020-06-04 12:15     ` Peter Stephenson
2020-06-04 20:35       ` Bart Schaefer
2020-06-05  1:59         ` Daniel Shahaf

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

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 NNTP newsgroup(s).