Hi to all,
I discovered a little bug in hdparm.c
(really two little bugs...I've made...sigh! Mea culpa).
Some vars were modified only locally and this could lead to wrong
results to be displayed with the -I switch and maybe with others.
Attached is a patch that fix it ( +88b).
Also attached is second patch that reduces the size a little bit:
text data bss dec hex filename
27984 624 900 29508 7344 hdparm.o (without bug-fix)
28072 624 900 29596 739c hdparm.o (with bug-fix)
28141 624 900 29665 73e1 hdparm.o (original)
but maybe this one can wait as we are in a feature freeze.
Ciao,
Tito
sed -i "/^boo/a fred" ipsec.conf
Which works in gnu sed. (And is _supposed_ to strip all the whitespace before
"fred".)
It also broke:
sed -i -e "/^boo/a \\" -e " fred" ipsec.conf
I.E. there can legally be spaces between the a and the backslash at the end of
the line.
And strangely enough, gnu sed accepts the following syntax as well:
sed -i "/^boo/a \\ fred" ipsec.conf
Which is a way of having the significant whitespace at the start of the line,
all on one line. (But notice that the whitespace BEFORE the slash is still
stripped, as is the slash itself. And notice that the naieve placement of
"\n" there doesn't work, it puts an n at the start of the appended line. The
double slashing is for shell escapes because you could escape the quote, you
see. It's turned into a single backslash. But \n there is _not_ turned into
a newline by the shell. So there.)
This makes all three syntaxes work in my tests. I should probably start
writing better documentation at some point. I posted my current sedtests.py
file to the list, which needs a lot more tests added as well...
The sed command in busybox 1.0.0-pre8 loses leading whitespace
in 'a' command ('i' and 'c' commands are also affected). A
patch to fix this is attached at the end of this message.
The following is a transcript that reproduces the problem. The
first run uses busybox 1.0.0-pre3 as "/bin/sed" command, which
gets the expected result. Later in the test, /bin/sed symlink
is changed to point at busybox 1.0.0-pre8 and the test script is
run again, which shows the failure.
=== reproduction recipe ===
* Part 1. Use busybox 1.0.0-pre3 as sed; this works.
root# cd /tmp
root# cat 1.sh
#!/bin/sh
cd /tmp
rm -f ipsec.conf ipsec.conf+
cat >ipsec.conf <<\EOF
version 2.0
config setup
klipsdebug=none
plutodebug=none
plutostderrlog=/dev/null
conn %default
keyingtries=1
...
EOF
sed -e '/^config setup/a\
nat_traversal=yes' ipsec.conf >ipsec.conf+
mv -f ipsec.conf+ ipsec.conf
root# sh -x 1.sh
+ cd /tmp
+ rm -f ipsec.conf ipsec.conf+
+ cat
+ sed -e /^config setup/a\
nat_traversal=yes ipsec.conf
+ mv -f ipsec.conf+ ipsec.conf
root# cat ipsec.conf
version 2.0
config setup
nat_traversal=yes
klipsdebug=none
plutodebug=none
plutostderrlog=/dev/null
conn %default
keyingtries=1
...
root# sed --version
sed: invalid option -- -
BusyBox v1.00-pre3 (2004.02.26-18:47+0000) multi-call binary
Usage: sed [-nef] pattern [files...]
* Part 2. Continuing from the above, use busybox 1.0.0-pre8
as sed; this fails.
root# ln -s busybox-pre8 /bin/sed-8
root# mv /bin/sed-8 /bin/sed
root# sed --version
This is not GNU sed version 4.0
root# sed --
BusyBox v1.00-pre8 (2004.03.30-02:44+0000) multi-call binary
Usage: sed [-nef] pattern [files...]
root# sh -x 1.sh
+ cd /tmp
+ rm -f ipsec.conf ipsec.conf+
+ cat
+ sed -e /^config setup/a\
nat_traversal=yes ipsec.conf
+ mv -f ipsec.conf+ ipsec.conf
root# cat ipsec.conf
version 2.0
config setup
nat_traversal=yes
klipsdebug=none
plutodebug=none
plutostderrlog=/dev/null
conn %default
keyingtries=1
...
root#
=== reproduction recipe ends here ===
This problem was introduced in 1.0.0-pre4. The problem is that
the command argument parsing code strips leading whitespaces too
aggressively. When running the above example, the piece of code
in question gets "\n\tnat_traversal=yes" as its argument in
cmdstr variable (shown part in the following patch). What it
needs to do at this point is to strip the first newline and
nothing else, but it instead strips all the leading whitespaces
at the beginning of the string, thus losing the tab character.
The following patch fixes this.
Ok. Last patch reduce 73 bytes for compensate (and over) your changes ;-)
Comments:
Added cin_fileno variable, auto setted to 0 from BSS and have "eq" stdin
descriptor if isatty(stout)==0, removed global variable FILE* cin.
Removed default setting to terminal_width/terminal_height, this used
only from main() and setted after call get_terminal_width_height()
always correct.
Variable please_display_more_prompt changed to bits logic, have size
reducing.
--w
vodz
I've noticed a bug in the "autowidth" feature more, and is probably in
others. The call to the function get_terminal_width_height() passes
in a file descriptor but that file descriptor is never used, instead
the ioctl() is called with 0. In more_main() the call to
get_terminal_width_height() passes 0 as the file descriptor instead of
fileno(cin). This isn't a problem when you more a file (e.g. "more
/etc/passwd") but when you pipe a file to it (e.g. "cat /etc/passwd |
more") the size of the terminal cannot be determined because file
descriptor 0 is not a terminal. The fix is simple, I've attached a
patch for more.c and get_terminal_width_height.c.
BAPper
have checked in. Vladimir writes:
Your patch have many problem.
1. You always added + time(). This cannot reset RANDOM=value for debuging
with
replay sequential.
2. Hmm. I examine bash 2.04 source. This pseudorandom generator use low bits
of
counter value. You use high bits. This make bad pseudorandom values after
have
0-value. For example, if + time() do remove, your generator always return 0
after
first generate 0.
3. Memory leak per call. Use ash-unlike unecessary bb_strdup function.
4. Unsupport show last $RANDOM value for "set" and "export" command.
5. Bloat code. Busybox-unlike patch - added unstandart feature as default
hardcode.
Last patch attached.
Erik, why you apply Paul patch with have 5-th point problem? :(
Last patch have ash change xwrite() to fresh libbb/bb_full_write interfase
(haved loop after EINTR).
--w
vodz
Current `tr' implementation has a problem, if `plain char' is signed.
[current cvs version]
>echo a | _install/usr/bin/tr '\0' '\377'
Segmentation fault (core dumped)
[patched version]
>echo a | _install/usr/bin/tr '\0' '\377'
a
Here's a follow-up replacement to the patch I sent earlier, this adjusts some
of the semantics of the dynamic variable setting. Namely, dynamic vars can hook
a set handler (which RANDOM uses to adjust the seed). They'll only lose their
dynamic status if they're unset.
I've used the same approach that bash does to come up with the random number,
mostly just for consistency.
For example:
$ echo $RANDOM
13759
$ echo $RANDOM
20057
$ echo $RANDOM
1502
$ export RANDOM=42
$ echo $RANDOM
24179
$ echo $RANDOM
2046
$ unset RANDOM
$ echo $RANDOM
$ export RANDOM=42
$ echo $RANDOM
42
$
