On Fri, 05 Aug 2011 11:03:55 +0900
Hyoyoung Chang <[email protected]> wrote:

> I missed account char len. 
> Improved patch is attached.
> 
> Thank you.
> 
> > -----Original Message-----
> > From: Mike Blumenkrantz [mailto:[email protected]]
> > Sent: Friday, August 05, 2011 10:26 AM
> > To: Enlightenment developer list
> > Cc: [email protected]
> > Subject: Re: [E-devel] [patch] elm_cnp_helper - adding paragraph
> > separator(<ps>) handling
> > 
> > On Fri, 05 Aug 2011 10:11:39 +0900
> > Hyoyoung Chang <[email protected]> wrote:
> > 
> > > Dear developers.
> > >
> > >
> > >
> > > This patch is about elm_cnp_helper.
> > >
> > > When using elm_entry, sometimes newline isn't copied.
> > >
> > > That's because <ps> tag. It's represent of 'paragraph separator'.
> > >
> > > (Other case is using <br>. It works well)
> > >
> > > I just adding <ps> handling for separated paragraph.
> > >
> > >
> > >
> > > After url entry bug (isn't copied in some browsers.), I'll clean up
> > internal
> > > char handling.
> > >
> > >
> > >
> > > Thank you.
> > >
> > >
> > >
> > I can't commit your patch because it allows for a buffer overflow.
> > 
> > elm_cnp_helper.c:1038 has
> > 
> >    q = malloc(strlen(p) + 1);
> > 
> > which allocates exactly the length of the string in the new buffer
> > (because
> > previously the escaping code was a 1:1 substitution), but in your patch
> > you are
> > copying more than 1 byte into this new buffer. You will need to account
> > for
> > this somehow.
> > 
> > Additionally, don't use strlen on a statically allocated string such as
> > _PARAGRAPH_SEPARATOR. Instead, you can use (sizeof(_PARAGRAPH_SEPARATOR) -
> > 1),
> > which has the benefit of not requiring optimization to determine instantly
> > the
> > length of _PARAGRAPH_SEPARATOR since it is done automatically during
> > compile
> > time.
> > 
> > --
> > Mike Blumenkrantz
> > Zentific: Coding in binary since '10.
mm as I said in IRC, you will almost definitely be better off just doing a block
allocation of ((sizeof(LONGEST_ESCAPE) - 1) * LENGTH) + 1) at the start instead
of your strstr() block. While this will waste some memory (lots of memory on
large selections, unfortunately), you have the guarantee of avoiding a buffer
overflow and the benefit of only doing one malloc() call instead of a strstr()
+ malloc().
To make this more futureproof, you could also add something like

#define LONGEST_ESCAPE _PARAGRAPH_SEPARATOR

which could then be easily changed later. Otherwise, with your method you will
run into cases where you have multiple strings that you need to call strstr to
find, which is bad for obvious reasons.

-- 
Mike Blumenkrantz
Zentific: Coding in binary since '10.

------------------------------------------------------------------------------
BlackBerry&reg; DevCon Americas, Oct. 18-20, San Francisco, CA
The must-attend event for mobile developers. Connect with experts. 
Get tools for creating Super Apps. See the latest technologies.
Sessions, hands-on labs, demos & much more. Register early & save!
http://p.sf.net/sfu/rim-blackberry-1
_______________________________________________
enlightenment-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to