'>>' does not create file if set -C (noclobber) is active

'>>' does not create file if set -C (noclobber) is active

[Martijn Dekker]

If the 'noclobber' option (set -C) is active, the append ('>>') output
redirection will not create a file if it doesn't exist.

% set -C
% echo hi >> blah
zsh: no such file or directory: blah

POSIX:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_07_03
"If the file does not exist, it shall be created." It does not mention
that the noclobber option should influence this.

By definition, a file that doesn't exist cannot be clobbered, so it
seems to me that logically this shell option shouldn't apply.

bash, (d)ash, ksh93, mksh, and yash all act like POSIX says.

I have no opinion on whether this should be an overall fix or a fix for
the emulation modes.

Thanks,

- Martijn

Re: '>>' does not create file if set -C (noclobber) is active

[Bart Schaefer]

On Jun 25,  3:02am, Martijn Dekker wrote:
}
} If the 'noclobber' option (set -C) is active, the append ('>>') output
} redirection will not create a file if it doesn't exist.
} 
} POSIX:

The noclobber option is an extension, and has nothing whatsoever to do
with POSIX.  It is not normally on in any emulation, so as soon as you
set it you're off in the weeds and appeal to POSIX is meaningless.

This is working the way it's intended to work.

Re: '>>' does not create file if set -C (noclobber) is active

[Martijn Dekker]

Bart Schaefer schreef op 25-06-15 om 03:49:
> On Jun 25,  3:02am, Martijn Dekker wrote:
> }
> } If the 'noclobber' option (set -C) is active, the append ('>>') output
> } redirection will not create a file if it doesn't exist.
> } 
> } POSIX:
> 
> The noclobber option is an extension, and has nothing whatsoever to do
> with POSIX.  It is not normally on in any emulation, so as soon as you
> set it you're off in the weeds and appeal to POSIX is meaningless.

That is not correct.

See:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_25

-C
    (Uppercase C.) Prevent existing files from being overwritten by the
shell's '>' redirection operator (see Redirecting Output); the ">|"
redirection operator shall override this noclobber option for an
individual file.

[...]

-o  option
[...]
noclobber
    Equivalent to -C (uppercase C).

Thanks,

- M.

Re: '>>' does not create file if set -C (noclobber) is active

[Bart Schaefer]

On Jun 25,  4:22am, Martijn Dekker wrote:
} Subject: Re: '>>' does not create file if set -C (noclobber) is active
}
} > The noclobber option is an extension, and has nothing whatsoever to do
} > with POSIX.
} 
} That is not correct.

Well, damn.  When did they do that?

As far as I know, this was first implemented in zsh and has worked as it
does in zsh since around 1990 -- there is no mention of the -C option in
the ksh88 manual, for example.  So I'm still inclined to say that this
is just not going to change, because we'd either have to (1) change it in
native mode or (2) do one of those hacks that tests the emulation mode
so it only works if you actually start the shell that way or (3) add an
entirely new setopt just for this.  I don't like any of those choices.

Re: '>>' does not create file if set -C (noclobber) is active

[Stephane Chazelas]

2015-06-25 00:30:47 -0700, Bart Schaefer:
> On Jun 25,  4:22am, Martijn Dekker wrote:
> } Subject: Re: '>>' does not create file if set -C (noclobber) is active
> }
> } > The noclobber option is an extension, and has nothing whatsoever to do
> } > with POSIX.
> } 
> } That is not correct.
> 
> Well, damn.  When did they do that?
> 
> As far as I know, this was first implemented in zsh and has worked as it
> does in zsh since around 1990 -- there is no mention of the -C option in
> the ksh88 manual, for example.  So I'm still inclined to say that this
> is just not going to change, because we'd either have to (1) change it in
> native mode or (2) do one of those hacks that tests the emulation mode
> so it only works if you actually start the shell that way or (3) add an
> entirely new setopt just for this.  I don't like any of those choices.

Though that's initially a csh feature (from the start: 1979),
AFAIK, noclobber has always been in ksh. But the -C alias may
have been added in ksh93 and retrofitted to ksh88. The ksh88a
(http://www.in-ulm.de/~mascheck/various/shells/ksh88-fixes)
release notes mention noclobber. The ksh93 release notes mention
adding -C as an alias for noclobber.

The original pdksh didn't have either. I don't know when they
were added.

It was added to bash in January 1990 but first as a variable.
1.12 (Dec 1991) had it as both set -C and set -o noclobber.

I suspect both were already in POSIX.2. It was added to ash in
4.4BSD-Alpha (1992):
http://www.in-ulm.de/~mascheck/various/ash/index.html#44bsdalpha

I don't think it was ever added to the Bourne shell though.

-- 
Stephane

Re: '>>' does not create file if set -C (noclobber) is active

[Bart Schaefer]

On Jun 25,  3:04pm, Stephane Chazelas wrote:
} Subject: Re: '>>' does not create file if set -C (noclobber) is active
}
} Though that's initially a csh feature (from the start: 1979),

Well, yeah, but the odds that POSIX adopted it from csh are pretty low.
Zsh adopted it from csh.

Thanks for the other summary, though.

Re: '>>' does not create file if set -C (noclobber) is active

[Chet Ramey]

On 6/25/15 12:00 PM, Bart Schaefer wrote:
> On Jun 25,  3:04pm, Stephane Chazelas wrote:
> } Subject: Re: '>>' does not create file if set -C (noclobber) is active
> }
> } Though that's initially a csh feature (from the start: 1979),
> 
> Well, yeah, but the odds that POSIX adopted it from csh are pretty low.
> Zsh adopted it from csh.

ksh88 picked up noclobber from csh, but used `set -o noclobber' and added
the >| redirection.  Posix.2 invented `set -C'.  Bash adopted noclobber
from ksh in 1990 and set -C/-o noclobber from a pre-1992 Posix.2 draft in
1991.

Chet
-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
		 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, ITS, CWRU    chet@case.edu    http://cnswww.cns.cwru.edu/~chet/

Re: '>>' does not create file if set -C (noclobber) is active

[Martijn Dekker]

Bart Schaefer schreef op 25-06-15 om 09:30:
> So I'm still inclined to say that this
> is just not going to change, because we'd either have to (1) change it in
> native mode or (2) do one of those hacks that tests the emulation mode
> so it only works if you actually start the shell that way or (3) add an
> entirely new setopt just for this.  I don't like any of those choices.

I can understand and sympathize with this point of view. But, from my
point of view as someone writing cross-platform POSIX shell programs,
this is a legitimate compliance bug that breaks scripts running under
'set -C' that should be expected to work according to the standard. (It
has done in at least one real-life case, i.e. mine). A shell-specific
workaround ought not to be necessary for this. If the goal of 'emulate
sh' is to provide a compatible POSIX environment then this situation is
clearly inconsistent with that goal. So I hope you're willing to
reconsider and think of a solution -- perhaps a SH_APPEND shell option
(which would also apply to ksh emulation).

Thanks,

- M.

Re: '>>' does not create file if set -C (noclobber) is active

[Bart Schaefer]

On Jun 26,  4:14pm, Martijn Dekker wrote:
}
} workaround ought not to be necessary for this. If the goal of 'emulate
} sh' is to provide a compatible POSIX environment then this situation is
} clearly inconsistent with that goal.

The goal of "emulate sh" is to provide a Bourne-sh compatible environment,
for the most part.

Just for example, neither POSIX_ARGZERO nor POSIX_CD is turned on by
default in any of the "emulate" modes, and there are a number of other
POSIX-isms that are not (yet, at least) supported at all.

That said, I'm not trying to shut down discussion of this issue, I'm just
pointing out that we're going to be choosing a least of evils whether we
do something or do nothing.

Re: '>>' does not create file if set -C (noclobber) is active

[Martijn Dekker]

Bart Schaefer schreef op 26-06-15 om 22:15:
> The goal of "emulate sh" is to provide a Bourne-sh compatible environment,
> for the most part.

I see. Since POSIX is now the lowest common denominator for the
overwhelming majority of Unix-like systems out there, I suppose I was
just assuming that was the intended target by now.

> Just for example, neither POSIX_ARGZERO nor POSIX_CD is turned on by
> default in any of the "emulate" modes, and there are a number of other
> POSIX-isms that are not (yet, at least) supported at all.

Hmm. On my system (zsh 5.0.8) POSIX_CD is in fact turned on by 'emulate
sh'. The only options I've found so far that aren't turned on, but
should be for POSIX compliance, are POSIX_ARGZERO (as you say), and
MULTIBYTE (but that was changed in git shortly after 5.0.8 was released).

So far, I've found that zsh 5.0.8 under 'emulate sh -o POSIX_ARGZERO -o
MULTIBYTE' is very close to a complete POSIX shell. I'd be very
interested to know exactly what other POSIX-isms are not yet supported
by zsh, so I can implement any necessary tests and workarounds in the
shell library I'm developing.

> That said, I'm not trying to shut down discussion of this issue, I'm just
> pointing out that we're going to be choosing a least of evils whether we
> do something or do nothing.

I understand. Thanks for your time and consideration in any case.

- Martijn

Re: '>>' does not create file if set -C (noclobber) is active

[Bart Schaefer]

On Jun 27,  3:54am, Martijn Dekker wrote:
}
} Hmm. On my system (zsh 5.0.8) POSIX_CD is in fact turned on by 'emulate
} sh'.

Documentation error, then.

diff --git a/Doc/Zsh/options.yo b/Doc/Zsh/options.yo
index b4a5e10..2371e35 100644
--- a/Doc/Zsh/options.yo
+++ b/Doc/Zsh/options.yo
@@ -124,7 +124,7 @@ pindex(POSIXCD)
 pindex(NO_POSIX_CD)
 pindex(NOPOSIXCD)
 cindex(CDPATH, order of checking)
-item(tt(POSIX_CD))(
+item(tt(POSIX_CD) <K> <S>)(
 Modifies the behaviour of tt(cd), tt(chdir) and tt(pushd) commands
 to make them more compatible with the POSIX standard. The behaviour with
 the option unset is described in the documentation for the tt(cd)

Re: '>>' does not create file if set -C (noclobber) is active

[Peter Stephenson]

This isn't very elegant, but nor is it obviously problematic, and it
does at least draw attention to a potential issue.

pws

diff --git a/Doc/Zsh/options.yo b/Doc/Zsh/options.yo
index 2371e35..6dd2239 100644
--- a/Doc/Zsh/options.yo
+++ b/Doc/Zsh/options.yo
@@ -1819,6 +1819,20 @@ Make the tt(echo) builtin compatible with the BSD manref(echo)(1) command.
 This disables backslashed escape sequences in echo strings unless the
 tt(-e) option is specified.
 )
+pindex(CLOBBER_APPEND)
+pindex(NO_CLOBBER_APPEND)
+pindex(CLOBBERAPPEND)
+pindex(NOCLOBBERAPPEND)
+cindex(clobbering, POSIX compatibility)
+cindex(file clobbering, POSIX compatibility)
+cindex(no clobber, POSIX compatible)
+item(tt(CLOBBER_APPEND) <K> <S>)(
+This option only applies when tt(NO_CLOBBER) (-tt(C)) is in effect.
+
+If this option is not set, the shell will report an error when a
+append redirection (tt(>>)) is used on a file that does not already
+exists.  If the option is set, no error is reported.
+)
 pindex(CONTINUE_ON_ERROR)
 pindex(NO_CONTINUE_ON_ERROR)
 pindex(CONTINUEONERROR)
diff --git a/Src/exec.c b/Src/exec.c
index 50a11eb..4ddfdf4 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -3315,7 +3315,8 @@ execcmd(Estate state, int input, int output, int how, int last1)
 		    fil = -1;
 		else if (IS_APPEND_REDIR(fn->type))
 		    fil = open(unmeta(fn->name),
-			       (unset(CLOBBER) && !IS_CLOBBER_REDIR(fn->type)) ?
+			       ((unset(CLOBBER) || isset(CLOBBERAPPEND)) &&
+				!IS_CLOBBER_REDIR(fn->type)) ?
 			       O_WRONLY | O_APPEND | O_NOCTTY :
 			       O_WRONLY | O_APPEND | O_CREAT | O_NOCTTY, 0666);
 		else
diff --git a/Src/options.c b/Src/options.c
index da3d830..3a0c838 100644
--- a/Src/options.c
+++ b/Src/options.c
@@ -110,6 +110,7 @@ static struct optname optns[] = {
 {{NULL, "chaselinks",	      OPT_EMULATE},		 CHASELINKS},
 {{NULL, "checkjobs",	      OPT_EMULATE|OPT_ZSH},	 CHECKJOBS},
 {{NULL, "clobber",	      OPT_EMULATE|OPT_ALL},	 CLOBBER},
+{{NULL, "clobberappend",      OPT_EMULATE|OPT_BOURNE},	 CLOBBER},
 {{NULL, "combiningchars",     0},			 COMBININGCHARS},
 {{NULL, "completealiases",    0},			 COMPLETEALIASES},
 {{NULL, "completeinword",     0},			 COMPLETEINWORD},
diff --git a/Src/zsh.h b/Src/zsh.h
index ce9b979..05efa5f 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -2120,6 +2120,7 @@ enum {
     CHASELINKS,
     CHECKJOBS,
     CLOBBER,
+    CLOBBERAPPEND,
     COMBININGCHARS,
     COMPLETEALIASES,
     COMPLETEINWORD,

Re: '>>' does not create file if set -C (noclobber) is active

[Martijn Dekker]

Peter Stephenson schreef op 27-06-15 om 19:02:
> This isn't very elegant, but nor is it obviously problematic, and it
> does at least draw attention to a potential issue.

I've tested this patch against the current git version and noticed the
following:

- The new clobberappend option is on by default, but 'set -C' (-o
noclobber) turns clobberappend off. Conversely, 'set +C' (+o noclobber)
turns it back on. This applies even when noclobber is already on or off.

- Contrary to the documentation, unsetting clobberappend has effect even
when noclobber is not active. IOW, this currently provides a way to keep
'>>' from creating files even when noclobber is off.

- 'emulate sh' and 'emulate ksh' do not make any difference to the above.

While it's good to be able to modify this behaviour, this does not
achieve POSIX compatibility for 'set -C': even in the emulation modes,
'>>' still won't create nonexistent files, and enabling this ability
requires a zsh-specific 'set -o clobberappend' command.

At the risk of pedantry, I also wonder if 'createappend' or
'appendcreate' would be a better name, since "clobbering" is commonly
understood to mean overwriting pre-existing files, not creating new ones.

Hope this helps,

- M.

Re: '>>' does not create file if set -C (noclobber) is active

[Martijn Dekker]

Martijn Dekker schreef op 28-06-15 om 02:02:
> - The new clobberappend option is on by default, but 'set -C' (-o
> noclobber) turns clobberappend off. Conversely, 'set +C' (+o noclobber)
> turns it back on. This applies even when noclobber is already on or off.
> 
> - Contrary to the documentation, unsetting clobberappend has effect even
> when noclobber is not active. IOW, this currently provides a way to keep
> '>>' from creating files even when noclobber is off.

Scratch that. Actually turning either of them on turns the other off,
and vice versa.

- M.

Re: '>>' does not create file if set -C (noclobber) is active

[Stephane Chazelas]

2015-06-28 02:02:05 +0200, Martijn Dekker:
[...]
> At the risk of pedantry, I also wonder if 'createappend' or
> 'appendcreate' would be a better name, since "clobbering" is commonly
> understood to mean overwriting pre-existing files, not creating new ones.
[...]

Agreed, I also wonder of the rationale behind that feature. For
zsh, I understand it copied it from csh, but why would
"noclobber" prevent >> from creating files, that has nothing to
do with clobbering.

In 18 years of using zsh and tcsh before that I had never
realised they were doing that and when you reported it, I pretty
much assumed it was a bug and was very surprised when it
revealed to be a feature.

I agree it's a potentially useful feature to prevent >> from
creating files, but it shouldn't be done upon a "noclobber"
option as it's the opposite meaning that noclobber conveys.

BTW, slightly related, I think it would be nice for the shell to
have access to some of the other open() flags. I often found
myself wishing I could use O_NOFOLLOW for instance.

How about a <(flags)> operator

where flags is flag[flag...] and flag being [+] or - followed
by a one-letter flag (like r, w, a, @ (for follow)..., some of
which on by default).

So >> would be <(a)> (a implies -r+w, +w implies +c (create)).
You'd use: <(w-@)> for no-follow...

And we could implement that noclobber on >> with <(a-c)>.

That could even be extended to do lseeks(), dups(), (3<(>+20)>
to lseek(3, 20, SEEK_CUR))...

What do you think?

Yes, with that <(..)> syntax there's a slight risk of breaking
scripts that do:

diff <(cmd1) <(cmd2)> out.txt

-- 
Stephane

Re: '>>' does not create file if set -C (noclobber) is active

[Oliver Kiddle]

Stephane Chazelas wrote:
> I agree it's a potentially useful feature to prevent >> from
> creating files, but it shouldn't be done upon a "noclobber"
> option as it's the opposite meaning that noclobber conveys.

If we're preserving backward compatibility for both states of the
clobber option, it has to be linked to the noclobber option.

I'm not so bothered about the name but perhaps a CSH_ prefix on it would
make sense.

> BTW, slightly related, I think it would be nice for the shell to
> have access to some of the other open() flags. I often found
> myself wishing I could use O_NOFOLLOW for instance.
> 
> How about a <(flags)> operator

O_NOFOLLOW is fairly easily emulated, either with a condition or, if you
really want succinct, a glob qualifier: > file(^@)
But for other flags like O_CLOEXEC or O_SYNC, you can't do much.
We'd also have to worry about how portable each of the open flags are.

New syntax has additional costs such as making scripts less readable. An
alternative would be to add a sysopen builtin to the zsh/system module.
Or perhaps a -o option to exec, e.g. exec -o nofollow 3>file

> That could even be extended to do lseeks(), dups(), (3<(>+20)>
> to lseek(3, 20, SEEK_CUR))...

Recent ksh has <#((offset)) and <##pattern for seeking.
Along with (( fd<# )) to get the current position.
I'm not sure how useful that would really be, especially as byte offsets
are less meaningful when you have multibyte encodings.

Oliver

Re: '>>' does not create file if set -C (noclobber) is active

[Stephane Chazelas]

2015-06-28 11:15:53 +0200, Oliver Kiddle:
[...]
> I'm not so bothered about the name but perhaps a CSH_ prefix on it would
> make sense.

Agreed.

> 
> > BTW, slightly related, I think it would be nice for the shell to
> > have access to some of the other open() flags. I often found
> > myself wishing I could use O_NOFOLLOW for instance.
> > 
> > How about a <(flags)> operator
> 
> O_NOFOLLOW is fairly easily emulated, either with a condition or, if you
> really want succinct, a glob qualifier: > file(^@)

Well, the idea with O_NOFOLLOW is that it's race free. Also
open("a/b/c", O_NOFOLLOW) guarantees the whole path is symlink
free (none of "a", "a/b", "a/b/c" are symlinks or the open
fails).

The idea being to use those in non-trusted areas (world writable
dirs etc) to process files created  by others.

To be fully usable, cd would probably need a similar option and
some frontend to cdat() openat() would be needed as well.

> But for other flags like O_CLOEXEC or O_SYNC, you can't do much.
> We'd also have to worry about how portable each of the open flags are.

Yes.

> New syntax has additional costs such as making scripts less readable. An
> alternative would be to add a sysopen builtin to the zsh/system module.
> Or perhaps a -o option to exec, e.g. exec -o nofollow 3>file

Yes, that would work as well, though would make it more cumbersome
to use like:

(sysopen -o O_SYNC,O_WRONLY,O_CREAT -d 3 -- "$file" && cmd >&3)

instead of

cat 1<ws> "$file"

(but such a need is probably rare enough that it would not be a
concern).

I'd have thought a <(flags)> operator would have appealed to zsh
developers/users as it's similar to things like the parameter
expansion or globbing flags:

while other shells have various operators of different shape and
behaviour nesting or not in non obvious ways, zsh has a clear,
consistent, extensible ${(flags)var} API.

That's very similar with redirection operators. Some shells like
ksh93 have a plethora of different operators. We could for
instance add a >@ for O_NOFOLLOW, >* for O_CLOEXEC... etc, but
it becomes unclear how they can be combined, while <(flags)>
would make it neater.

> > That could even be extended to do lseeks(), dups(), (3<(>+20)>
> > to lseek(3, 20, SEEK_CUR))...
> 
> Recent ksh has <#((offset)) and <##pattern for seeking.
> Along with (( fd<# )) to get the current position.
> I'm not sure how useful that would really be, especially as byte offsets
> are less meaningful when you have multibyte encodings.
[...]

Yes, a few examples of usage of <#(()):

https://unix.stackexchange.com/a/180039
https://unix.stackexchange.com/a/209648
https://unix.stackexchange.com/a/70718
https://unix.stackexchange.com/a/177652

Having a way to tell the current position (like that ((fd<#))
would be necessary to be able to seek back to a previous
position.

Again, I'd agree a sysseek and systell would work as well and
probably be enough.

ksh93's builtin head command has a -s option for that.

-- 
Stephane

Re: '>>' does not create file if set -C (noclobber) is active

[Peter Stephenson]

On Sun, 28 Jun 2015 02:02:05 +0200
Martijn Dekker <martijn@inlv.org> wrote:
> Peter Stephenson schreef op 27-06-15 om 19:02:
> > This isn't very elegant, but nor is it obviously problematic, and it
> > does at least draw attention to a potential issue.
> 
> I've tested this patch against the current git version and noticed the
> following:

Yep, I got throughly confused about the logic.  I think this is now
correct.

It doesn't have to have a name with CLOBBER in it, though it does have
to be related to NO_CLOBBER --- the whole point is to choose between
long-standing and documented zsh behaviour, and POSIX behaviour, when
you have NO_CLOBBER set.  You can now also make your own decision which
you want, and don't need to post messages to the list to tell us :-).
I've called it APPEND_CREATE.

diff --git a/Doc/Zsh/options.yo b/Doc/Zsh/options.yo
index 2371e35..833c975 100644
--- a/Doc/Zsh/options.yo
+++ b/Doc/Zsh/options.yo
@@ -1094,10 +1094,12 @@ pindex(NOCLOBBER)
 cindex(clobbering, of files)
 cindex(file clobbering, allowing)
 item(tt(CLOBBER) (tt(PLUS()C), ksh: tt(PLUS()C)) <D>)(
-Allows `tt(>)' redirection to truncate existing files,
-and `tt(>>)' to create files.
-Otherwise `tt(>!)' or `tt(>|)' must be used to truncate a file,
-and `tt(>>!)' or `tt(>>|)' to create a file.
+Allows `tt(>)' redirection to truncate existing files.
+Otherwise `tt(>!)' or `tt(>|)' must be used to truncate a file.
+
+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(CORRECT)
 pindex(NO_CORRECT)
@@ -1792,6 +1794,21 @@ enditem()
 
 subsect(Shell Emulation)
 startitem()
+pindex(APPEND_CREATE
+pindex(NO_APPEND_CREATE)
+pindex(APPENDCREATE)
+pindex(NOAPPENDCREATE)
+cindex(clobbering, POSIX compatibility)
+cindex(file clobbering, POSIX compatibility)
+cindex(no clobber, POSIX compatible)
+item(tt(APPEND_CREATE) <K> <S>)(
+This option only applies when tt(NO_CLOBBER) (-tt(C)) is in effect.
+
+If this option is not set, the shell will report an error when a
+append redirection (tt(>>)) is used on a file that does not already
+exists (the traditional zsh behaviour of tt(NO_CLOBBER)).  If the option
+is set, no error is reported (POSIX behaviour).
+)
 pindex(BASH_REMATCH)
 pindex(NO_BASH_REMATCH)
 pindex(BASHREMATCH)
diff --git a/Src/exec.c b/Src/exec.c
index 39d1326..960601f 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -3315,7 +3315,8 @@ execcmd(Estate state, int input, int output, int how, int last1)
 		    fil = -1;
 		else if (IS_APPEND_REDIR(fn->type))
 		    fil = open(unmeta(fn->name),
-			       (unset(CLOBBER) && !IS_CLOBBER_REDIR(fn->type)) ?
+			       ((unset(CLOBBER) && unset(APPENDCREATE)) &&
+				!IS_CLOBBER_REDIR(fn->type)) ?
 			       O_WRONLY | O_APPEND | O_NOCTTY :
 			       O_WRONLY | O_APPEND | O_CREAT | O_NOCTTY, 0666);
 		else
diff --git a/Src/options.c b/Src/options.c
index da3d830..320f46b 100644
--- a/Src/options.c
+++ b/Src/options.c
@@ -110,6 +110,7 @@ static struct optname optns[] = {
 {{NULL, "chaselinks",	      OPT_EMULATE},		 CHASELINKS},
 {{NULL, "checkjobs",	      OPT_EMULATE|OPT_ZSH},	 CHECKJOBS},
 {{NULL, "clobber",	      OPT_EMULATE|OPT_ALL},	 CLOBBER},
+{{NULL, "appendcreate",	      OPT_EMULATE|OPT_BOURNE},	 APPENDCREATE},
 {{NULL, "combiningchars",     0},			 COMBININGCHARS},
 {{NULL, "completealiases",    0},			 COMPLETEALIASES},
 {{NULL, "completeinword",     0},			 COMPLETEINWORD},
diff --git a/Src/zsh.h b/Src/zsh.h
index ce9b979..183620f 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -2120,6 +2120,7 @@ enum {
     CHASELINKS,
     CHECKJOBS,
     CLOBBER,
+    APPENDCREATE,
     COMBININGCHARS,
     COMPLETEALIASES,
     COMPLETEINWORD,

Re: '>>' does not create file if set -C (noclobber) is active

[Bart Schaefer]

On Jun 28,  3:00pm, Stephane Chazelas wrote:
}
} That's very similar with redirection operators. Some shells like
} ksh93 have a plethora of different operators. We could for
} instance add a >@ for O_NOFOLLOW, >* for O_CLOEXEC... etc, but
} it becomes unclear how they can be combined, while <(flags)>
} would make it neater.

"<>" already means open r/w, and "<flags>" is syntactically ambiguous
with redirecting input from the file named flags followed by redirecting
output.  

">*" is multio syntax, and if you add ksh globbing, ">@" is ambiguous.

Let me remind everyone that redirection operators were designed as a
front-end on stdio, e.g. fopen(3) and friends, and not on open(2) and
similar OS-level calls.  fopen(3) doesn't interface to the full suite
of flags supported by open(2).  Throw in all this additional detail and
you are effectively preventing the shell implementation from using the
stdio library, which could limit portability.

Therefore the sysopen suggestion is probably the most appropriate one.

The next-best idea I can think of offhand would be to have a variable
that behaves like the umask does for file permissions, except that it
applies to the file opening mode instead.  You could tweak this in
the list of variables that prefix the command, e.g.,

    OPENMODE=04600 cat < $file

There could be a builtin command to translate symbolic names into the
bits for OPENMODE e.g.

    OPENMODE=$(openmode O_EXCL O_NOCTTY O_NONBLOCK) cat < $file

Or I suppose this could all be done as a pre-command modifier:

    openmode O_EXCL,O_NOCTTY,O_NONBLOCK cat < $file

All of this would simply be ignored in environments where it could not
be supported, which is why I think an explicit sysopen that could fail
with well-defined error conditions is preferable.

Re: '>>' does not create file if set -C (noclobber) is active

[Stephane Chazelas]

2015-06-28 11:38:14 -0700, Bart Schaefer:
> On Jun 28,  3:00pm, Stephane Chazelas wrote:
> }
> } That's very similar with redirection operators. Some shells like
> } ksh93 have a plethora of different operators. We could for
> } instance add a >@ for O_NOFOLLOW, >* for O_CLOEXEC... etc, but
> } it becomes unclear how they can be combined, while <(flags)>
> } would make it neater.
> 
> "<>" already means open r/w, and "<flags>" is syntactically ambiguous
> with redirecting input from the file named flags followed by redirecting
> output.  
[...]

Oh yes sorry, I mentionned the conflict with process
substitution (diff <(cmd)> output) in my earlier post, because
I had initially (when I first had that idea of a generic
redirection operator a few months ago and which this thread
reminded me of) thought of: <(flags)>. Another alternative could
be <{flags}>. I like <(flags)> because it's consistent with
${(flags)var}, ${var[(flags)x]}, *glob*(flags)

[...]
> Let me remind everyone that redirection operators were designed as a
> front-end on stdio, e.g. fopen(3) and friends, and not on open(2) and
> similar OS-level calls.  fopen(3) doesn't interface to the full suite
> of flags supported by open(2).  Throw in all this additional detail and
> you are effectively preventing the shell implementation from using the
> stdio library, which could limit portability.
[...]

I'm not sure what you mean here. Surely the shell only opens
files on file descriptors (stdio having no exposed notion of
file descriptors) and then runs applications that may or may not
use stdio but that's independent from the shell. The Bourne
shell never used stdio. And a shell can't buffer its (limited)
output since it mostly runs external commands.

> Therefore the sysopen suggestion is probably the most appropriate one.

Yes, I like that one too and the idea of having a low-level
interface to all the file-related system calls.

> The next-best idea I can think of offhand would be to have a variable
> that behaves like the umask does for file permissions, except that it
> applies to the file opening mode instead.  You could tweak this in
> the list of variables that prefix the command, e.g.,
> 
>     OPENMODE=04600 cat < $file
> 
> There could be a builtin command to translate symbolic names into the
> bits for OPENMODE e.g.
> 
>     OPENMODE=$(openmode O_EXCL O_NOCTTY O_NONBLOCK) cat < $file
> 
> Or I suppose this could all be done as a pre-command modifier:
> 
>     openmode O_EXCL,O_NOCTTY,O_NONBLOCK cat < $file

Except that that would mean all redirections would get the same
flags. The idea with a generic redirection operator is that it
could combine different O_RDONLY, O_RDWR, O_APPEND... flag
combinations (and the extra O_DIRECTORY, O_SYNC, O_EXCL) and
also deal with openat, lseek, and potentially more to come
later.

One thing I like with zsh is that it provides with alternatives
to the glob option settings (like $~var (for globsubst), *(N)
(for nullglob)). Having a way to avoid having to use "set -C" on
a per redirection basis appeals to me like in:

cmd 1<(we)> out 2>> /var/log/my.log

(noclobber for out, normal append with O_CREAT for my.log).

-- 
Stephane

PATCH: sysopen (was Re: '>>' does not create file if set -C (noclobber) is active)

[Oliver Kiddle]

On 28 Jun, Bart wrote:
> Therefore the sysopen suggestion is probably the most appropriate one.

The following patch adds that to the system module along with sysseek
and systell. systell is in the form of a math function which I think
is less awkward to use than a builtin.

I've used -u as the option for indicating the file descriptor because it
is consistent with print and read. You have to be a Fortran programmer
for -u to make sense (unit) so it isn't as helpful now as it was when
ksh was written.

-r/-w/-a are provided for read/write/append as these are common. I
probably would have done -c for create except there is both O_CREAT
and O_EXCL. Permissions for created files is specified with -m and an
octal number. Options after -o are case-insensitive with an initial
O_ ignored. For O_CLOEXEC, there is an fcntl based implementation for
Solaris. I didn't include all possible options only those that I tested
and could think of cases where they might be useful. Adding more
wouldn't be hard. O_NOCTTY is included unconditionally.

Oliver

diff --git a/Doc/Zsh/mod_system.yo b/Doc/Zsh/mod_system.yo
index 7f9c011..e4d4c31 100644
--- a/Doc/Zsh/mod_system.yo
+++ b/Doc/Zsh/mod_system.yo
@@ -28,6 +28,47 @@ system's range), a return status of 1 indicates an error in the
 parameters, and a return status of 2 indicates the error name was
 not recognised (no message is printed for this).
 )
+findex(sysopen)
+redef(SPACES)(0)(tt(ifztexi(NOTRANS(@ @ @ @ @ @ @ @ ))ifnztexi(        )))
+xitem(tt(sysopen) [ tt(-arw) ] [ tt(-m) var(permissions) ] [ tt(-o) var(options) ])
+item(SPACES()[ tt(-u) var(fd) ] var(file))(
+This command opens a file. The tt(-r), tt(-w) and tt(-a) flags indicate
+whether the file should be opened for reading, writing and appending,
+respectively. The tt(-m) option allows the initial permissions to use when
+creating a file to be specified in octal form.  The file descriptor is
+specified with -u. Either an explicit file descriptor in the range 0 to 9 can
+be specified or a variable name can be given to which the file descriptor
+number will be assigned.
+
+The tt(-o) option allows various system specific options to be
+specified as a comma-separated list. The following is a list of possible
+options. Note that, depending on the system, some may not be available.
+startitem()
+item(tt(cloexec))(
+mark file to be closed when other programs are executed
+)
+xitem(tt(create))
+item(tt(creat))(
+create file if it does not exist
+)
+item(tt(excl))(
+create file, error if it already exists
+)
+item(tt(noatime))(
+suppress updating of the file atime
+)
+item(tt(nofollow))(
+fail if var(file) is a symbolic link
+)
+item(tt(sync))(
+request that writes wait until data has been physically written
+)
+xitem(tt(truncate))
+item(tt(trunc))(
+truncate file to size 0
+)
+enditem()
+)
 findex(sysread)
 redef(SPACES)(0)(tt(ifztexi(NOTRANS(@ @ @ @ @ @ @ @ ))ifnztexi(        )))
 xitem(tt(sysread )[ tt(-c) var(countvar) ] [ tt(-i) var(infd) ] [ tt(-o) var(outfd) ])
@@ -89,6 +130,14 @@ usual rules; no write to var(outfd) is attempted.
 )
 enditem()
 )
+item(tt(sysseek) [ tt(-u) var(fd) ] [ tt(-w) tt(start)|tt(end)|tt(current) ] var(offset))(
+The current file position at which future reads and writes will take place is
+adjusted to the specified byte offset. The var(offset) is evaluated as a math
+expression. The tt(-u) option allows the file descriptor to be specified. By
+default the offset is specified relative to the start or the file but, with the
+tt(-w) option, it is possible to specify that the offset should be relative to
+the current position or the end of the file.
+)
 item(tt(syswrite) [ tt(-c) var(countvar) ] [ tt(-o) var(outfd) ] var(data))(
 The data (a single string of bytes) are written to the file descriptor
 var(outfd), or 1 if that is not given, using the tt(write) system call.
@@ -161,6 +210,15 @@ version of the shell before it was implemented).
 )
 enditem()
 
+subsect(Math Functions)
+
+startitem()
+item(tt(systell+LPAR()var(fd)RPAR()))(
+The systell math function returns the current file position for the file
+descriptor passed as an argument.
+)
+enditem()
+
 subsect(Parameters)
 
 startitem()
diff --git a/Src/Modules/system.c b/Src/Modules/system.c
index f6a21d1..a1ed33a 100644
--- a/Src/Modules/system.c
+++ b/Src/Modules/system.c
@@ -279,6 +279,183 @@ bin_syswrite(char *nam, char **args, Options ops, UNUSED(int func))
 }
 
 
+static struct { char *name; int oflag; } openopts[] = {
+#ifdef O_CLOEXEC
+    { "cloexec", O_CLOEXEC },
+#else
+# ifdef FD_CLOEXEC
+    { "cloexec", 0  }, /* this needs to be first in the table */
+# endif
+#endif
+#ifdef O_NOFOLLOW
+    { "nofollow", O_NOFOLLOW },
+#endif
+#ifdef O_SYNC
+    { "sync", O_SYNC },
+#endif
+#ifdef O_NOATIME
+    { "noatime", O_NOATIME },
+#endif
+    { "excl", O_EXCL | O_CREAT },
+    { "creat", O_CREAT },
+    { "create", O_CREAT },
+    { "truncate", O_TRUNC },
+    { "trunc", O_TRUNC }
+};
+
+/**/
+static int
+bin_sysopen(char *nam, char **args, Options ops, UNUSED(int func))
+{
+    int read = OPT_ISSET(ops, 'r');
+    int write = OPT_ISSET(ops, 'w');
+    int append = OPT_ISSET(ops, 'a') ? O_APPEND : 0;
+    int flags = O_NOCTTY | append | ((append || write) ?
+	(read ? O_RDWR : O_WRONLY) :
+	(!append || read) ? O_RDONLY : O_WRONLY);
+    char *opt, *ptr, *nextopt, *fdvar;
+    int o, fd, explicit = -1;
+    mode_t perms = 0666;
+#if defined(FD_CLOEXEC) && !defined(O_CLOEXEC)
+    int fdflags;
+#endif
+
+    if (!OPT_ISSET(ops, 'u')) {
+	zwarnnam(nam, "file descriptor not specified");
+	return 1;
+    }
+
+    /* file descriptor, either 0-9 or a variable name */
+    fdvar = OPT_ARG(ops, 'u');
+    if (idigit(*fdvar) && !fdvar[1]) {
+	explicit = atoi(fdvar);
+    } else if (!isident(fdvar)) {
+	zwarnnam(nam, "not an identifier: %s", fdvar);
+	return 1;
+    }
+
+    /* open options */
+    if (OPT_ISSET(ops, 'o')) {
+	opt = OPT_ARG(ops, 'o');
+	while (opt) {
+	    if (!strncasecmp(opt, "O_", 2)) /* ignore initial O_ */
+		opt += 2;
+	    if ((nextopt = strchr(opt, ',')))
+		*nextopt++ = '\0';
+	    for (o = sizeof(openopts)/sizeof(*openopts) - 1; o >= 0 &&
+		strcasecmp(openopts[o].name, opt); o--) {}
+	    if (o < 0) {
+		zwarnnam(nam, "unsupported option: %s\n", opt);
+		return 1;
+	    }
+#if defined(FD_CLOEXEC) && !defined(O_CLOEXEC)
+	    if (!openopts[o].oflag)
+		fdflags = FD_CLOEXEC;
+#endif
+	    flags |= openopts[o].oflag;
+	    opt = nextopt;
+	}
+    }
+
+    /* -m: permissions or mode for created files */
+    if (OPT_ISSET(ops, 'm')) {
+	ptr = opt = OPT_ARG(ops, 'm');
+	while (*ptr >= '0' && *ptr <= '7') ptr++;
+	if (*ptr || ptr - opt < 3) {
+	    zwarnnam(nam, "invalid mode %s", opt);
+	    return 1;
+	}
+	perms = zstrtol(opt, 0, 8); /* octal number */
+    }
+
+    if (flags & O_CREAT)
+	fd = open(*args, flags, perms);
+    else
+	fd = open(*args, flags);
+
+    if (fd == -1) {
+	zwarnnam(nam, "can't open file %s: %e", *args, errno);
+	return 1;
+    }
+    fd = (explicit > -1) ? redup(fd, explicit) : movefd(fd);
+    if (fd == -1) {
+	zwarnnam(nam, "can't open file %s", *args);
+	return 1;
+    }
+
+#if defined(FD_CLOEXEC) && !defined(O_CLOEXEC)
+    if (fdflags)
+	fcntl(fd, F_SETFD, FD_CLOEXEC);
+#endif
+    if (explicit == -1) {
+	fdtable[fd] = FDT_EXTERNAL;
+	setiparam(fdvar, fd);
+	/* if setting the variable failed, close fd to avoid leak */
+	if (errflag)
+	    zclose(fd);
+    }
+
+    return 0;
+}
+
+
+/*
+ * Return values of bin_sysseek:
+ *	0	Success
+ *	1	Error in parameters to command
+ *	2	Error on seek, ERRNO set by system
+ */
+
+/**/
+static int
+bin_sysseek(char *nam, char **args, Options ops, UNUSED(int func))
+{
+    int w = SEEK_SET, fd = 0;
+    char *whence;
+    off_t pos;
+
+    /* -u:  file descriptor if not stdin */
+    if (OPT_ISSET(ops, 'u')) {
+	fd = getposint(OPT_ARG(ops, 'u'), nam);
+	if (fd < 0)
+	    return 1;
+    }
+
+    /* -w:  whence - starting point of seek */
+    if (OPT_ISSET(ops, 'w')) {
+	whence = OPT_ARG(ops, 'w');
+        if (!(strcasecmp(whence, "current") && strcmp(whence, "1")))
+	    w = SEEK_CUR;
+        else if (!(strcasecmp(whence, "end") && strcmp(whence, "2")))
+	    w = SEEK_END;
+	else if (strcasecmp(whence, "start") && strcmp(whence, "0")) {
+	    zwarnnam(nam, "unknown argument to -w: %s", whence);
+	    return 1;
+	}
+    }
+
+    pos = (off_t)mathevali(*args);
+    return (lseek(fd, pos, w) == -1) ? 2 : 0;
+}
+
+/**/
+static mnumber
+math_systell(UNUSED(char *name), int argc, mnumber *argv, UNUSED(int id))
+{
+    int fd = (argv->type == MN_INTEGER) ? argv->u.l : (int)argv->u.d;
+    mnumber ret;
+    ret.type = MN_INTEGER;
+    ret.u.l = 0;
+
+    if (fd < 0) {
+	zerr("file descriptor out of range");
+	return ret;
+    }
+    ret.u.l = lseek(fd, 0, SEEK_CUR);
+    return ret;
+}
+
+
 /*
  * Return values of bin_syserror:
  *	0	Successfully processed error
@@ -542,6 +719,8 @@ static struct builtin bintab[] = {
     BUILTIN("syserror", 0, bin_syserror, 0, 1, 0, "e:p:", NULL),
     BUILTIN("sysread", 0, bin_sysread, 0, 1, 0, "c:i:o:s:t:", NULL),
     BUILTIN("syswrite", 0, bin_syswrite, 1, 1, 0, "c:o:", NULL),
+    BUILTIN("sysopen", 0, bin_sysopen, 1, 1, 0, "rwau:o:m:", NULL),
+    BUILTIN("sysseek", 0, bin_sysseek, 1, 1, 0, "u:w:", NULL),
     BUILTIN("zsystem", 0, bin_zsystem, 1, -1, 0, NULL, NULL)
 };
 
@@ -611,6 +790,9 @@ scanpmsysparams(UNUSED(HashTable ht), ScanFunc func, int flags)
     func(&spm.node, flags);
 }
 
+static struct mathfunc mftab[] = {
+    NUMMATHFUNC("systell", math_systell, 1, 1, 0)
+};
 
 static struct paramdef partab[] = {
     SPECIALPMDEF("errnos", PM_ARRAY|PM_READONLY,
@@ -622,7 +804,7 @@ static struct paramdef partab[] = {
 static struct features module_features = {
     bintab, sizeof(bintab)/sizeof(*bintab),
     NULL, 0,
-    NULL, 0,
+    mftab, sizeof(mftab)/sizeof(*mftab),
     partab, sizeof(partab)/sizeof(*partab),
     0
 };
diff --git a/Src/Modules/system.mdd b/Src/Modules/system.mdd
index 46f02d1..eed0c1b 100644
--- a/Src/Modules/system.mdd
+++ b/Src/Modules/system.mdd
@@ -2,7 +2,7 @@ name=zsh/system
 link=dynamic
 load=no
 
-autofeatures="b:sysread b:syswrite b:syserror p:errnos"
+autofeatures="b:sysread b:syswrite b:sysopen b:sysseek b:syserror p:errnos f:systell"
 
 objects="system.o errnames.o"
 

Re: PATCH: sysopen (was Re: '>>' does not create file if set -C (noclobber) is active)

[Peter Stephenson]

On Thu, 23 Jul 2015 04:56:58 +0200
Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> On 28 Jun, Bart wrote:
> > Therefore the sysopen suggestion is probably the most appropriate one.
> 
> The following patch adds that to the system module along with sysseek
> and systell. systell is in the form of a math function which I think
> is less awkward to use than a builtin.

One very minor comment now I've tried this:

% sysread -i fd
sysread: integer expected: fd

I think it would probably be appropriate and consistent with other
similar contexts for this to do matn eval, even though the workaround is
obvious and trivial.  C.f. "sysopen -u fd" (though of course there's no
choice in that case) and

% print $(( systell(fd) ))
8192

However, we're not actually missing anything by not.

pws

Re: PATCH: sysopen (was Re: '>>' does not create file if set -C (noclobber) is active)

[Peter Stephenson]

Also, I suppose we should say something like...

(although adding sysclose isn't necessarily a bad thing)

pws

diff --git a/Doc/Zsh/mod_system.yo b/Doc/Zsh/mod_system.yo
index e4d4c31..b9b07bb 100644
--- a/Doc/Zsh/mod_system.yo
+++ b/Doc/Zsh/mod_system.yo
@@ -68,6 +68,11 @@ item(tt(trunc))(
 truncate file to size 0
 )
 enditem()
+
+To close the file, use one of the following:
+
+example(tt(exec {)var(fd)tt(}<&-)
+tt(exec {)var(fd)tt(}>&-))
 )
 findex(sysread)
 redef(SPACES)(0)(tt(ifztexi(NOTRANS(@ @ @ @ @ @ @ @ ))ifnztexi(        )))

Re: PATCH: sysopen (was Re: '>>' does not create file if set -C (noclobber) is active)

[Mikael Magnusson]

On Fri, Jul 24, 2015 at 1:55 PM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
> Also, I suppose we should say something like...
>
> (although adding sysclose isn't necessarily a bad thing)
>
> pws
>
> diff --git a/Doc/Zsh/mod_system.yo b/Doc/Zsh/mod_system.yo
> index e4d4c31..b9b07bb 100644
> --- a/Doc/Zsh/mod_system.yo
> +++ b/Doc/Zsh/mod_system.yo
> @@ -68,6 +68,11 @@ item(tt(trunc))(
>  truncate file to size 0
>  )
>  enditem()
> +
> +To close the file, use one of the following:
> +
> +example(tt(exec {)var(fd)tt(}<&-)
> +tt(exec {)var(fd)tt(}>&-))
>  )
>  findex(sysread)
>  redef(SPACES)(0)(tt(ifztexi(NOTRANS(@ @ @ @ @ @ @ @ ))ifnztexi(        )))

I remembered this being mentioned and was looking through the docs
because I always forget where the & goes, but it looks like you never
committed it. It is mentioned in the paragraph about zsystem flock
though.

Was there a reason the new commands were not made subcommands to
zsystem? I don't see any rationale in the commit message. An advantage
would be that "zsystem supports foo" already will tell you that foo is
not a recognized subcommand. We could add the sysopen/tell/close/seek
commands to zsystem supports anyway, even though they're not
subcommands.

> One very minor comment now I've tried this:
>
> % sysread -i fd
> sysread: integer expected: fd
>
> I think it would probably be appropriate and consistent with other
> similar contexts for this to do matn eval, even though the workaround is
> obvious and trivial.

I feel like this might be slightly dangerous, as if you get used to fd
being treated as an integer here, you may try sysopen -u fd and it
will silently use another fd and assign to your parameter instead.


It also looks like the -u option for sysopen is not optional, so it
shouldn't be listed in [ ].

--- i/Doc/Zsh/mod_system.yo
+++ w/Doc/Zsh/mod_system.yo
@@ -31,7 +31,7 @@ not recognised (no message is printed for this).
 findex(sysopen)
 redef(SPACES)(0)(tt(ifztexi(NOTRANS(@ @ @ @ @ @ @ @ ))ifnztexi(        )))
 xitem(tt(sysopen) [ tt(-arw) ] [ tt(-m) var(permissions) ] [ tt(-o)
var(options) ])
-item(SPACES()[ tt(-u) var(fd) ] var(file))(
+item(SPACES()tt(-u) var(fd) var(file))(
 This command opens a file. The tt(-r), tt(-w) and tt(-a) flags indicate
 whether the file should be opened for reading, writing and appending,
 respectively. The tt(-m) option allows the initial permissions to use when


-- 
Mikael Magnusson

Re: PATCH: sysopen (was Re: '>>' does not create file if set -C (noclobber) is active)

[Oliver Kiddle]

On 24 Jul, Peter wrote:
> One very minor comment now I've tried this:
> 
> % sysread -i fd
> sysread: integer expected: fd
> 
> I think it would probably be appropriate and consistent with other
> similar contexts for this to do matn eval, even though the workaround is
> obvious and trivial.  C.f. "sysopen -u fd" (though of course there's no
> choice in that case) and
> 
> % print $(( systell(fd) ))
> 8192
> 
> However, we're not actually missing anything by not.

I don't have a strong opinion either way on this.
This could apply to print and read just as much as sysread, syswrite and
sysseek.

By the way, I'm happy with both documentation patches.

Mikael wrote:
> Was there a reason the new commands were not made subcommands to
> zsystem?

sysread and syswrite are not zsystem subcommands.

What is the purpose of zsystem (as opposed to a direct sysflock)? Is it
that flock is not supported by some common systems?

Coverity noticed that I'd made one part more complicated than it needs
to be so this cleans that up (the !append was tautologous).

Oliver

diff --git a/Src/Modules/system.c b/Src/Modules/system.c
index a1ed33a..1ab1fb1 100644
--- a/Src/Modules/system.c
+++ b/Src/Modules/system.c
@@ -311,8 +311,7 @@ bin_sysopen(char *nam, char **args, Options ops, UNUSED(int func))
     int write = OPT_ISSET(ops, 'w');
     int append = OPT_ISSET(ops, 'a') ? O_APPEND : 0;
     int flags = O_NOCTTY | append | ((append || write) ?
-	(read ? O_RDWR : O_WRONLY) :
-	(!append || read) ? O_RDONLY : O_WRONLY);
+	(read ? O_RDWR : O_WRONLY) : O_RDONLY);
     char *opt, *ptr, *nextopt, *fdvar;
     int o, fd, explicit = -1;
     mode_t perms = 0666;

Re: PATCH: sysopen (was Re: '>>' does not create file if set -C (noclobber) is active)

[Mikael Magnusson]

On Fri, Jul 31, 2015 at 2:41 PM, Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> Mikael wrote:
>> Was there a reason the new commands were not made subcommands to
>> zsystem?
>
> sysread and syswrite are not zsystem subcommands.
>
> What is the purpose of zsystem (as opposed to a direct sysflock)? Is it
> that flock is not supported by some common systems?

Ah, I didn't actually notice that sysread and syswrite were already
there, and not added by your patch. I just saw zsystem after all the
others in the manpage and thought it seemed a bit inconsistent. It
would seem the only purpose is forward compatibility (eg, being able
to use zsystem supports from a script and have it work on older zshs
too).

> Coverity noticed that I'd made one part more complicated than it needs
> to be so this cleans that up (the !append was tautologous).

Aha, I submitted the new build and saw the warning but couldn't really
figure out what it was trying to tell me :).

-- 
Mikael Magnusson