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.
Index: src/lib/elm_cnp_helper.c
===================================================================
--- src/lib/elm_cnp_helper.c (리ë¹ì 62079)
+++ src/lib/elm_cnp_helper.c (ìì
ì¬ë³¸)
@@ -78,7 +78,7 @@
struct _Escape
{
const char *escape;
- const char value;
+ const char *value;
};
struct _Tmp_Info
@@ -147,16 +147,19 @@
static Eina_Bool pasteimage_append(char *file, Evas_Object *entry);
+#define _PARAGRAPH_SEPARATOR "\xE2\x80\xA9"
+
/* Optimisation: Turn this into a 256 byte table:
* then can lookup in one index, not N checks */
static const Escape escapes[] = {
- { "<br>", '\n' },
- { "<\t>", '\t' },
- { "gt;", '>' },
- { "lt;", '<' },
- { "amp;", '&' },
- { "quot;", '\'' },
- { "dquot;", '"' }
+ { "<ps>", _PARAGRAPH_SEPARATOR },
+ { "<br>", "\n" },
+ { "<\t>", "\t" },
+ { "gt;", ">" },
+ { "lt;", "<" },
+ { "amp;", "&" },
+ { "quot;", "\'" },
+ { "dquot;", "\"" }
};
#define N_ESCAPES ((int)(sizeof(escapes) / sizeof(escapes[0])))
@@ -1035,7 +1038,7 @@
int i;
if (!p) return NULL;
- q = malloc(strlen(p) + 1);
+ q = malloc(strlen(p)+1);
if (!q) return NULL;
ret = q;
@@ -1047,6 +1050,11 @@
if ((p[1] == 'b') && (p[2] == 'r') &&
((p[3] == ' ') || (p[3] == '/') || (p[3] == '>')))
*q++ = '\n';
+ else if ((p[1] == 'p') && (p[2] == 's') && (p[3] == '>'))
+ {
+ strcpy(q, _PARAGRAPH_SEPARATOR);
+ q += (sizeof(_PARAGRAPH_SEPARATOR)-1);
+ }
while ((*p) && (*p != '>')) p++;
p++;
}
@@ -1058,8 +1066,8 @@
if (!strncmp(p,escapes[i].escape, strlen(escapes[i].escape)))
{
p += strlen(escapes[i].escape);
- *q = escapes[i].value;
- q++;
+ strcpy(q, escapes[i].value);
+ q += strlen(escapes[i].value);
break;
}
}
@@ -1087,9 +1095,10 @@
{
for (i = 0 ; i < N_ESCAPES ; i ++)
{
- if (*p == escapes[i].value)
+ if (*p == escapes[i].value[0])
{
- l += strlen(escapes[i].escape);
+ if (!strncmp(p, escapes[i].value, strlen(escapes[i].value)))
+ l += strlen(escapes[i].escape);
break;
}
}
@@ -1103,11 +1112,14 @@
{
for (i = 0; i < N_ESCAPES; i++)
{
- if (*p == escapes[i].value)
+ if (*p == escapes[i].value[0])
{
- strcpy(q, escapes[i].escape);
- q += strlen(escapes[i].escape);
- p ++;
+ if (!strncmp(p, escapes[i].value, strlen(escapes[i].value)))
+ {
+ strcpy(q, escapes[i].escape);
+ q += strlen(escapes[i].escape);
+ p += strlen(escapes[i].value);
+ }
break;
}
}
------------------------------------------------------------------------------
BlackBerry® 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
