On Fri, 05 Aug 2011 13:09:41 +0900
Hyoyoung Chang <[email protected]> wrote:

> I found what we've been missing.
> "<ps>" is 4 bytes. And PS char is 3byte(in utf-8).
> So we don't need to allocate maximum buffer size.
> Isn't it?
> 
> If it's correct, plz review it.
> Thanks
> 
> 
> > -----Original Message-----
> > From: Mike Blumenkrantz [mailto:[email protected]]
> > Sent: Friday, August 05, 2011 11:17 AM
> > To: Hyoyoung Chang
> > Cc: 'Enlightenment developer list'
> > Subject: Re: [E-devel] [patch] elm_cnp_helper - adding paragraph
> > separator(<ps>) handling
> > 
> > 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.
looks good, r62153

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