Fix compilation on Solaris 10

2019-02-28 Thread coypu
Hi folks,

in Solaris 10 apparently O_CLOEXEC is not defined.
(got a bug report in http://gnats.netbsd.org/54025)

Attached is a patch wrapping the code using it in #ifdef O_CLOEXEC.

thanks.
>From e9d262e43c6d4ada77f1b722f352fb55a4107ec0 Mon Sep 17 00:00:00 2001
From: Maya Rashish 
Date: Thu, 28 Feb 2019 15:26:52 +0200
Subject: [PATCH 1/1] Handle O_CLOEXEC not being defined (Solaris 10)

---
 examples/loadables/fdflags.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/examples/loadables/fdflags.c b/examples/loadables/fdflags.c
index f3094666..30355c9f 100644
--- a/examples/loadables/fdflags.c
+++ b/examples/loadables/fdflags.c
@@ -113,8 +113,10 @@ getflags(int fd, int p)
   return -1;
 }
 
+#ifdef O_CLOEXEC
   if (c)
 f |= O_CLOEXEC;
+#endif
 
   return f & getallflags();
 }
@@ -198,6 +200,7 @@ setone(int fd, char *v, int verbose)
 
   parseflags(v, &pos, &neg);
 
+#ifdef O_CLOEXEC
   cloexec = -1;
   if ((pos & O_CLOEXEC) && (f & O_CLOEXEC) == 0)
 cloexec = FD_CLOEXEC;
@@ -209,6 +212,7 @@ setone(int fd, char *v, int verbose)
   pos &= ~O_CLOEXEC;
   neg &= ~O_CLOEXEC;
   f &= ~O_CLOEXEC;
+#endif
 
   n = f;
   n |= pos;
-- 
2.20.1



Re: Fix compilation on Solaris 10

2019-02-28 Thread Chet Ramey
On 2/28/19 8:38 AM, co...@sdf.org wrote:
> Hi folks,
> 
> in Solaris 10 apparently O_CLOEXEC is not defined.
> (got a bug report in http://gnats.netbsd.org/54025)
> 
> Attached is a patch wrapping the code using it in #ifdef O_CLOEXEC.

Thanks for the report. This was fixed in the devel branch a few weeks
ago.

Chet

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/



Re: Fix compilation on Solaris 10

2019-02-28 Thread Eric Blake
On 2/28/19 7:38 AM, co...@sdf.org wrote:
> Hi folks,
> 
> in Solaris 10 apparently O_CLOEXEC is not defined.
> (got a bug report in http://gnats.netbsd.org/54025)
> 
> Attached is a patch wrapping the code using it in #ifdef O_CLOEXEC.

Incomplete - if you are unable to atomically set the CLOEXEC flag during
open() due to lack of O_CLOEXEC, then you should use
fcntl(F_GETFD)/fcntl(F_SETFD) to set FD_CLOEXEC relatively soon after
the open(), so that the fd is still properly closed on forks either way.

In multi-threaded applications, O_CLOEXEC is essential to avoid data
races where one thread calls fork() in between another window calling
open() vs. fcntl() and thus inadvertently leaking the fd into the child;
but since bash is single-threaded, the race is only possible via signal
handlers, and hopefully bash isn't trying to fork from a signal handler
that might be interrupting the window between open() and fcntl().

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



Re: "$@" expansion when it is consists of only null strings

2019-02-28 Thread Chet Ramey
On 2/24/19 6:37 PM, Grisha Levit wrote:
> There are some what seem to be regressions (?) in bash-4.4 and
> bash-5.0 regarding the handling of "$@" expansion when it consists
> entirely of null strings.

Thanks for the report and additional tests.

Chet
-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/



Re: "$@" expansion when it is consists of only null strings

2019-02-28 Thread Chet Ramey
On 2/25/19 5:38 PM, Grisha Levit wrote:
> On Sun, Feb 24, 2019 at 11:22 PM Robert Elz  wrote:
>> I think these are actually more the insertion of explicit null
>> strings, rather than $@ when it contains null strings.   These
>> are essentially the same as the ${b+s ''} issue that was discussed
>> a week or so ago, and which Chet already said he would look into.
> 
> Apologies to Chet if I'm bringing up issues with something that still
> a WIP. 

Well, it's sort of always a work in progress. As long as you keep sending
me reports of apparently incorrect results, we can debate the proper
behavior and fix bash, if necessary.


> The two older commits I mentioned were changes to bring a great
> number of things in compliance with interpretation 888 and the corner
> case bugs I mention above are AFAICT casualties of those fixes. 

There were some longstanding assumptions that were deeply baked into
bash's word expansion code. I'm sure there are still some more that need
to be changed as a result of 888 and a couple other interpretations.

>>   | And these have always been the case, but I'm not sure if it should be so?
>>   |
>>   | $ set --
>>   | $ n "${_+"$@"}"
>>   | 1
>>   | $ n "${_+$@}"
>>   | 1
>>
>> Those are, I believe, unspecified cases.   The first is definitely
>> because of the strange quoting (the " chare are in order, open, close,
>> open, close, not open open close close).

There is an interpretation that somewhat decoupled quotes outside the
expansion with quotes inside it, but I can't remember the specifics
right now. It might be 888, but I seem to remember another.

The root of the issue is whether or not the expansion starts a new logical
"quoting context," so the $@ is treated as double-quoted instead of
unquoted.

> 
> I think I'm missing something but how can that be the case regarding
> the quoting?
> For example "${x+" a   b "}" expands to a single field in
> bash/dash/yash/zsh/netbsd sh (though not in ksh..)

Because ksh uses the open, open, close, close interpretation.

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/



Re: Fix compilation on Solaris 10

2019-02-28 Thread Chet Ramey
On 2/28/19 10:21 AM, Eric Blake wrote:
> On 2/28/19 7:38 AM, co...@sdf.org wrote:
>> Hi folks,
>>
>> in Solaris 10 apparently O_CLOEXEC is not defined.
>> (got a bug report in http://gnats.netbsd.org/54025)
>>
>> Attached is a patch wrapping the code using it in #ifdef O_CLOEXEC.
> 
> Incomplete - if you are unable to atomically set the CLOEXEC flag during
> open() due to lack of O_CLOEXEC, then you should use
> fcntl(F_GETFD)/fcntl(F_SETFD) to set FD_CLOEXEC relatively soon after
> the open(), so that the fd is still properly closed on forks either way.

Not relevant. The use here is the loadable builtin to report and set
file descriptor flags (hence the `fdflags').


-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/



Re: "$@" expansion when it is consists of only null strings

2019-02-28 Thread Robert Elz
Date:Mon, 25 Feb 2019 17:38:07 -0500
From:Grisha Levit 
Message-ID:  



First, apologies from me for missing this message from you.   I don't
know if my spam filters caught it (for some unknown reason) or whether
it was delivered and I simply discarded it without noticing that it was
real mail and not an example of spam that made it past my spam filters.
I have cleaned out my spam folder between when you sent this and now,
so I can't simply go look there to find out (if it had been filtered,
it would have gone there, but if it was my eyeballs failing, then it
would have just been removed ...)

I had no idea your reply existed until I saw Chet's reply to you, and
went hunting (fortunately I keep many backups of all mail received,
including spam, so I could go hunting, using the referenced message-id
in his message, to find where your message appeared, and recover it.)

  | Yup I was referencing the devel version that fixed the ${b+s ''} issue.

Oh.  I didn't know there had been a fix for that.   Great.

  | I think I'm missing something but how can that be the case regarding
  | the quoting?

It all relates to the way the original Bourne shell (1978/9 vintage,
on a pdp-11 ... so very little code or data space) parsed quoting.

The rule was basically very simple, each (unquoted) " (and same for ')
in the input byte stream simply toggled the "quoted" flag - and this was
done very early in the input data processing.

Every other character (except \ or course, which simply quoted the following
char) was combined with the "quoted" flag to form the character that existed
internally (the "quoted" flag was 0x80, and "combined" meant (ch|quoted).
(The few chars with special meaning insiode "" were handled differently,
but I don't remember exactly how right now.)

The effect of this is that a quoted '*' for example would not compare equal
to an unquoted '*' (one is 0x2A, the other 0xAA), so quoting the magic
chars for glob simply worked (glob looks only for 2A, an unquoted '*').
When comparing normal chars for equality the quote flag was simply stripped
(if ((ch & 0x7f) == *p) ... or similar).

Similarly, IFS can only contain unquoted characters (quote removal
happens -- which at the time that meant &= 0xFF -- before the value is
assigned), so field splitting never worked on a quoted char, only on an
unquoted one.

This allowed all kinds of simplifications to the code, so it could be
implemented in very little space.Unfortunately it also meant that
we ended up with the weird spec for "${var+w"or"d}" where the "or" are
not quoted, but the 'w' and 'd' are - it is also what leads to
"${var+w'or'd)}" (when var is set) expanding to "w'or'd"  as the '
characters are still quoted by the "" and are not quoting chars themselves.

When Korn first implemented ksh (so I am told, I have never seen a ksh this
old) he fixed that, and made the quoting context for word in those 4
expansions, as well as the new substring extraction expansions he invented,
be unrelated to the quoting context surrounding the expansion.   But, so
I have been told, we was convinced that was incompatible, and so changed
the 4 original operators - + = and ?, back, so they were processed the
same way that the original Bourne shell did them, but left the substring
operators (% %% # and ##) the new way.

Until relatively recently, that was the supposed state of the world, and
what POSIX demanded.

But in the interim, several shells have been convinced by their users
(or never understood in the first place perhaps for some of them) that
it is (was) supposed to work this way, and made the implementation be
more like what users expect, rather than what the original Bourne shell
implemented.

Eventually POSIX, which is supposed to be telling script writers what
they should expect to work in the wild, had to relent, and make this be
an unspecified case, as ...

  | For example "${x+" a   b "}" expands to a single field in
  | bash/dash/yash/zsh/netbsd sh (though not in ksh..)

is simply reality - the FreeBSD shell is in the ksh camp (they have a
very POSIX conforming implementation, modulo the occasional bug of course).

The effect is that if you want to write portable code, you simply cannot
put quotes inside a quoted variable expansion using any of the older 4
operators (+ =- = ?) ... but with anything newer it works fine.

And incidentally, after more research, I can no longer justify:

  | > The second because there's no real agreement whether it should produce
  | > 0 or 1 (different shells do different things for that one, and there's
  | > no particularly good argument for one or the other, so posix, I believe,
  | > makes that one unspecified as well.)

There's nothing I can find which makes that happen, it appears that the
standard still expects nothing (no fields) to result from

"${X+$@}"

when X is set, and there are no positional parameters, despite almost
no shells (not even FreeBSD) implementing it that way.  

Re: "$@" expansion when it is consists of only null strings

2019-02-28 Thread Chet Ramey
On 2/28/19 2:32 PM, Robert Elz wrote:

> chet.ra...@case.edu also said:
>   | Because ksh uses the open, open, close, close interpretation.
> 
> Actually just the opposite,

Correct, my bad.

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/



Re[2]: history -a misbehaves with $HISTSIZE

2019-02-28 Thread Айрат Васбикарамов
> It seems like what you want is min(history_lines_this_session, 
> history_offset),
> kind of like what you say below. Try the attached patch and see if it
> does what you want.

Yes, that's what I mean.

> I don't think this would happen too much in practice, though, because if
> you wait until you have more than $HISTSIZE history entries, you'll lose
> information no matter what you use.

I don't think that happens frequent either. But it's possible. Someone set 
HISTSIZE to 1000 as he don't care about too old commands and it's happened that 
he typed more than 1000 command. Then he run "history -a" to synchronize 
current history with another started bash for example. And lost recent 
commands. Possibly he notice it. But may not.

Anyway I don't see downsides of this behavior comparing to current. And it 
seems more intuitive to me. So should it be default?

>Воскресенье, 24 февраля 2019, 1:16 +03:00 от Chet Ramey :
>
>On 2/13/19 3:47 PM, Айрат Васбикарамов wrote:
>
>Sorry it took a while to respond; this message ended up in my spam folder
>for some reason.
>
>> Thanks for clarification. But I still consider this behavior inconsistent.
>> 
>> 1)  Why we need to check that history_lines_this_session is less than 
>> history_offset? history_lines_this_session always stores number of lines in 
>> this session. So we can just append this much lines.
>If you're at the end of the history, history_offset is the index of the
>last entry in the history list. You can't just append
>history_lines_this_session; there aren't that many entries in the history
>list. You don't really have to worry about this; append_history already
>takes care of it.
>
>> 
>> 2) Let me rewrite what happens in preceding example. We can set $HISTSIZE 
>> which we prefer (for example in bashrc). Then we start new session. And it's 
>>  happened that we type more than $HISTSIZE commands. Then if I type "history 
>> -a", I expect that $HISTSIZE commands will be appended to history file 
>> (leading commands will disappear, as our buffer can't hold this much 
>> commands). But actually nothing will be added!
>
>OK. So in this case, it doesn't append anything. I think it violates one of
>the assumptions in place when the code was written (1990!). The interesting
>thing is that if you type only one more command before running `history -a'
>again, it will "work", since maybe_append_history() resets the value of
>history_lines_this_session to 0 no matter what.
>
>I don't think this would happen too much in practice, though, because if
>you wait until you have more than $HISTSIZE history entries, you'll lose
>information no matter what you use.
>
>It seems like what you want is min(history_lines_this_session, history_offset),
>kind of like what you say below. Try the attached patch and see if it
>does what you want.
>
>
>> 
>> 3) Actually this behavior already present in code if histappend option is 
>> set.
>>    bashhist.c    maybe_save_shell_history()
>>     ... 
>> if (history_lines_this_session <= where_history () || force_append_history)
>> ...
>>  So if we say to append history at exit it doesn't bother to check that 
>> history_lines_this_session is less than offset. It just save 
>> min(history_lines_this_session, HISTSIZE) to history file. So it's not clear 
>> why when we explicitly tell to append history behavior is "weaker" than when 
>> we set histappend option.
>
>The `histappend' option explicitly doesn't care about duplicates, so it
>never cared about trying to figure out the history entries that hadn't
>been read from the history file.
>
>Chet
>
>-- 
>``The lyf so short, the craft so long to lerne.'' - Chaucer
>``Ars longa, vita brevis'' - Hippocrates
>Chet Ramey, UTech, CWRU  c...@case.edu http://tiswww.cwru.edu/~chet/


-- 
Airat