On Tue, Dec 30, 2014 at 10:49:34AM +0100, Andreas Henriksson wrote:
> Hello Adam Majer!

Hello!

> Stumbled across your bug report while browsing over release-critical ones...
> 
> I haven't even looked at the lpe source, but just from looking at the
> hunk included as context in your patch it looks like the source
> could really use some wider review then just targeted fixes.

That's quite correct. lpe hasn't really been updated in a decade. But
it's still a very simple editor I prefer over vim or emacs for simple
tasks.

 - Ctrl-C close without saving
 - Ctrl-X close and save
 - Ctrl-V Ctrl-V/A/S - pass buffer through shell command, awk, or sed

Can't get faster than that.


> On Tue, Dec 23, 2014 at 09:34:20AM -0600, Adam Majer wrote:
> [...]
> > diff -u lpe-1.2.7/src/buffer.c lpe-1.2.7/src/buffer.c
> > --- lpe-1.2.7/src/buffer.c  2014-06-23 22:53:33.582593198 -0500
> > +++ lpe-1.2.7/src/buffer.c  2014-12-23 09:08:54.888625050 -0600
> > @@ -158,8 +158,8 @@
> >             int (*accept) (buffer *);
> >  
> 
> Consider the case where strlen(ent->d_name) == basename_len.
> 
> >                  if (strlen(ent->d_name) > basename_len) {
> This should probably use >= .            ^^^^

No. This already includes null when accounting for basename_len. If
strlen == basename_len then there is enough room in the buffer. Output
is just before the \0 that is already allocated.

That is also why that +1 to account for \0 in the calculation was in
the wrong spot and caused issues on next loop when filename could be
+1 character longer. And those happen,

cppmode.so
htmlmode.so


> As mentioned, I haven't looked at the full source so I might very well
> be missing something.  As I understood it this is the second attempt at
> fixing an issue here.  Possibly a wider review of how to avoid
> off-by-one in the entire source could be useful.

In this case this is the problem. I think it was caused by trying to
be "fancy" and retaining old code that used fixed length array and
then apply it to dynamic array. Better approach would have been to
simplify all those concatenations in one spot instead of all over the
loop.

You are 100% correct. lpe would benefit from overall update.

Thanks for looking at this.

- Adam

-- 
Adam Majer
ad...@zombino.com


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to