Re: [Toybox] [PATCH] Fix the operator precedence in expr.
Rob, not sure if you're done with this yet, but the strategy for
freeing here doesn't quite work:
https://github.com/landley/toybox/commit/0ec95b7c2e20cd7be33bae6adba20bf89c5f3e86
I thought about exactly this: just free in eval_op(). But there are a
few problems -- for example:
1) re() allocates a string for the match
2) You free it in eval_op() free(v->s). You still have a struct
value, but it has an invalid pointer.
3) You use the returned string for another op
Basically something like:
expr foo : '\(.+\)' == foo
There are other possible bugs with that scheme too, e.g. involving
coercion. ASAN easily flags the memory leaks and regex tests fail.
With the strategy I sent, it doesn't detect any memory leaks.
(I will send a patch for adding make asantest, make asantest_expr,
etc. which is on top of my test harness fixes.)
-
Also, while your mind is on expr, and since you mentioned $(()) in
bash ... I happened to look at the mksh source code (Android shell),
and it uses the same algorithm for their $(()). This is in contrast
coreutils and busybox expr which use recursive descent.
https://github.com/MirBSD/mksh/blob/master/expr.c#L404 (loop that
breaks on precedence)
I guess the main difference is that there are unary operators there
like -- and ++, and a ternary ? : operator. I guess it's not much
info but I thought it was interesting! :)
Andy
On Wed, Mar 16, 2016 at 6:20 PM, Andy Chu wrote:
>> I applied your two patches and I've done one round of cleanup on top of
>> them so far. Then I started replacing "TT.tok = toys.optargs++" with
>> directly incrementing TT.tok and I introduced a bug with parentheses
>> management and got myself confused and went to do something else for a
>> while and haven't gotten back to it yet.
>
> Yeah it's a little fiddly, but elegant when you get used to the
> algorithm's logic. To fix the misleading error message on expr \( \),
> try this.
>
> + if (!strcmp(TT.tok, ")")) { // an atom can't start with )
> + error_exit("Unexpected )");
> - if (!strcmp(TT.tok, "(")) { // parenthesized expression
> + } else if (!strcmp(TT.tok, "(")) { // parenthesized expression
>
> The entry into eval_expr() expects an "atom", which is either
> something like "1" or "a" or a parenthesized expression like ( 1 + 2
> ).
>
> Note that expr \* is valid -- it treats it like a literal string. So
> expr \) was ALSO valid.
>
> expr \( \) \) was valid -- it is a string inside parentheses. Thus
> expr \( \) was being parsed as OPEN PAREN, STRING, and then ERROR,
> expected closing \). So it was technically correct, though very
> confusing This tiny patch will disallow things like expr \) and expr
> \( \) \).
>
> I actually had this check in my original version, but removed it
> because of minimalism, but didn't realize that confusing case.
>
>>> I wanted to bring up the one TODO in the patches... syntax_error()
>>> defaults to printing "syntax error", but the caller passes a more
>>> detailed error message, which can be enabled with "if (1)".
>>
>> This morning's cleanup pass replaced the calls to syntax_error() with
>> calls to error_exit(), making it always print the more detailed error
>> message.
>
> OK awesome! Makes sense.
>
>>> That would make it clearer that + is both a unary and binary operator,
>>> while - is only a binary operator.
>>
>> So my help text is wrong and not _all_ operators are infix.
>>
>> Sigh...
>
> No your help is correct! I didn't implement what GNU expr does (nor
> did existing code). It "puns" + as a unary escaping operator (in
> addition to infix addition).
>
> I wrote about this in my message about find / expr / test.
>
> It's because GNU has "match" / "substr" operators, which we don't
> have. This is the problem:
>
> $ expr foo = foo
> 1
>
> $ expr match = match
> expr: syntax error # it's expecting arguments to the operator "match"
>
> $ expr + match = + match # escape match operator as string
> 1
>
> I think the command is OK as is -- it's POSIX conformant... It's
> possible we could run into something in Aboriginal LInux that uses the
> match / substr operators, in which case we could add them and the
> annoying + unary escape.
>
> Andy
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
[Toybox] [PATCH] Fix a buffer overflow in diff -r.
We were doing two 32-byte memset()s instead of two 16-byte memset()s.
'dir' referred to the instance (array of 2) and not the struct type.
Add some test coverage for diff, including a case that hit this bug.
The bug was found by running cp.test under AddressSanitizer, since it
happens to use diff.
From 4945bdb5f0c87c8f8f69761de55ea23477343338 Mon Sep 17 00:00:00 2001
From: Andy Chu
Date: Sat, 19 Mar 2016 23:11:30 -0700
Subject: [PATCH] Fix a buffer overflow in diff -r.
We were doing two 32-byte memset()s instead of two 16-byte memset()s.
'dir' referred to the instance (array of 2) and not the struct type.
Add some test coverage for diff, including a case that hit this bug.
The bug was found by running cp.test under AddressSanitizer, since it
happens to use diff.
---
tests/diff.test | 30 ++
toys/pending/diff.c | 6 +++---
2 files changed, 33 insertions(+), 3 deletions(-)
create mode 100755 tests/diff.test
diff --git a/tests/diff.test b/tests/diff.test
new file mode 100755
index 000..ca0b682
--- /dev/null
+++ b/tests/diff.test
@@ -0,0 +1,30 @@
+#!/bin/bash
+
+#testing "name" "command" "result" "infile" "stdin"
+
+seq 10 > left
+seq 11 > right
+
+expected='--- left
right
+@@ -8,3 +8,4 @@
+ 8
+ 9
+ 10
++11
+'
+# Hm this only gives unified diffs?
+testing "simple" "diff left right" "$expected" "" ""
+
+
+expected='--- tree1/file
tree2/file
+@@ -1 +1 @@
+-foo
++food
+'
+mkdir -p tree1 tree2
+echo foo > tree1/file
+echo food > tree2/file
+
+testing "simple" "diff -r tree1 tree2 |tee out" "$expected" "" ""
diff --git a/toys/pending/diff.c b/toys/pending/diff.c
index da6c13a..53bdbce 100644
--- a/toys/pending/diff.c
+++ b/toys/pending/diff.c
@@ -59,7 +59,7 @@ struct diff {
long a, b, c, d, prev, suff;
};
-static struct dir {
+static struct dir_t {
char **list;
int nr_elm;
} dir[2];
@@ -69,7 +69,7 @@ struct candidate {
struct candidate *prev, *next;
};
-static struct file {
+static struct file_t {
FILE *fp;
int len;
} file[2];
@@ -797,7 +797,7 @@ void diff_main(void)
if (S_ISDIR(st[0].st_mode) && S_ISDIR(st[1].st_mode)) {
for (j = 0; j < 2; j++) {
- memset(&dir[j], 0, sizeof(dir));
+ memset(&dir[j], 0, sizeof(struct dir_t));
dirtree_flagread(files[j], DIRTREE_SYMFOLLOW, list_dir);
dir[j].nr_elm = TT.size; //size updated in list_dir
qsort(&(dir[j].list[1]), (TT.size - 1), sizeof(char*), cmp);
--
1.9.1
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
[Toybox] [PATCH] Fix a small buffer overflow in 'rev' when passed an empty line.
Caught by running test_rev under AddressSanitizer. When the length of
the buffer was 0, it would still try to swap characters due to an off by
one bug.
From c617df571cfbf20c7e19d0097f768e53fc1a9777 Mon Sep 17 00:00:00 2001
From: Andy Chu
Date: Sat, 19 Mar 2016 22:23:35 -0700
Subject: [PATCH] Fix a small buffer overflow in 'rev' when passed an empty
line.
Caught by running test_rev under AddressSanitizer. When the length of
the buffer was 0, it would still try to swap characters due to an off by
one bug.
---
toys/other/rev.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/toys/other/rev.c b/toys/other/rev.c
index 4cf7214..a61957c 100644
--- a/toys/other/rev.c
+++ b/toys/other/rev.c
@@ -23,12 +23,11 @@ static void do_rev(int fd, char *name)
int len, i;
if (!(c = get_line(fd))) break;
-len = strlen(c) - 1;
-for (i = 0; i <= len/2; i++) {
+len = strlen(c);
+for (i = 0; i < len/2; i++) {
char tmp = c[i];
-
- c[i] = c[len-i];
- c[len-i] = tmp;
+ c[i] = c[len-1-i];
+ c[len-1-i] = tmp;
}
xputs(c);
free(c);
--
1.9.1
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] testing / branching (was Re: [PATCH] Fix the operator precedence in expr.)
On Wed, Mar 16, 2016 at 5:18 PM, Rob Landley wrote: > On 03/16/2016 03:51 PM, Andy Chu wrote: >>> I often fix the test case before I fix the code, to remind me to fix the >>> code. :) >> >> Yes I think it's a good idea to add failing tests that are known to be >> good, but not done yet. I think we could annotate them as follows: >> >> testing "name" "command" "result" "infile" "stdin" # good test >> known-fail "name" "command" "result" "infile" "stdin" # test that >> fails, but is valuable >> >> If we had a continuous build, we could keep it green, preventing >> regressions of the tests that do actually pass. > > Regular testing is nice, but I was convinced that it's no substitute for > regular time-based releases by this video: continuous testing is a tool that enables you to have regular time-based releases that work. you always want to know if your change broke something, regardless of whether you're doing feature- or time-based releases. > http://www.youtube.com/watch?v=IKsQsxubuAA > > April 19, 2007 Release Management in Large Free Software Projects - > Martin Michlmayr (Debian) > > ABSTRACT: Time based releases are made according to a specific time > interval, instead of making a release when a particular functionality or > set of features have been implemented. This talk argues that time based > release management acts as an effective coordination mechanism in large > volunteer projects and shows examples from seven projects that have > moved to time based releases: Debian, GCC, GNOME, Linux, OpenOffice, > Plone, and X.org. > > (That's why "ballpark 3 months after last release, we should have > another one" last message.) > >> And then there could be a mode of the test harness that runs the >> known-fail tests. If they pass, the function can be changed to >> "testing". Although there is the complication that they could pass in >> one environment but not another. > > For 1.0 I want to _fix_ all the known-fail tests. Right now the sad > state of the testing suite is, in part, a way of tracking my todo items. :P > > That said, we can rename "blah.tests" to "blah.pending" and have tests > with known failures be in the latter category, moving over to blah.tests > once a test is good and fixed and everything in it runs _and_ we're > pretty happy with the coverage it provides. > >>> (I don't always check those in though, because "git diff" showing me a >>> comment or hunk of code is an easy way of keeping track of todo items.) >> >> How about just working on a branch "rob-todo" ? And then "git diff >> master.." instead. And then 'git pull' becomes 'git pull' and 'git >> merge master'. > > I don't make very deep use of git. I have multiple working directories > when I really need them, and I try to avoid merge commits in the > history. I know, I should get with the 21st century... > >> I noticed you just switched from hg, and so did I. I liked hg a lot >> better because the commands actually make sense, but git's branching >> is why it's popular. It really is pretty killer and lets me work on >> the test harness while you are reviewing the expr code, etc. > > I still use multiple directories instead of branches because of said > history with mercurial. Too busy using the tools to play with the tools > so far. (Also still at the point where when I break it, and oh boy do I > break everything, I don't necessarily know how to fix it and just revert > to a backup instead.) > >> The way I think of it is that hg is for single person projects (most >> of mine were) while git is for collaboration... and that explains why >> git is popular despite being incomprehensible. > > No, git won because Linux was using it, and everybody else copied Linux > as usual. Same way the GPL won because it was the Linux license, until > the FSF split it into incompatible factions and set them at war with > each other. (Copying Sun's "CDDL to fracture the GPL" attack from a few > years earlier, Sun didn't have the weight to pull it off, but the FSF > did and destroyed its ecosystem. Pity, I was kind of invested in that at > the time. Oh well, we all move on.) > > In theory git and mercurial could each do anything the other could. In > practice mercurial had some very strong ideas about what should be > allowed (they kept trying to dissuade me from using hg rollback, and > would never give me multiple levels of that. In git: "git reset HEAD^1" > and repeat until you're where you want to be, it's all fine if you > haven't pushed yet). > > Git has the annoying tick that the way to understand its command line is > to learn how its' on-disk format is implemented. (No really, THAT'S WHAT > THEY EXPECT YOU TO DO. Grumble grumble.) Luckily, there are some > reasonable videos on this. I saw the Linuxcon Japan 2015 presentation of: > > https://www.youtube.com/watch?v=I-lGyn3PHP4 > >> I find it really helps to put the git branch name in your bash prompt >> (if you want a pointer let me know.) > > One of my todo items for the post-1.
[Toybox] [PATCH] Fix bug where all tests aren't being run with 'make test'.
The tests/*.test files shouldn't explicitly exit, because they are sourced in scripts/test.sh. No tests after sed were being run. From 6ef2afe0e206e71b734541db6c417753f8f84e63 Mon Sep 17 00:00:00 2001 From: Andy Chu Date: Thu, 17 Mar 2016 00:06:32 -0700 Subject: [PATCH] Fix bug where all tests aren't being run with 'make test'. The tests/*.test files shouldn't explicitly exit, because they are sourced in scripts/test.sh. No tests after sed were being run. --- tests/sed.test | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/sed.test b/tests/sed.test index ec06baa..b308b69 100755 --- a/tests/sed.test +++ b/tests/sed.test @@ -151,6 +151,3 @@ testing "bonus backslashes" \ "sed -e 'a \l \x\' -e \"\$(echo -e 'ab\\\nc')\"" \ "hello\nl x\nab\nc\n" "" "hello\n" # -i with $ last line test - - -exit $FAILCOUNT -- 1.9.1 ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH] Fix bug where all tests aren't being run with 'make test'.
Another patch in the same vein... I thoguht xargs was alphabetically last, but no! It was preventing 3 more tests from running. Andy On Thu, Mar 17, 2016 at 12:11 AM, Andy Chu wrote: > The tests/*.test files shouldn't explicitly exit, because they are > sourced in scripts/test.sh. No tests after sed were being run. From f0223a67527d7c79a213288eb87a7c6e5a253ecc Mon Sep 17 00:00:00 2001 From: Andy Chu Date: Thu, 17 Mar 2016 12:02:55 -0700 Subject: [PATCH 2/2] Remove 'exit' from xargs test too. --- tests/xargs.test | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/xargs.test b/tests/xargs.test index c99de40..966bc5d 100755 --- a/tests/xargs.test +++ b/tests/xargs.test @@ -24,12 +24,10 @@ testing "command -opt" "xargs -n2 ls -1" "one\ntwo\nthree\n" "" \ "one two three" rm one two three -exit - -testing "-n exact match" -testing "-s exact match" -testing "-s 0" -testing "-s impossible" +#testing "-n exact match" +#testing "-s exact match" +#testing "-s 0" +#testing "-s impossible" # xargs command_not_found - returns 127 # xargs false - returns 1 -- 1.9.1 ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH] Fix the operator precedence in expr.
On Wed, Mar 16, 2016 at 3:00 PM, Rob Landley wrote: > I've already started cleanup on this command, but I'll integrate this > with what I've got. > > Sounds like fun. I hope to have this done and promoted for the next > release. (Ballpark end of April.) OK great! I'd like to see the diff for future reference -- I've looked through your cleanup page. Perhaps you can apply at least the first 2 patches verbatim (as long as there are no egregious errors), and then your cleanup commits, so I can just look at the history? I wanted to bring up the one TODO in the patches... syntax_error() defaults to printing "syntax error", but the caller passes a more detailed error message, which can be enabled with "if (1)". If it were my choice I would always use the verbose error message and get rid of the if. I added the option for the cryptic message because I thought that was the style of the project -- a smaller vocabulary and less space for constant strings (?). But if I have to debug this code again, I much prefer each error message to have a unique message, so you don't have to go into a debugger to see what happened (and also for general usability). In fact I should have probably tested each of the syntax_error() calls like this: testing "error: missing )" "expr \( 5 + 5 2>&1" "expr: Expected )" ... To illustrate the advantages, I think you wouldn't have been mystified in your blog post about this behavior if GNU expr had better error messages: $ expr + 1 1 $ expr - 1 expr: syntax error $ expr + expr: syntax error $ expr - - I would have written this as: $ expr - 1 expr: expected atom, got operator - $ expr + expr: escape requires operand That would make it clearer that + is both a unary and binary operator, while - is only a binary operator. thanks, Andy Andy ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH] Fix the operator precedence in expr.
On 03/16/2016 06:44 PM, Andy Chu wrote: > On Wed, Mar 16, 2016 at 3:00 PM, Rob Landley wrote: >> I've already started cleanup on this command, but I'll integrate this >> with what I've got. >> >> Sounds like fun. I hope to have this done and promoted for the next >> release. (Ballpark end of April.) > > OK great! I'd like to see the diff for future reference First pass was pushed to https://github.com/landley/toybox around lunchtime: https://github.com/landley/toybox/commit/1af3bad8b63b > -- I've > looked through your cleanup page. Perhaps you can apply at least the > first 2 patches verbatim (as long as there are no egregious errors), > and then your cleanup commits, so I can just look at the history? I applied your two patches and I've done one round of cleanup on top of them so far. Then I started replacing "TT.tok = toys.optargs++" with directly incrementing TT.tok and I introduced a bug with parentheses management and got myself confused and went to do something else for a while and haven't gotten back to it yet. > I wanted to bring up the one TODO in the patches... syntax_error() > defaults to printing "syntax error", but the caller passes a more > detailed error message, which can be enabled with "if (1)". This morning's cleanup pass replaced the calls to syntax_error() with calls to error_exit(), making it always print the more detailed error message. > If it were my choice I would always use the verbose error message and > get rid of the if. I added the option for the cryptic message because > I thought that was the style of the project -- a smaller vocabulary > and less space for constant strings (?). It's not less space for constant strings, it's an intentionally restricted vocabulary to be less hard on non-english speakers. I don't have a direct jump-to#tag in http://landley.net/toybox/design.html but it's "error messages and internationalization". (And I probably should add an index with #tags to that page...) > But if I have to debug this code again, I much prefer each error > message to have a unique message, so you don't have to go into a > debugger to see what happened (and also for general usability). In > fact I should have probably tested each of the syntax_error() calls > like this: > > testing "error: missing )" "expr \( 5 + 5 2>&1" "expr: Expected )" ... By the way, "expr ( )" printed "missing )", I have a better error message in the current one but as I said: buggy at present. > To illustrate the advantages, I think you wouldn't have been mystified > in your blog post about this behavior if GNU expr had better error > messages: > > $ expr + 1 > 1 > > $ expr - 1 > expr: syntax error > > $ expr + > expr: syntax error > > $ expr - > - > > I would have written this as: > > $ expr - 1 > expr: expected atom, got operator - > > $ expr + > expr: escape requires operand > > That would make it clearer that + is both a unary and binary operator, > while - is only a binary operator. So my help text is wrong and not _all_ operators are infix. Sigh... (I did a cleanup pass earlier, and often with cleanup I start with the help text. What SHOULD this command be doing? Documentation is _not_ an afterthought...) Rob ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH SET] Fix test harness issues and fix tests
Forgot to change this one line. From 12ff9bf88c3695a57b9a615a105c9cc7f7e4159e Mon Sep 17 00:00:00 2001 From: Andy Chu Date: Sat, 19 Mar 2016 11:04:09 -0700 Subject: [PATCH 3/3] Small fix to make.sh: use OUTNAME instead of toybox --- scripts/make.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/make.sh b/scripts/make.sh index ab2719c..cc85b30 100755 --- a/scripts/make.sh +++ b/scripts/make.sh @@ -309,6 +309,6 @@ fi # its output the way SUSv4 suggests it do so. While we're at it, make sure # we don't have the "w" bit set so things like bzip2's "cp -f" install don't # overwrite our binary through the symlink. -do_loudly chmod 555 toybox || exit 1 +do_loudly chmod 555 $OUTNAME || exit 1 echo -- 1.9.1 ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH] Fix the operator precedence in expr.
On 03/16/2016 01:34 PM, Andy Chu wrote:
>> if (p<*toys.optargs || p>toys.optargs[toys.optc-1]) not_argv();
>>
>> It would be nice to free this data ourselves because expr is close to
>> $((1+2)) in the shell, and it would be nice if the shell could just
>> internall call expr nofork in that case.
>
> I attached to a patch to track the strings allocated and then free
> them at the end.
I've already started cleanup on this command, but I'll integrate this
with what I've got.
> (Note that there is NOT a "node" per argument ('struct value' which
> which contains an int or string). The nodes are on the stack and used
> destructively in the style of a *= b, a += b, etc. What we are
> talking about is the strings that the nodes (may) point to. These are
> almost always argv strings which don't need to be freed, but in the 2
> cases I mentioned, we allocate them.
I noticed that during review earlier today, but hadn't gotten to the
point of doing anything about that yet. :)
> So now this is fixed, and I tested it with ASAN (with LLVM 3.8 which
> can be downloaded easily). It easily finds the memory leak before
> (only the regex test cases where we allocate memory fail, not every
> test case) and confirms they pass after the patch.
>
> ASAN also found another existing memory leak in the code! (not
> calling regfree()) When I'm triaging the tests I'll definitely see
> which ones pass under ASAN too.
Sounds like fun. I hope to have this done and promoted for the next
release. (Ballpark end of April.)
> Andy
Rob
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
[Toybox] [PATCH] Make lsof 10x faster by caching /proc/net socket info.
I deliberately didn't do this first time round because for me our lsof
is already 10x faster than traditional lsof, and caching means we potentially
give less information about a socket that's created while we're running.
It turns out that traditional lsof caches anyway, so I guess nobody cares.
This also fixes a mistake where lsof used CFG_FREE instead of CFG_TOYBOX_FREE.
---
toys/pending/lsof.c | 96 ++---
1 file changed, 61 insertions(+), 35 deletions(-)
From 24412b61c52a591d4c14be566f2c025ea59bf097 Mon Sep 17 00:00:00 2001
From: Elliott Hughes
Date: Sat, 19 Mar 2016 08:56:09 -0700
Subject: [PATCH] Make lsof 10x faster by caching /proc/net socket info.
I deliberately didn't do this first time round because for me our lsof
is already 10x faster than traditional lsof, and caching means we potentially
give less information about a socket that's created while we're running.
It turns out that traditional lsof caches anyway, so I guess nobody cares.
This also fixes a mistake where lsof used CFG_FREE instead of CFG_TOYBOX_FREE.
---
toys/pending/lsof.c | 96 ++---
1 file changed, 61 insertions(+), 35 deletions(-)
diff --git a/toys/pending/lsof.c b/toys/pending/lsof.c
index 3a1c7f7..e99b011 100644
--- a/toys/pending/lsof.c
+++ b/toys/pending/lsof.c
@@ -26,6 +26,7 @@ GLOBALS(
struct stat *sought_files;
+ struct double_list *all_sockets;
struct double_list *files;
int last_shown_pid;
int shown_header;
@@ -81,11 +82,12 @@ static void print_info(void *data)
fi->fd, fi->rw, fi->locks, fi->type, fi->device, fi->size_off,
fi->node, fi->name);
}
+}
- if (CFG_FREE) {
-free(((struct file_info *)data)->name);
-free(data);
- }
+static void free_info(void *data)
+{
+ free(((struct file_info *)data)->name);
+ free(data);
}
static void fill_flags(struct file_info *fi)
@@ -109,46 +111,49 @@ static void fill_flags(struct file_info *fi)
fclose(fp);
}
-static int scan_proc_net_file(char *path, int family, char type,
-void (*fn)(char *, int, char, struct file_info *, long),
-struct file_info *fi, long sought_inode)
+static void scan_proc_net_file(char *path, int family, char type,
+void (*fn)(char *, int, char))
{
FILE *fp = fopen(path, "r");
char *line = NULL;
size_t line_length = 0;
- if (!fp) return 0;
+ if (!fp) return;
- if (!getline(&line, &line_length, fp)) return 0; // Skip header.
+ if (!getline(&line, &line_length, fp)) return; // Skip header.
while (getline(&line, &line_length, fp) > 0) {
-fn(line, family, type, fi, sought_inode);
-if (fi->name != 0) break;
+fn(line, family, type);
}
free(line);
fclose(fp);
+}
- return fi->name != 0;
+static struct file_info *add_socket(ino_t inode, const char *type)
+{
+ struct file_info *fi = xzalloc(sizeof(struct file_info));
+
+ dlist_add_nomalloc(&TT.all_sockets, (struct double_list *)fi);
+ fi->st_ino = inode;
+ strcpy(fi->type, type);
+ return fi;
}
-static void match_unix(char *line, int af, char type, struct file_info *fi,
- long sought_inode)
+static void scan_unix(char *line, int af, char type)
{
long inode;
int path_pos;
- if (sscanf(line, "%*p: %*X %*X %*X %*X %*X %lu %n", &inode, &path_pos) >= 1 &&
-inode == sought_inode) {
+ if (sscanf(line, "%*p: %*X %*X %*X %*X %*X %lu %n", &inode, &path_pos) >= 1) {
+struct file_info *fi = add_socket(inode, "unix");
char *name = chomp(line + path_pos);
-strcpy(fi->type, "unix");
fi->name = strdup(*name ? name : "socket");
}
}
-static void match_netlink(char *line, int af, char type, struct file_info *fi,
- long sought_inode)
+static void scan_netlink(char *line, int af, char type)
{
unsigned state;
long inode;
@@ -159,18 +164,17 @@ static void match_netlink(char *line, int af, char type, struct file_info *fi,
"ENCRYPTFS", "RDMA", "CRYPTO"
};
- if (sscanf(line, "%*p %u %*u %*x %*u %*u %*u %*u %*u %lu",
- &state, &inode) < 2 || inode != sought_inode) {
+ if (sscanf(line, "%*p %u %*u %*x %*u %*u %*u %*u %*u %lu", &state, &inode)
+ < 2) {
return;
}
- strcpy(fi->type, "netlink");
+ struct file_info *fi = add_socket(inode, "netlink");
fi->name =
strdup(state < ARRAY_LEN(netlink_states) ? netlink_states[state] : "?");
}
-static void match_ip(char *line, int af, char type, struct file_info *fi,
- long sought_inode)
+static void scan_ip(char *line, int af, char type)
{
char *tcp_states[] = {
"UNKNOWN", "ESTABLISHED", "SYN_SENT", "SYN_RECV", "FIN_WAIT1", "FIN_WAIT2",
@@ -198,9 +202,9 @@ static void match_ip(char *line, int af, char type, struct file_info *fi,
&(remote.s6_addr32[2]), &(remote.s6_addr32[3]),
&remote_port, &state, &inode) == 12;
}
- if (!ok || inode != sought_inode) return;
+ if (
Re: [Toybox] [PATCH] lsof should use CFG_TOYBOX_FREE, not CFG_FREE.
(i've rolled this into the 10x faster patch, since this code changes more drastically there.) On Fri, Mar 18, 2016 at 8:22 PM, enh wrote: > --- > toys/pending/lsof.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) -- Elliott Hughes - http://who/enh - http://jessies.org/~enh/ Android native code/tools questions? Mail me/drop by/add me as a reviewer. ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH] Use the correct types for recvfrom.
On Sat, Mar 19, 2016 at 7:56 AM, Rob Landley wrote: > > > On 03/18/2016 10:31 PM, enh wrote: >> On Fri, Mar 4, 2016 at 2:28 PM, Rob Landley wrote: > ... >>> My pending cleanup of lsof needs to address the fact it takes 18 seconds >>> to produce its first line of output when run with no arguments on my >>> netbook (the ubuntu version takes 0.7). That's a largeish thing. >> >> do you know why? for me on my desktop, toybox lsof is 10x faster than >> the GNU one (2s vs 18s). (that's time to complete. but the initial >> pause is longer for the GNU one too.) > > Not yet. Haven't cycled back around to that todo item yet. i had a look, and most of the time taken by our lsof goes into checking the /proc/net files over and over. i'll send out a patch adding a cache that makes lsof 10x faster. (that is, for me our lsof is now 100x faster than the traditional one, 0.19s instead of 18s.) Also, did you say a few months back that you'd started on 'ioctl'? If so, do you want to check that in? If not, correct my misapprehension and I'll do it at some point. (For that and prctl I'm also slowed down by the fact that there's much to dislike about the existing commands...) >>> >>> Oh right, that. I should go do that. >> >> ping. others can't start fixing it until something's checked in. >> >> (i think it's fine to have a directory even more broken than pending/ >> if that makes you more comfortable.) > > toys/pending/sh.c doesn't resolve environment variables, or handle pipes > or & > > toys/pending/mke2fs.c only makes a superblock, and does it wrong. (I > have the infrastructure to do that right now, just haven't gotten back > to it. I've been looking at fat instead.) > > toys/pending/ping.c I got distracted from finishing _five_times_ and I > don't think it currently does anything if you run it. (I had other > versions contributed but very much wanted to get ipv4 and ipv6 in the > same binary, and base it on https://lwn.net/Articles/420799/ instead of > stuff requiring root access, and it doesn't take that much to implement...) > > Seriously, pending has a lot of broken. it varies wildly though, from "barely started" to "almost done". > But yeah, I should do ioctl this weekend. > >>> Rob > > Still Rob -- Elliott Hughes - http://who/enh - http://jessies.org/~enh/ Android native code/tools questions? Mail me/drop by/add me as a reviewer. ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH] Fix "tail -f single_file".
Blah, I fixed this on my netbook (and another fix on top of it, he added an extra space before the first ==> line, and added tail -f to the test suite)... But forgot to push it to github. Oops. Rob On 03/18/2016 10:16 PM, enh wrote: > ping... > > (it's friday again :-) ) > > On Wed, Mar 16, 2016 at 3:52 PM, Josh Gao wrote: >> TT.file_no was being incorrectly calculated as 0 when tail -f was passed a >> single argument, so tail -f with one argument wouldn't actually do the '-f' >> part. >> >> Patch attached. >> >> ___ >> Toybox mailing list >> Toybox@lists.landley.net >> http://lists.landley.net/listinfo.cgi/toybox-landley.net >> > > > ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH] Use the correct types for recvfrom.
On 03/18/2016 10:31 PM, enh wrote: > On Fri, Mar 4, 2016 at 2:28 PM, Rob Landley wrote: ... >> My pending cleanup of lsof needs to address the fact it takes 18 seconds >> to produce its first line of output when run with no arguments on my >> netbook (the ubuntu version takes 0.7). That's a largeish thing. > > do you know why? for me on my desktop, toybox lsof is 10x faster than > the GNU one (2s vs 18s). (that's time to complete. but the initial > pause is longer for the GNU one too.) Not yet. Haven't cycled back around to that todo item yet. >>> Also, did you say a few months back that you'd started on 'ioctl'? If >>> so, do you want to check that in? If not, correct my misapprehension >>> and I'll do it at some point. (For that and prctl I'm also slowed >>> down by the fact that there's much to dislike about the existing >>> commands...) >> >> Oh right, that. I should go do that. > > ping. others can't start fixing it until something's checked in. > > (i think it's fine to have a directory even more broken than pending/ > if that makes you more comfortable.) toys/pending/sh.c doesn't resolve environment variables, or handle pipes or & toys/pending/mke2fs.c only makes a superblock, and does it wrong. (I have the infrastructure to do that right now, just haven't gotten back to it. I've been looking at fat instead.) toys/pending/ping.c I got distracted from finishing _five_times_ and I don't think it currently does anything if you run it. (I had other versions contributed but very much wanted to get ipv4 and ipv6 in the same binary, and base it on https://lwn.net/Articles/420799/ instead of stuff requiring root access, and it doesn't take that much to implement...) Seriously, pending has a lot of broken. But yeah, I should do ioctl this weekend. >> Rob Still Rob ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH] Fix the operator precedence in expr.
> if (p<*toys.optargs || p>toys.optargs[toys.optc-1]) not_argv();
>
> It would be nice to free this data ourselves because expr is close to
> $((1+2)) in the shell, and it would be nice if the shell could just
> internall call expr nofork in that case.
I attached to a patch to track the strings allocated and then free
them at the end.
(Note that there is NOT a "node" per argument ('struct value' which
which contains an int or string). The nodes are on the stack and used
destructively in the style of a *= b, a += b, etc. What we are
talking about is the strings that the nodes (may) point to. These are
almost always argv strings which don't need to be freed, but in the 2
cases I mentioned, we allocate them.
So now this is fixed, and I tested it with ASAN (with LLVM 3.8 which
can be downloaded easily). It easily finds the memory leak before
(only the regex test cases where we allocate memory fail, not every
test case) and confirms they pass after the patch.
ASAN also found another existing memory leak in the code! (not
calling regfree()) When I'm triaging the tests I'll definitely see
which ones pass under ASAN too.
Andy
From 1069d89210edbf2b16571fa905a4e30eebde9e80 Mon Sep 17 00:00:00 2001
From: Andy Chu
Date: Wed, 16 Mar 2016 11:21:50 -0700
Subject: [PATCH 3/3] Track and free all strings allocated during 'expr'
evaluation.
Also fix a pre-existing memory leak in re() by calling regfree().
---
toys/pending/expr.c | 29 ++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/toys/pending/expr.c b/toys/pending/expr.c
index 71d5d19..4a49371 100644
--- a/toys/pending/expr.c
+++ b/toys/pending/expr.c
@@ -48,6 +48,7 @@ config EXPR
GLOBALS(
char* tok; // current token, not on the stack since recursive calls mutate it
+ struct arg_list *allocated; // list of strings allocated during evaluation
)
// Scalar value. If s != NULL, it's a string, otherwise it's an int.
@@ -69,6 +70,14 @@ void syntax_error(char *msg, ...) {
#define LONG_LONG_MAX_LEN 21
+// Keep track of an allocated string.
+void track_str(char* str) {
+ struct arg_list *node = xmalloc(sizeof(struct arg_list));
+ node->arg = str;
+ node->next = TT.allocated;
+ TT.allocated = node;
+}
+
// Get the value as a string.
void get_str(struct value *v, char** ret)
{
@@ -77,6 +86,7 @@ void get_str(struct value *v, char** ret)
else {
*ret = xmalloc(LONG_LONG_MAX_LEN);
snprintf(*ret, LONG_LONG_MAX_LEN, "%lld", v->i);
+track_str(*ret); // free it later
}
}
@@ -118,9 +128,10 @@ static void re(char *target, char *pattern, struct value *ret)
xregcomp(&pat, pattern, 0);
if (!regexec(&pat, target, 2, m, 0) && m[0].rm_so == 0) { // match at pos 0
regmatch_t* g1 = &m[1]; // group capture 1
-if (pat.re_nsub > 0 && g1->rm_so >= 0) // has capture
+if (pat.re_nsub > 0 && g1->rm_so >= 0) { // has capture
ret->s = xmprintf("%.*s", g1->rm_eo - g1->rm_so, target + g1->rm_so);
-else
+ track_str(ret->s); // free it later
+} else
assign_int(ret, m[0].rm_eo);
} else { // no match
if (pat.re_nsub > 0) // has capture
@@ -128,6 +139,7 @@ static void re(char *target, char *pattern, struct value *ret)
else
assign_int(ret, 0);
}
+ regfree(&pat);
}
// 4 different signatures of operators. S = string, I = int, SI = string or
@@ -271,5 +283,16 @@ void expr_main(void)
if (ret.s) printf("%s\n", ret.s);
else printf("%lld\n", ret.i);
- exit(is_false(&ret));
+ int status = is_false(&ret);
+
+ // Free all the strings we allocated.
+ struct arg_list *h, *head = TT.allocated;
+ while (head) {
+h = head;
+head = head->next;
+free(h->arg); // free the string
+free(h); // free the node for tracking the string
+ }
+
+ exit(status);
}
--
1.9.1
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
Re: [Toybox] [PATCH] Fix the operator precedence in expr.
> I applied your two patches and I've done one round of cleanup on top of
> them so far. Then I started replacing "TT.tok = toys.optargs++" with
> directly incrementing TT.tok and I introduced a bug with parentheses
> management and got myself confused and went to do something else for a
> while and haven't gotten back to it yet.
Yeah it's a little fiddly, but elegant when you get used to the
algorithm's logic. To fix the misleading error message on expr \( \),
try this.
+ if (!strcmp(TT.tok, ")")) { // an atom can't start with )
+ error_exit("Unexpected )");
- if (!strcmp(TT.tok, "(")) { // parenthesized expression
+ } else if (!strcmp(TT.tok, "(")) { // parenthesized expression
The entry into eval_expr() expects an "atom", which is either
something like "1" or "a" or a parenthesized expression like ( 1 + 2
).
Note that expr \* is valid -- it treats it like a literal string. So
expr \) was ALSO valid.
expr \( \) \) was valid -- it is a string inside parentheses. Thus
expr \( \) was being parsed as OPEN PAREN, STRING, and then ERROR,
expected closing \). So it was technically correct, though very
confusing This tiny patch will disallow things like expr \) and expr
\( \) \).
I actually had this check in my original version, but removed it
because of minimalism, but didn't realize that confusing case.
>> I wanted to bring up the one TODO in the patches... syntax_error()
>> defaults to printing "syntax error", but the caller passes a more
>> detailed error message, which can be enabled with "if (1)".
>
> This morning's cleanup pass replaced the calls to syntax_error() with
> calls to error_exit(), making it always print the more detailed error
> message.
OK awesome! Makes sense.
>> That would make it clearer that + is both a unary and binary operator,
>> while - is only a binary operator.
>
> So my help text is wrong and not _all_ operators are infix.
>
> Sigh...
No your help is correct! I didn't implement what GNU expr does (nor
did existing code). It "puns" + as a unary escaping operator (in
addition to infix addition).
I wrote about this in my message about find / expr / test.
It's because GNU has "match" / "substr" operators, which we don't
have. This is the problem:
$ expr foo = foo
1
$ expr match = match
expr: syntax error # it's expecting arguments to the operator "match"
$ expr + match = + match # escape match operator as string
1
I think the command is OK as is -- it's POSIX conformant... It's
possible we could run into something in Aboriginal LInux that uses the
match / substr operators, in which case we could add them and the
annoying + unary escape.
Andy
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
[Toybox] date test failures
On the master branch, I'm getting these failures. The difference is "Wed" vs "Sat" in the error message. Elliot, it looks like you added these tests -- are you getting the same failures? Is it a libc thing? (I'm on Ubuntu 14.04 but I tried on an older machine too and the failure is the same.) I don't really understand what it's trying to tell me though... FYI GNU coreutils gives: $ date 1438053157 date: invalid date ‘1438053157’ $ make VERBOSE=1 test_date ... PASS: date -d 111014312015.30 FAIL: date Unix time missing @ echo -ne '' | TZ=UTC date 1438053157 2>&1 --- expected2016-03-19 00:40:21.056862385 -0700 +++ actual 2016-03-19 00:40:21.056862385 -0700 @@ -1 +1 @@ -date: bad date '1438053157'; Wed February 38 05:31:00 UTC 2057 != Sun Mar 10 05:31:00 UTC 2058 +date: bad date '1438053157'; Sat February 38 05:31:00 UTC 2057 != Sun Mar 10 05:31:00 UTC 2058 FAIL: date Feb 29th echo -ne '' | TZ=UTC date 02291975 2>&1 --- expected2016-03-19 00:40:21.056862385 -0700 +++ actual 2016-03-19 00:40:21.060862385 -0700 @@ -1 +1 @@ -date: bad date '02291975'; Wed Feb 29 00:00:00 UTC 1975 != Sat Mar 1 00:00:00 UTC 1975 +date: bad date '02291975'; Sat Feb 29 00:00:00 UTC 1975 != Sat Mar 1 00:00:00 UTC 1975 make: *** [test_date] Error 1 Andy ___ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
[Toybox] [PATCH] Implement mv -n / cp -n (no clobber).
This fixes a failing test case in mv.test. Test changes: - Add coverage for -i (interactive). - Better descriptions, better formatting, and removed some redundant cases. From 27a5d5bcc4cd76b188f5797f29b4d8620ee2050e Mon Sep 17 00:00:00 2001 From: Andy Chu Date: Sat, 19 Mar 2016 00:34:42 -0700 Subject: [PATCH] Implement mv -n / cp -n (no clobber). This fixes a failing test case in mv.test. Test changes: - Add coverage for -i (interactive). - Better descriptions, better formatting, and removed some redundant cases. --- tests/mv.test | 133 ++-- toys/posix/cp.c | 17 2 files changed, 99 insertions(+), 51 deletions(-) diff --git a/tests/mv.test b/tests/mv.test index ab2ca5e..b699d01 100755 --- a/tests/mv.test +++ b/tests/mv.test @@ -8,98 +8,145 @@ #testing "name" "command" "result" "infile" "stdin" touch file -testing "old_file to new_file" "mv file file1 && [ ! -e file -a -f file1 ] && - echo 'yes'" "yes\n" "" "" +testing "file to file" \ + "mv file file1 && [ ! -e file -a -f file1 ] && echo yes" \ + "yes\n" "" "" rm -f file* touch file mkdir dir -testing "file to a dir" "mv file dir && [ ! -e file -a -f dir/file ] && - echo 'yes'" "yes\n" "" "" +testing "file to dir" \ + "mv file dir && [ ! -e file -a -f dir/file ] && echo yes" \ + "yes\n" "" "" rm -rf file* dir* mkdir dir -testing "old_dir to new_dir" "mv dir dir1 && [ ! -e dir -a -d dir1 ] && - echo 'yes'" "yes\n" "" "" +testing "dir to dir" \ + "mv dir dir1 && [ ! -e dir -a -d dir1 ] && echo yes" \ + "yes\n" "" "" rm -rf dir* mkdir dir1 dir2 touch file1 file2 dir1/file3 ln -s file1 link1 -testing "multiple files/dir to a dir" "mv file1 file2 link1 dir1 dir2 && - [ ! -e file1 -a ! -e file2 -a ! -e link1 -a ! -e dir1 ] && - [ -f dir2/file1 -a -f dir2/file2 -a -L dir2/link1 -a -d dir2/dir1 ] && - [ -f dir2/dir1/file3 ] && readlink dir2/link1" "file1\n" "" "" +testing "multiple files/dirs to a dir" \ + "mv file1 file2 link1 dir1 dir2 && + [ ! -e file1 -a ! -e file2 -a ! -e link1 -a ! -e dir1 ] && + [ -f dir2/file1 -a -f dir2/file2 -a -L dir2/link1 -a -d dir2/dir1 ] && + [ -f dir2/dir1/file3 ] && readlink dir2/link1" \ + "file1\n" "" "" rm -rf file* link* dir* -touch file1 -testing "a empty file to new_file" "mv file1 file2 && - [ ! -e file1 -a -f file2 ] && stat -c %s file2" "0\n" "" "" -rm -rf file* - -mkdir dir1 -testing "enpty dir to new_dir" "mv dir1 dir2 && - [ ! -d dir1 -a -d dir2 ] && echo 'yes'" "yes\n" "" "" -rm -rf dir* - dd if=/dev/zero of=file1 seek=10k count=1 >/dev/null 2>&1 -testing "file new_file (random file)" "mv file1 file2 && - [ ! -e file1 -a -f file2 ] && stat -c %s file2" "5243392\n" "" "" +testing "random file to new file" \ + "mv file1 file2 && [ ! -e file1 -a -f file2 ] && stat -c %s file2" \ + "5243392\n" "" "" rm -f file* touch file1 ln -s file1 link1 -testing "link new_link (softlink)" "mv link1 link2 && - [ ! -e link1 -a -L link2 ] && readlink link2" "file1\n" "" "" +testing "symlink to new symlink" \ + "mv link1 link2 && [ ! -e link1 -a -L link2 ] && readlink link2" \ + "file1\n" "" "" unlink tLink2 &>/dev/null rm -f file* link* touch file1 ln file1 link1 -testing "link new_link (hardlink)" "mv link1 link2 && - [ ! -e link1 -a -f link2 -a file1 -ef link2 ] && echo 'yes'" "yes\n" "" "" +testing "hard link to new hardlink" \ + "mv link1 link2 && [ ! -e link1 -a -f link2 -a file1 -ef link2 ] && echo yes" \ + "yes\n" "" "" unlink link2 &>/dev/null rm -f file* link* touch file1 chmod a-r file1 -testing "file new_file (unreadable)" "mv file1 file2 && - [ ! -e file1 -a -f file2 ] && echo 'yes'" "yes\n" "" "" +testing "file to unreadable file" \ + "mv file1 file2 && [ ! -e file1 -a -f file2 ] && echo yes" \ + "yes\n" "" "" rm -f file* touch file1 ln file1 link1 mkdir dir1 -testing "file link dir (hardlink)" "mv file1 link1 dir1 && - [ ! -e file1 -a ! -e link1 -a -f dir1/file1 -a -f dir1/link1 ] && - [ dir1/file1 -ef dir1/link1 ] && echo 'yes'" "yes\n" "" "" +testing "file hardlink dir" \ + "mv file1 link1 dir1 && + [ ! -e file1 -a ! -e link1 -a -f dir1/file1 -a -f dir1/link1 ] && + [ dir1/file1 -ef dir1/link1 ] && echo yes" \ + "yes\n" "" "" rm -rf file* link* dir* mkdir -p dir1/dir2 dir3 touch dir1/dir2/file1 dir1/dir2/file2 -testing "dir1/dir2 dir3/new_dir" "mv dir1/dir2 dir3/dir4 && - [ ! -e dir1/dir2 -a -d dir3/dir4 -a -f dir3/dir4/file1 ] && - [ -f dir3/dir4/file2 ] && echo 'yes'" "yes\n" "" "" +testing "dir to new dir" \ + "mv dir1/dir2 dir3/new && + [ ! -e dir1/dir2 -a -d dir3/new -a -f dir3/new/file1 ] && + [ -f dir3/new/file2 ] && echo yes" \ + "yes\n" "" "" rm -rf file* dir* mkdir dir1 dir2 -testing "dir new_dir (already exist)" "mv dir1 dir2 && - [ ! -e dir1 -a -d dir2/dir1 ] && echo 'yes'" "yes\n" "" "" +testing "dir to existing dir" \ + "mv dir1 dir2 && [ ! -e dir1 -a -d dir2/dir1 ] && echo yes" \ + "yes\n" "" "" rm -rf dir* touch
[Toybox] [PATCH] Fix "tail -f single_file".
TT.file_no was being incorrectly calculated as 0 when tail -f was passed a
single argument, so tail -f with one argument wouldn't actually do the '-f'
part.
Patch attached.
From 3feb4ae50b5135e7c9c8870968f87e318c615ba2 Mon Sep 17 00:00:00 2001
From: Josh Gao
Date: Wed, 16 Mar 2016 15:41:13 -0700
Subject: [PATCH] Fix "tail -f single_file".
TT.file_no was being incorrectly calculated as 0 when tail -f was passed
a single argument.
---
toys/posix/tail.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/toys/posix/tail.c b/toys/posix/tail.c
index d1c6250..bf25a14 100644
--- a/toys/posix/tail.c
+++ b/toys/posix/tail.c
@@ -136,7 +136,7 @@ static void do_tail(int fd, char *name)
int linepop = 1;
if (toys.optflags & FLAG_f) {
-int f = TT.file_no*2;
+int f = (TT.file_no++)*2;
char *s = name;
if (!fd) sprintf(s = toybuf, "/proc/self/fd/%d", fd);
@@ -146,7 +146,7 @@ static void do_tail(int fd, char *name)
}
if (toys.optc > 1) {
-if (TT.file_no++) xputc('\n');
+if (TT.file_no) xputc('\n');
xprintf("==> %s <==\n", name);
}
--
2.7.0.rc3.207.g0ac5344
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
