Re: [dwm] input status text bug?
On Fri, Nov 02, 2007 at 10:49:18PM +0100, Sander van Dijk wrote: On Nov 2, 2007 10:02 PM, Anselm R. Garbe [EMAIL PROTECTED] wrote: Hmm, if that's the reason I tend to write a read()-based getline function which does not block ;) Try doing this: for i in `seq 1 10` do echo -n bla if test $i = 5 then echo fi sleep 3 done | dwm Than dwm will be unresponsive for a while, after some time it'll update the statustext as blablablablabla and continue to be unresponsive. Well I extended the old low-level approach with an offset handling, and your example and all others work really quite well now (recheck hg tip). One thing which behaves differently now is, that dwm will drop subsequent multiline data except the first line during a single read(). This approach makes the algorithm more readable and elegant, and usually nobody writes more than a single line to dwm per status update. Regards, -- Anselm R. Garbe http://www.suckless.org/ GPG key: 0D73F361
Re: [dwm] input status text bug?
It doesn't work at all... 2007/11/2, Anselm R. Garbe [EMAIL PROTECTED]: Ritesh, I pushed a simplified status text processing version which uses fgets again, for readability and simplicity reasons. Could you please recheck if hg tip fixes your issue? Regards, -- Anselm R. Garbe http://www.suckless.org/ GPG key: 0D73F361 -- http://www.gnuffy.org - Real Community Distro http://www.gnuffy.org/index.php/GnuEm - Gnuffy on Ipaq (Codename Peggy)
Re: [dwm] input status text bug?
The reason was something in my .xinitrc. I changed it and everything is fine. 2007/11/2, Anselm R. Garbe [EMAIL PROTECTED]: On Fri, Nov 02, 2007 at 05:21:36PM +0100, Enno Gottox Boland wrote: It doesn't work at all... Sorry, but I can't reproduce. It works correctly for me so far. 2007/11/2, Anselm R. Garbe [EMAIL PROTECTED]: Ritesh, I pushed a simplified status text processing version which uses fgets again, for readability and simplicity reasons. Could you please recheck if hg tip fixes your issue? -- Anselm R. Garbe http://www.suckless.org/ GPG key: 0D73F361 -- http://www.gnuffy.org - Real Community Distro http://www.gnuffy.org/index.php/GnuEm - Gnuffy on Ipaq (Codename Peggy)
Re: [dwm] input status text bug?
On Nov 2, 2007 6:37 PM, Anselm R. Garbe [EMAIL PROTECTED] wrote: On Fri, Nov 02, 2007 at 05:21:36PM +0100, Enno Gottox Boland wrote: It doesn't work at all... Sorry, but I can't reproduce. It works correctly for me so far. It seems to me that the call to fgets() blocks, that is, when it's reading for example from a fifo, and data is coming in over that fifo, it will ignore any events until it encounters a newline (the statusbar isn't updated until it does, and it doesn't respond to keypresses and such). I'm not sure if that is what we want, but if it is, than I think it should at least be explicitly documented somewhere (probably both in the README and in the BUGS section of the manpage). Greetings, Sander.
Re: [dwm] input status text bug?
On Nov 2, 2007 10:02 PM, Anselm R. Garbe [EMAIL PROTECTED] wrote: Hmm, if that's the reason I tend to write a read()-based getline function which does not block ;) Try doing this: for i in `seq 1 10` do echo -n bla if test $i = 5 then echo fi sleep 3 done | dwm Than dwm will be unresponsive for a while, after some time it'll update the statustext as blablablablabla and continue to be unresponsive. Greetings, Sander.
Re: [dwm] input status text bug?
Hi Ritesh,
On Tue, Oct 30, 2007 at 11:04:33PM -0400, Ritesh Kumar wrote:
I used to run dwm using the following bash script:
while true; do
date;
sleep 1;
done | dwm
... and things used to run fine :).
I changed it to:
while true; do
cat $HOME/.dwm/status_* ;
date;
sleep 1;
done | dwm
The status_* files contain text without trailing \n's.
Surprisingly , the contents of the statusbar didn't change (only date, no
other status text) ... even though the while loop outputs something like
some text Tue Oct 30 22:46:56 EDT 2007
some text Tue Oct 30 22:46:57 EDT 2007
some text Tue Oct 30 22:46:58 EDT 2007
...
My guess is that read() reads the status_* text and date in two successive
calls causing the date to prevail till sleep 1 is done.
Well let's have a look at the relevant bits in the code:
(a) switch(r = read(STDIN_FILENO, stext, sizeof stext - 1)) {
[...]
default:
(b) for(stext[r] = '\0', p = stext + strlen(stext) - 1; p = stext
*p == '\n'; *p-- = '\0');
(c) for(; p = stext *p != '\n'; --p);
(d) if(p stext)
(e) strncpy(stext, p + 1, sizeof stext);
[...]
(a) Reads the next bunch of stdin, beginning at stext[0] and
ending at stext[r].
(b) 0-terminates stext, and trims rightmost '\n' from right to left only.
(c) If p still does not point to stext, it now seeks for a '\n'
character leftwards.
(d) If during (c) such a '\n' character was found, it is the
begin of the last line which has been read.
(e) Hence, we copy the last line read only to stext, beginning
at the '\n' character + 1 to stext. It is 0-terminated already.
So in my eyes, assumed the complete output of the input script
is read, the code is correct and does what it is supposed
to do: it only displays the last line read from stdin (this might
be a bunch of data which is not '\n'-terminated yet during this read
round of course).
If you like to debug, you could add an
fprintf(stderr, 'stext == %s'\n, stext);
in there between (c) and (d). Then we can make sure that your
guess is correct.
If the input is not read during one read call as you believe,
then at least there must be some flashing, because dwm will
actually render the some text at the status bar, but maybe
your system is really fast and you don't notice this.
I went through the source code and fixed the status input handling code to
make sure it flushes the input status text only if in encounters a '\n'. The
patch is given... unfortunately it is over my (recently posted) status text
patch. I moved the trimming of trailing \n's to another function setstext()
(introduced by my status text patch)... however, the relevant code can be
moved to the function run().
What do you guys think... did I accidentally break something?
I see the problem in the above code, and also believe that your
solution is more correct. What scares me is, that it might seen
as valid input if someone feeds dwm with for example:
echo -n foo
during while... but we can fix this. Please lemme know if your
guess is correct through using the above fprintf-debug trick.
Regards,
--
Anselm R. Garbe http://www.suckless.org/ GPG key: 0D73F361
Re: [dwm] input status text bug?
On Wed, Oct 31, 2007 at 09:28:33AM +0100, Anselm R. Garbe wrote: I see the problem in the above code, and also believe that your solution is more correct. What scares me is, that it might seen as valid input if someone feeds dwm with for example: echo -n foo during while... but we can fix this. Please lemme know if your guess is correct through using the above fprintf-debug trick. The question I see (apart from certain flaws in your patch) after all is: Do we really want that dwm's status text is displayed line-based? Isn't it fine as is, that it even does not break if input is feed without line-terminators? If we decide for line-based input reading, one easily could block dwm from further event processing through running echo -n foo | dwm I think that'd be a big disadvantage. Regards, -- Anselm R. Garbe http://www.suckless.org/ GPG key: 0D73F361
Re: [dwm] input status text bug?
Anselm R. Garbe [EMAIL PROTECTED] writes: If we decide for line-based input reading, one easily could block dwm from further event processing through running echo -n foo | dwm I think that'd be a big disadvantage. It could append read text to the end of stext[] whenever it successfully reads(), except that the first character after a '\n' clears stext[] and starts a new status. It needn't block. Best wishes, Chris.
Re: [dwm] input status text bug?
On 10/31/07, Sander van Dijk [EMAIL PROTECTED] wrote:
On Oct 31, 2007 7:01 AM, Vasil Dimov [EMAIL PROTECTED] wrote:
On Tue, Oct 30, 2007 at 23:04:33 -0400, Ritesh Kumar wrote:
[...]
I went through the source code and fixed the status input handling
code to
make sure it flushes the input status text only if in encounters a
'\n'.
[...]
if(FD_ISSET(STDIN_FILENO, rd)) {
- switch(r = read(STDIN_FILENO, inputtext,
sizeof inputtext - 1)) {
+ pos = inputtext[strlen(inputtext)-1] != '\n' ?
strlen(inputtext) : 0;
+ switch(r = read(STDIN_FILENO,
(inputtext[pos]), sizeof inputtext - 1 - pos)) {
[...]
Hmmz, aren't you trying to calculate strlen() of something undefined
here?
And also, when inputtext[0] = '\0', where does
inputtext[strlen(inputtext)-1] point to?
Nice catch! Actually, I didn't think of this when I first developed the
patch.
However, the read() call always returns atleast one char in inputtext for
the default switch case. So, we are always bound to have strlen(inputtext)
1 for all cases.
But I guess its better to be safe and put a check in for strlen(inputtext)
while calculating pos.
_r
Re: [dwm] input status text bug?
On 10/31/07, Anselm R. Garbe [EMAIL PROTECTED] wrote:
Hi Ritesh,
snip
Well let's have a look at the relevant bits in the code:
(a) switch(r = read(STDIN_FILENO, stext, sizeof stext - 1)) {
[...]
default:
(b) for(stext[r] = '\0', p = stext + strlen(stext) - 1; p =
stext *p == '\n'; *p-- = '\0');
(c) for(; p = stext *p != '\n'; --p);
(d) if(p stext)
(e) strncpy(stext, p + 1, sizeof stext);
[...]
(a) Reads the next bunch of stdin, beginning at stext[0] and
ending at stext[r].
(b) 0-terminates stext, and trims rightmost '\n' from right to left only.
(c) If p still does not point to stext, it now seeks for a '\n'
character leftwards.
(d) If during (c) such a '\n' character was found, it is the
begin of the last line which has been read.
(e) Hence, we copy the last line read only to stext, beginning
at the '\n' character + 1 to stext. It is 0-terminated already.
So in my eyes, assumed the complete output of the input script
is read, the code is correct and does what it is supposed
to do: it only displays the last line read from stdin (this might
be a bunch of data which is not '\n'-terminated yet during this read
round of course).
The problem is that select will notify dwm the moment *anything* is
available on stdin... and that could be partial data.
There is one semantic difference between the code snippets. While the
original one retains the last line of given multiline input, the suggested
snippet shows the first line. However, its hard to think of a situation
where somebody would pour in multiline text (all in a small amount of time)
for the status as dwm would not be able to show all of it anyways regardless
of the semantics.
If you like to debug, you could add an
fprintf(stderr, 'stext == %s'\n, stext);
in there between (c) and (d). Then we can make sure that your
guess is correct.
If the input is not read during one read call as you believe,
then at least there must be some flashing, because dwm will
actually render the some text at the status bar, but maybe
your system is really fast and you don't notice this.
I will do this and post the results.
You are right about my system... its really fast :). Actually, I did see a
flash but it was merely a flicker and happened only sometimes.
_r
Re: [dwm] input status text bug?
On 10/31/07, Ritesh Kumar [EMAIL PROTECTED] wrote:
On 10/31/07, Anselm R. Garbe [EMAIL PROTECTED] wrote:
Hi Ritesh,
snip
Well let's have a look at the relevant bits in the code:
(a) switch(r = read(STDIN_FILENO, stext, sizeof stext - 1)) {
[...]
default:
(b) for(stext[r] = '\0', p = stext + strlen(stext) - 1; p =
stext *p == '\n'; *p-- = '\0');
(c) for(; p = stext *p != '\n'; --p);
(d) if(p stext)
(e) strncpy(stext, p + 1, sizeof stext);
[...]
(a) Reads the next bunch of stdin, beginning at stext[0] and
ending at stext[r].
(b) 0-terminates stext, and trims rightmost '\n' from right to left
only.
(c) If p still does not point to stext, it now seeks for a '\n'
character leftwards.
(d) If during (c) such a '\n' character was found, it is the
begin of the last line which has been read.
(e) Hence, we copy the last line read only to stext, beginning
at the '\n' character + 1 to stext. It is 0-terminated already.
So in my eyes, assumed the complete output of the input script
is read, the code is correct and does what it is supposed
to do: it only displays the last line read from stdin (this might
be a bunch of data which is not '\n'-terminated yet during this read
round of course).
The problem is that select will notify dwm the moment *anything* is
available on stdin... and that could be partial data.
There is one semantic difference between the code snippets. While the
original one retains the last line of given multiline input, the suggested
snippet shows the first line. However, its hard to think of a situation
where somebody would pour in multiline text (all in a small amount of time)
for the status as dwm would not be able to show all of it anyways regardless
of the semantics.
If you like to debug, you could add an
fprintf(stderr, 'stext == %s'\n, stext);
in there between (c) and (d). Then we can make sure that your
guess is correct.
If the input is not read during one read call as you believe,
then at least there must be some flashing, because dwm will
actually render the some text at the status bar, but maybe
your system is really fast and you don't notice this.
I will do this and post the results.
You are right about my system... its really fast :). Actually, I did see a
flash but it was merely a flicker and happened only sometimes.
_r
Okay, I tried the debug: the output with the previous code is
inputtext == sometext
inputtext == Wed Oct 31 14:46:32 EDT 2007
inputtext == sometext
inputtext == Wed Oct 31 14:46:33 EDT 2007
inputtext == sometext
inputtext == Wed Oct 31 14:46:34 EDT 2007
inputtext == sometext
inputtext == Wed Oct 31 14:46:35 EDT 2007
inputtext == sometext
inputtext == Wed Oct 31 14:46:36 EDT 2007
So, read() is really not reading all of the 'intended text to be read' in
one go.
_r
[dwm] input status text bug?
Hi folks,
I came across this curious behavior for the input status text...
I used to run dwm using the following bash script:
while true; do
date;
sleep 1;
done | dwm
... and things used to run fine :).
I changed it to:
while true; do
cat $HOME/.dwm/status_* ;
date;
sleep 1;
done | dwm
The status_* files contain text without trailing \n's.
Surprisingly , the contents of the statusbar didn't change (only date, no
other status text) ... even though the while loop outputs something like
some text Tue Oct 30 22:46:56 EDT 2007
some text Tue Oct 30 22:46:57 EDT 2007
some text Tue Oct 30 22:46:58 EDT 2007
...
My guess is that read() reads the status_* text and date in two successive
calls causing the date to prevail till sleep 1 is done.
I went through the source code and fixed the status input handling code to
make sure it flushes the input status text only if in encounters a '\n'. The
patch is given... unfortunately it is over my (recently posted) status text
patch. I moved the trimming of trailing \n's to another function setstext()
(introduced by my status text patch)... however, the relevant code can be
moved to the function run().
What do you guys think... did I accidentally break something?
_r
diff -r a5b57a189379 dwm.c
--- a/dwm.c Tue Oct 30 22:17:27 2007 -0400
+++ b/dwm.c Tue Oct 30 22:31:24 2007 -0400
@@ -1295,8 +1295,7 @@ restack(void) {
void
run(void) {
- char *p;
- int r, xfd;
+ int r, pos, xfd;
fd_set rd;
XEvent ev;
@@ -1315,7 +1314,8 @@ run(void) {
eprint(select failed\n);
}
if(FD_ISSET(STDIN_FILENO, rd)) {
- switch(r = read(STDIN_FILENO, inputtext, sizeof
inputtext - 1)) {
+ pos = inputtext[strlen(inputtext)-1] != '\n' ?
strlen(inputtext) : 0;
+ switch(r = read(STDIN_FILENO, (inputtext[pos]),
sizeof inputtext - 1 - pos)) {
case -1:
strncpy(inputtext, strerror(errno), sizeof
inputtext - 1);
inputtext[sizeof inputtext - 1] = '\0';
@@ -1326,10 +1326,7 @@ run(void) {
readin = False;
break;
default:
- for(inputtext[r] = '\0', p = inputtext +
strlen(inputtext) - 1; p = inputtext *p == '\n'; *p-- = '\0');
- for(; p = inputtext *p != '\n'; --p);
- if(p inputtext)
- strncpy(inputtext, p + 1, sizeof
inputtext);
+ inputtext[pos+r] = '\0' ;
}
drawbar();
}
@@ -1426,6 +1423,9 @@ setstext(void) {
stext[0] = '\0';
for(i=0; isizeof(statusbar)/sizeof(char*); i++)
strncat(stext, statusbar[i], sizeof(stext) - strlen(stext));
+ // Trim the trailing \n's
+ for(i=strlen(stext)-1; stext[i]=='\n'; i--)
+ stext[i] = '\0';
}
Re: [dwm] input status text bug?
The full patch (against dwm 4.6 + clientspertag) is present at http://www.cs.unc.edu/~ritesh/dwm/index.html It should not be difficult to chop out unwanted bits from it :) _r On 10/30/07, Ritesh Kumar [EMAIL PROTECTED] wrote: Hi folks, I came across this curious behavior for the input status text... snip
Re: [dwm] input status text bug?
On Tue, Oct 30, 2007 at 23:04:33 -0400, Ritesh Kumar wrote:
[...]
I went through the source code and fixed the status input handling code to
make sure it flushes the input status text only if in encounters a '\n'.
[...]
if(FD_ISSET(STDIN_FILENO, rd)) {
- switch(r = read(STDIN_FILENO, inputtext, sizeof
inputtext - 1)) {
+ pos = inputtext[strlen(inputtext)-1] != '\n' ?
strlen(inputtext) : 0;
+ switch(r = read(STDIN_FILENO, (inputtext[pos]),
sizeof inputtext - 1 - pos)) {
[...]
Hmmz, aren't you trying to calculate strlen() of something undefined
here?
--
Vasil Dimov
[EMAIL PROTECTED]Software Developer @ Oracle/Innobase Oy
[EMAIL PROTECTED]Committer @ FreeBSD.org
[EMAIL PROTECTED]Home @ Sofia, Bulgaria
pgpr4itsL2xpO.pgp
Description: PGP signature
