On Sun, 9 Oct 2016 21:40:55 +0000 Thomas Mannay <[email protected]> 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 <[email protected]>
