Re: [hackers] [sbase] [patch] ed: standards compliance, manpage, double free and infinite loop fix

[Thomas Mannay]

It took a while longer than I'd planned, but I've gone through and redone my
patches. I found a second source of an infinite loop to do with joining
where a variable would be -1 and the counter started at 0, so could never
equal it. I also added a safety guard on j to prevent someone passing it a
0 address, which would cause a segfault.

Hopefully all's in order and my editor saved with tabs this time.

Thanks,
Thomas

--
Thomas Mannay 
>From e27ecb4eaf04fcec871c4eb5319f1c14bc151d3d Mon Sep 17 00:00:00 2001
From: Thomas Mannay 
Date: Sun, 9 Oct 2016 23:10:20 +
Subject: [PATCH 1/5] ed: remove infinite loops in join() and getindex()

---
 ed.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/ed.c b/ed.c
index 184ed30..f552234 100644
--- a/ed.c
+++ b/ed.c
@@ -192,7 +192,9 @@ getindex(int line)
 	struct hline *lp;
 	int n;
 
-	for (n = 0, lp = zero; n != line; ++n)
+	if (line == -1)
+		line = 0;
+	for (n = 0, lp = zero; n != line; n++)
 		lp = zero + lp->next;
 
 	return lp - zero;
@@ -806,9 +808,11 @@ join(void)
 	static char *s;
 
 	free(s);
-	for (s = NULL, i = line1; i <= line2; i = nextln(i)) {
+	for (s = NULL, i = line1;; i = nextln(i)) {
 		for (t = gettxt(i); (c = *t) != '\n'; ++t)
 			s = addchar(*t, s, , );
+		if (i == line2)
+			break;
 	}
 
 	s = addchar('\n', s, , );
-- 
2.10.0

>From 5069287a04fb182e83269b86b95f01169ae36233 Mon Sep 17 00:00:00 2001
From: Thomas Mannay 
Date: Sun, 9 Oct 2016 23:11:01 +
Subject: [PATCH 2/5] ed: giving j only one address does nothing

---
 ed.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/ed.c b/ed.c
index f552234..4ab40bc 100644
--- a/ed.c
+++ b/ed.c
@@ -1174,9 +1174,8 @@ repeat:
 	case 'j':
 		chkprint(1);
 		deflines(curln, curln+1);
-		if (!line1)
-			goto bad_address;
-		join();
+		if (line1 != line2 && curln != 0)
+	  		join();
 		break;
 	case 'z':
 		if (nlines > 1)
-- 
2.10.0

>From 90f8b32b4dac7553882fce39094a338a399a1d44 Mon Sep 17 00:00:00 2001
From: Thomas Mannay 
Date: Sun, 9 Oct 2016 23:12:46 +
Subject: [PATCH 3/5] ed: place newly joined lines correctly

---
 ed.c | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/ed.c b/ed.c
index 4ab40bc..d123814 100644
--- a/ed.c
+++ b/ed.c
@@ -299,13 +299,17 @@ undo(void)
 }
 
 static void
-inject(char *s)
+inject(char *s, int j)
 {
 	int off, k, begin, end;
 
-	begin = getindex(curln);
-	end = getindex(nextln(curln));
-
+	if (j) {
+		begin = getindex(curln-1);
+		end = getindex(nextln(curln-1));
+	} else {
+		begin = getindex(curln);
+		end = getindex(nextln(curln));
+	}
 	while (*s) {
 		k = makeline(s, );
 		s += off;
@@ -636,7 +640,7 @@ doread(char *fname)
 			s[n-1] = '\n';
 			s[n] = '\0';
 		}
-		inject(s);
+		inject(s, 0);
 	}
 	if (optdiag)
 		printf("%zu\n", cnt);
@@ -753,7 +757,7 @@ append(int num)
 	while (getline(, , stdin) > 0) {
 		if (*s == '.' && s[1] == '\n')
 			break;
-		inject(s);
+		inject(s, 0);
 	}
 	free(s);
 }
@@ -818,7 +822,7 @@ join(void)
 	s = addchar('\n', s, , );
 	s = addchar('\0', s, , );
 	delete(line1, line2);
-	inject(s);
+	inject(s, 1);
 	free(s);
 }
 
@@ -845,7 +849,7 @@ copy(int where)
 	curln = where;
 
 	for (i = line1; i <= line2; ++i)
-		inject(gettxt(i));
+		inject(gettxt(i), 0);
 }
 
 static void
@@ -1025,7 +1029,7 @@ subline(int num, int nth)
 	addpost(, , );
 	delete(num, num);
 	curln = prevln(num);
-	inject(s);
+	inject(s, 0);
 }
 
 static void
-- 
2.10.0

>From 977008c84e74f0c0fc1c4655ccd0b35920f0b5a3 Mon Sep 17 00:00:00 2001
From: Thomas Mannay 
Date: Sun, 9 Oct 2016 23:13:13 +
Subject: [PATCH 4/5] ed: remove double free in join()

---
 ed.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/ed.c b/ed.c
index d123814..de5321f 100644
--- a/ed.c
+++ b/ed.c
@@ -809,9 +809,8 @@ join(void)
 	int i;
 	char *t, c;
 	size_t len = 0, cap = 0;
-	static char *s;
+	char *s;
 
-	free(s);
 	for (s = NULL, i = line1;; i = nextln(i)) {
 		for (t = gettxt(i); (c = *t) != '\n'; ++t)
 			s = addchar(*t, s, , );
-- 
2.10.0

>From 7740d822cd5d52fd0abd5c8350987345c610309a Mon Sep 17 00:00:00 2001
From: Thomas Mannay 
Date: Sun, 9 Oct 2016 23:14:54 +
Subject: [PATCH 5/5] ed: add manpage

---
 ed.1 | 210 ++-
 1 file changed, 209 insertions(+), 1 deletion(-)

diff --git a/ed.1 b/ed.1
index 93e3012..50351e2 100644
--- a/ed.1
+++ b/ed.1
@@ -6,4 +6,212 @@
 .Nd text editor
 .Sh SYNOPSIS
 .Nm
-is the standard text editor.
+.Op Fl s
+.Op Fl p Ar string
+.Op Ar file
+.Sh DESCRIPTION
+.Nm
+is the standard text editor. It performs line-oriented operations on a buffer;
+The buffer's contents are manipulated in command mode and text is written to the
+buffer in input mode. Command mode is the default. To exit input mode 

Re: [hackers] [slstatus] announcement

[Laslo Hunhold]

On Mon, 10 Oct 2016 00:09:39 +0200
Aaron Marcher  wrote:

Hey Aaron,

> Now to the actual content: I always wanted some statusbar in dwm. I
> started with some shell script as pointed out in the wiki but it grew
> and used a lot of resources. So I started to hack something in C.
> That project, which was initially forked by "dwmstatus" and only
> intended to be used by me grew and became something bigger. I started
> to write it, so that it can be easily used by other people. Multiple
> entries per function (e.g. multiple batteries) are supported and
> everything can be reordered and customized via a C header file
> (similar to other suckless programs).

this looks really cool! I think it is something that we've all been
looking for in some way. The dwm wiki has lots of code snippets on the
topic and a thin library that provides said functionality, but such a
tool really looks like a good general purpose solution.

> Currently I am working on porting it to BSD as it is only made for
> Linux at the moment.

I'm looking forward to see how the porting efforts work out.

Cheers

Laslo

-- 
Laslo Hunhold 

[hackers] [slstatus] announcement

[Aaron Marcher]

Fellow hackers,

my name is Aaron Marcher but I am better known as drkhsh. I started using 
suckless software about two years ago. Since then I have been using dwm, st and 
all the other great programs. About a year ago I joined IRC. I never posted to 
the mailing list so thats the reason for this short introduction about myself.


Now to the actual content: I always wanted some statusbar in dwm. I started with 
some shell script as pointed out in the wiki but it grew and used a lot of 
resources. So I started to hack something in C. That project, which was 
initially forked by "dwmstatus" and only intended to be used by me grew and 
became something bigger. I started to write it, so that it can be easily used by 
other people. Multiple entries per function (e.g. multiple batteries) are

supported and everything can be reordered and customized via a C header file
(similar to other suckless programs).

The following information is included:
- Battery percentage/state
- CPU usage (in percent)
- Custom shell commands
- Date and time
- Disk[s] status (free storage, percentage, total storage and used storage)
- Available entropy
- Username/GID/UID
- Hostname
- IP addresses
- Load averages
- Memory status (free memory, percentage, total memory and used memory)
- Temperature
- Uptime
- Volume percentage (ALSA)
- WiFi signal percentage and ESSID

Currently I am working on porting it to BSD as it is only made for Linux at the 
moment.


Repo: https://git.nulltime.net/slstatus

Regards,
Aaron Marcher

--
web: https://www.nulltime.net/ or http://nulltime5w3shrrc.onion/
gpg: 0x435BF54B

Re: [hackers] [sbase] [patch] ed: standards compliance, manpage, double free and infinite loop fix

[Laslo Hunhold]

On Sun, 9 Oct 2016 21:40:55 +
Thomas Mannay  wrote:

Hey Thomas,

> > - the code within join() is not tab- but space-indented
> And here I thought I had set my editor to indent tabs...

this can happen, no problem.

> > - can getindex(curln-1) underflow? (if curln = 0)
> iirc currln can't be 0 since addresses start at 1 in ed. I've
> experienced no issues with joining a set of addresses beginning with
> the first line.

Alright, that sounds like a reasonable point. But what if curln is 1,
then the line index passed to getindex is "0". Is that a valid line
index?

> > - what's the purpose of the free(s)?
> The original code had two frees, which I assume was due to a static
> char pointer. This would cause a double free if you did two join
> commands in a row. The free at the end of the function is required
> because addchar returns a manually allocated char pointer.

Thanks for elaborating! I somehow forgot to notice the "static" for s.

> > - can you give an example in the patch-description where this
> >   infinite loop occurs?
> I'm unsure where the patch description would be, but it occurs when
> you issue 1,2j when there are only two lines in the buffer. Doing
> that on the unpatched ed causes it to stop accepting commands until
> given an interrupt due to an infinite loop.

Alright, so the patch description is the "commit description" so to
say. It would be nice to have a minimal working example so people know
what the patch fixes and test it quickly and easily.

> > - Do we really need an "EXTENDED DESCRIPTION"
> Yes, it's one of the standard manpage subheadings and allowed me to
> properly document ed. It felt like The Right Thing since correct
> documentation is as important as correct code. It's actually how I
> found the bugs I fixed. :^)

Fair point!

> > - Please check the other manpages for the standard format of
> >   the STANDARDS section
> Have amended this with the attached patch.

Alright, thanks!
 
> Should I redo those patches to account for changing commit messages
> to more active forms?

I would welcome this!

Cheers

Laslo

-- 
Laslo Hunhold 

Re: [hackers] [sbase] [patch] ed: standards compliance, manpage, double free and infinite loop fix

[Thomas Mannay]

I only went and forgot to attach the patch to ed.1...

---
 ed.1 | 5 +
 1 file changed, 5 insertions(+)

diff --git a/ed.1 b/ed.1
index c9f8a68..50351e2 100644
--- a/ed.1
+++ b/ed.1
@@ -205,6 +205,11 @@ characters. When
 returns a '!' is printed. The dot is unchanged.
 .El
 .Sh STANDARDS
+The
+.Nm
+utility is compliant with the
+.St -p1003.1-2013
+specification, except where noted here:
 g and v operate on single commands rather than lists delimited with '\\'.
 e, E, r, w, and W commands cannot accept shell escapes.
 .Sh SEE ALSO
-- 
2.10.0

--
Thomas Mannay 

Re: [hackers] [sbase] [patch] ed: standards compliance, manpage, double free and infinite loop fix

[Thomas Mannay]

Thank you for the feedback.

> - change the name to "ed: place newly-joined lines correctly"
Will bear in mind for future patches that I should use more active descriptions.
> - the code within join() is not tab- but space-indented
And here I thought I had set my editor to indent tabs...
> - can getindex(curln-1) underflow? (if curln = 0)
iirc currln can't be 0 since addresses start at 1 in ed. I've experienced no
issues with joining a set of addresses beginning with the first line.
> - what's the purpose of the free(s)?
The original code had two frees, which I assume was due to a static char 
pointer.
This would cause a double free if you did two join commands in a row. The free 
at
the end of the function is required because addchar returns a manually allocated
char pointer.
> - can you give an example in the patch-description where this
>   infinite loop occurs?
I'm unsure where the patch description would be, but it occurs when you issue
1,2j when there are only two lines in the buffer. Doing that on the unpatched ed
causes it to stop accepting commands until given an interrupt due to an infinite
loop.
> - Do we really need an "EXTENDED DESCRIPTION"
Yes, it's one of the standard manpage subheadings and allowed me to properly
document ed. It felt like The Right Thing since correct documentation is as
important as correct code. It's actually how I found the bugs I fixed. :^)
> - Please check the other manpages for the standard format of
>   the STANDARDS section
Have amended this with the attached patch.

Should I redo those patches to account for changing commit messages to more
active forms?

Thank you,
Thomas

--
Thomas Mannay 

Re: [hackers] [sbase] [patch] ed: standards compliance, manpage, double free and infinite loop fix

[Laslo Hunhold]

On Thu, 6 Oct 2016 11:04:17 +
Thomas Mannay  wrote:

Hey Thomas,

> Resubmission of my patchset for ed, albeit much better formatted so
> as to ease reading them.

as promised, I will give you some feedback on your patches you sent in.

### 0001-ed-newly-joined-lines-are-placed-correctly.patch

- change the name to "ed: place newly-joined lines correctly"
- the code within join() is not tab- but space-indented
- can getindex(curln-1) underflow? (if curln = 0)
- what's the purpose of the free(s)?

### 0002-ed-if-only-one-address-is-given-to-j-do-nothing.patch

### 0003-ed-fix-double-free-and-infinite-loop-in-join.patch

- can you give an example in the patch-description where this
  infinite loop occurs?

### 0004-ed-wrote-manpage.patch

- change the name to "ed: add a manpage"
- Do we really need an "EXTENDED DESCRIPTION"
- Please check the other manpages for the standard format of
  the STANDARDS section

Thanks for your submissions!

Cheers

Laslo

-- 
Laslo Hunhold 

[hackers] [sent][PATCH] README: add Xft to dependencies

[ssd]

From: ssd 

---
 README.md | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/README.md b/README.md
index 3583d33..c1e9385 100644
--- a/README.md
+++ b/README.md
@@ -11,8 +11,8 @@ worry about alignment. Instead you can really concentrate on 
the content.
 
 Dependencies
 
-You need Xlib to build sent and the farbfeld[0] tools installed to use images 
in
-your presentations.
+You need Xlib and Xft to build sent and the farbfeld[0] tools installed to use
+images in your presentations.
 
 Demo
 
@@ -38,6 +38,8 @@ with `#` will be ignored. A `\` at the beginning of the line 
escapes `@` and

depends on
- Xlib
+   - Xft
+   - farbfeld

sent FILENAME
one slide per paragraph
-- 
2.10.0

Re: [hackers] [sbase] [patch] cp: add -i flag

[Thomas Mannay]

Ah, I had forgotten to add fs.h to the commit. Added it in this time.

I fixed the behaviour of newline as you pointed out. Should all be in order.

--
Thomas Mannay 
>From c809e44cc83eb2b494b9c4e052506e47785f11e3 Mon Sep 17 00:00:00 2001
From: Thomas Mannay 
Date: Sun, 9 Oct 2016 13:05:53 +
Subject: [PATCH] cp: add -i flag

---
 cp.1 | 22 --
 cp.c |  7 ++-
 fs.h |  1 +
 libutil/cp.c | 23 +++
 4 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/cp.1 b/cp.1
index 54126e2..4bbe4d0 100644
--- a/cp.1
+++ b/cp.1
@@ -6,7 +6,7 @@
 .Nd copy files and directories
 .Sh SYNOPSIS
 .Nm
-.Op Fl afpv
+.Op Fl afipv
 .Oo
 .Fl R
 .Op Fl H | L | P
@@ -35,7 +35,15 @@ and
 .It Fl f
 If an existing
 .Ar dest
-cannot be opened, remove it and try again.
+cannot be opened, remove it and try again. Ignores
+.Fl i
+if this option is used after it
+.It Fl i
+If
+.Ar dest
+exists and is not a directory, prompt before overwriting it. Ignores
+.Fl f
+if this option is used after it.
 .It Fl p
 Preserve mode, timestamp and permissions.
 .It Fl v
@@ -62,10 +70,12 @@ The
 .Nm
 utility is compliant with the
 .St -p1003.1-2013
-specification except from the
-.Op Fl i
-flag.
+specification.
 .Pp
 The
 .Op Fl av
-flags are an extension to that specification.
+flags are an extension to that specification, as is the mutual exclusivity of the
+.Fl f
+and
+.Fl i
+flags.
diff --git a/cp.c b/cp.c
index d87e77e..f72de04 100644
--- a/cp.c
+++ b/cp.c
@@ -7,7 +7,7 @@
 static void
 usage(void)
 {
-	eprintf("usage: %s [-afpv] [-R [-H | -L | -P]] source ... dest\n", argv0);
+	eprintf("usage: %s [-afipv] [-R [-H | -L | -P]] source ... dest\n", argv0);
 }
 
 int
@@ -22,6 +22,11 @@ main(int argc, char *argv[])
 		break;
 	case 'f':
 		cp_fflag = 1;
+		cp_iflag = 0;
+		break;
+	case 'i':
+		cp_iflag = 1;
+		cp_fflag = 0;
 		break;
 	case 'p':
 		cp_pflag = 1;
diff --git a/fs.h b/fs.h
index 15ae5f4..36b9cb3 100644
--- a/fs.h
+++ b/fs.h
@@ -25,6 +25,7 @@ enum {
 
 extern int cp_aflag;
 extern int cp_fflag;
+extern int cp_iflag;
 extern int cp_pflag;
 extern int cp_rflag;
 extern int cp_vflag;
diff --git a/libutil/cp.c b/libutil/cp.c
index c398962..116addb 100644
--- a/libutil/cp.c
+++ b/libutil/cp.c
@@ -17,12 +17,22 @@
 
 int cp_aflag  = 0;
 int cp_fflag  = 0;
+int cp_iflag  = 0;
 int cp_pflag  = 0;
 int cp_rflag  = 0;
 int cp_vflag  = 0;
 int cp_status = 0;
 int cp_follow = 'L';
 
+inline static int
+exists(const char *s)
+{
+	struct stat st;
+
+	stat(s, );
+	return S_ISREG(st.st_mode);
+}
+
 int
 cp(const char *s1, const char *s2, int depth)
 {
@@ -34,6 +44,7 @@ cp(const char *s1, const char *s2, int depth)
 	ssize_t r;
 	int (*statf)(const char *, struct stat *);
 	char target[PATH_MAX], ns1[PATH_MAX], ns2[PATH_MAX], *statf_name;
+	int c, y; /* used for i flag */
 
 	if (cp_follow == 'P' || (cp_follow == 'H' && depth)) {
 		statf_name = "lstat";
@@ -112,6 +123,18 @@ cp(const char *s1, const char *s2, int depth)
 			return 0;
 		}
 	} else {
+		if (cp_iflag && exists(s2)) {
+			weprintf("overwrite %s with %s? ", s2, s1);
+			y = 0;
+			while ((c = getchar()) != '\n' && c != EOF) {
+if (c == 'y' || c == 'Y'){
+	y = 1;
+	break;
+}
+			}
+			if (!y)
+return 0;
+		}
 		if (!(f1 = fopen(s1, "r"))) {
 			weprintf("fopen %s:", s1);
 			cp_status = 1;
-- 
2.10.0

Re: [hackers] [sbase] [patch] cp: add -i flag

[Hiltjo Posthuma]

Two things I noticed:

It does not seem to compile, it requires cp_iflag in fs.h:

diff --git a/fs.h b/fs.h
index 15ae5f4..36b9cb3 100644
--- a/fs.h
+++ b/fs.h
@@ -25,6 +25,7 @@ enum {
 
 extern int cp_aflag;
 extern int cp_fflag;
+extern int cp_iflag;
 extern int cp_pflag;
 extern int cp_rflag;
 extern int cp_vflag;



Another one: when return is pressed it seems to be processed as affirmative
and overwrite the file. It should only overwrite if 'y' is entered.

-- 
Kind regards,
Hiltjo

[hackers] [sbase] [patch] cp: add -i flag

[Thomas Mannay]

---
 cp.1 | 22 --
 cp.c |  7 ++-
 libutil/cp.c | 18 ++
 3 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/cp.1 b/cp.1
index 54126e2..4bbe4d0 100644
--- a/cp.1
+++ b/cp.1
@@ -6,7 +6,7 @@
 .Nd copy files and directories
 .Sh SYNOPSIS
 .Nm
-.Op Fl afpv
+.Op Fl afipv
 .Oo
 .Fl R
 .Op Fl H | L | P
@@ -35,7 +35,15 @@ and
 .It Fl f
 If an existing
 .Ar dest
-cannot be opened, remove it and try again.
+cannot be opened, remove it and try again. Ignores
+.Fl i
+if this option is used after it
+.It Fl i
+If
+.Ar dest
+exists and is not a directory, prompt before overwriting it. Ignores
+.Fl f
+if this option is used after it.
 .It Fl p
 Preserve mode, timestamp and permissions.
 .It Fl v
@@ -62,10 +70,12 @@ The
 .Nm
 utility is compliant with the
 .St -p1003.1-2013
-specification except from the
-.Op Fl i
-flag.
+specification.
 .Pp
 The
 .Op Fl av
-flags are an extension to that specification.
+flags are an extension to that specification, as is the mutual exclusivity of 
the
+.Fl f
+and
+.Fl i
+flags.
diff --git a/cp.c b/cp.c
index d87e77e..f72de04 100644
--- a/cp.c
+++ b/cp.c
@@ -7,7 +7,7 @@
 static void
 usage(void)
 {
-   eprintf("usage: %s [-afpv] [-R [-H | -L | -P]] source ... dest\n", 
argv0);
+   eprintf("usage: %s [-afipv] [-R [-H | -L | -P]] source ... dest\n", 
argv0);
 }

 int
@@ -22,6 +22,11 @@ main(int argc, char *argv[])
break;
case 'f':
cp_fflag = 1;
+   cp_iflag = 0;
+   break;
+   case 'i':
+   cp_iflag = 1;
+   cp_fflag = 0;
break;
case 'p':
cp_pflag = 1;
diff --git a/libutil/cp.c b/libutil/cp.c
index c398962..2a0cdd1 100644
--- a/libutil/cp.c
+++ b/libutil/cp.c
@@ -17,12 +17,22 @@

 int cp_aflag  = 0;
 int cp_fflag  = 0;
+int cp_iflag  = 0;
 int cp_pflag  = 0;
 int cp_rflag  = 0;
 int cp_vflag  = 0;
 int cp_status = 0;
 int cp_follow = 'L';

+inline static int
+exists(const char *s)
+{
+   struct stat st;
+
+   stat(s, );
+   return S_ISREG(st.st_mode);
+}
+
 int
 cp(const char *s1, const char *s2, int depth)
 {
@@ -34,6 +44,7 @@ cp(const char *s1, const char *s2, int depth)
ssize_t r;
int (*statf)(const char *, struct stat *);
char target[PATH_MAX], ns1[PATH_MAX], ns2[PATH_MAX], *statf_name;
+   int c; /* used for i flag */

if (cp_follow == 'P' || (cp_follow == 'H' && depth)) {
statf_name = "lstat";
@@ -112,6 +123,13 @@ cp(const char *s1, const char *s2, int depth)
return 0;
}
} else {
+   if (cp_iflag && exists(s2)) {
+   weprintf("overwrite %s with %s? ", s2, s1);
+   while ((c = getchar()) != '\n' && c != EOF) {
+   if (c != 'y' && c != 'Y')
+   return 0;
+   }
+   }
if (!(f1 = fopen(s1, "r"))) {
weprintf("fopen %s:", s1);
cp_status = 1;
--
2.10.0