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

From: Laslo Hunhold <>
Date: Sun, 9 Oct 2016 23:48:15 +0200

On Sun, 9 Oct 2016 21:40:55 +0000
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

> > - 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!



Laslo Hunhold <>
Received on Sun Oct 09 2016 - 23:48:15 CEST

This archive was generated by hypermail 2.3.0 : Mon Oct 10 2016 - 00:00:34 CEST