Re: [E-devel] e16 : file.c
Vadik Mironov wrote:
þÅÔ×ÅÒÇ 02 óÅÎÔÑÂÒØ 2004 22:41, Kim Woelders ÎÁÐÉÓÁÌ:
1. The allocation on the stack around 16 kilobytes is not a god idea,
because stack was invented for function calls and recursion , but not for
storing data. And this could theoretically lead to a problem in general
case, if system cannot allocate such a big chunk on the stack.
We may be using a bit more stack space than necessary, but I doubt this
causes any problems in E in real life. This construction is used
throughout E's parsing code and recursion is never very deep.
It is also vastly faster and less error prone than allocating memory
dynamically.
I don't agree with the "storing data" issue. The stack is the perfect
place to store (moderate amounts of) data temporarily.
Point taken. But i hope if i continue to write malloced code, it will be not
count as an act of disobeying :-)
That depends on the context.
hopefully nothing, but still this two functions word and fword are not
safe, scanf("%n") is better, but it is not an easy tool for string
parsing. If somebody protected eesh from buffer smashing, then it is
fine, but still i'm suspicious in using word and fword anyway (and of
course i believe there is no suid binaries in disributions :-)).
If word() or fword() are broken they should be fixed. The solution is
not to introduce yet another string parsing function.
Agreed, but getword already there.
No, actually it's just gone :) fword() *is* used. And there should be
only one function doing this type of parsing!
I will look at this functions, hopefully do something.
That would make sense to me.
/Kim
---
This SF.Net email is sponsored by BEA Weblogic Workshop
FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Workshop 8.1 today.
http://ads.osdn.com/?ad_idP47&alloc_id808&op=click
___
enlightenment-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] e16 : file.c
Четверг 02 Сентябрь 2004 22:41, Kim Woelders написал:
> > 1. The allocation on the stack around 16 kilobytes is not a god idea,
> > because stack was invented for function calls and recursion , but not for
> > storing data. And this could theoretically lead to a problem in general
> > case, if system cannot allocate such a big chunk on the stack.
>
> We may be using a bit more stack space than necessary, but I doubt this
> causes any problems in E in real life. This construction is used
> throughout E's parsing code and recursion is never very deep.
> It is also vastly faster and less error prone than allocating memory
> dynamically.
> I don't agree with the "storing data" issue. The stack is the perfect
> place to store (moderate amounts of) data temporarily.
>
Point taken. But i hope if i continue to write malloced code, it will be not
count as an act of disobeying :-)
> > hopefully nothing, but still this two functions word and fword are not
> > safe, scanf("%n") is better, but it is not an easy tool for string
> > parsing. If somebody protected eesh from buffer smashing, then it is
> > fine, but still i'm suspicious in using word and fword anyway (and of
> > course i believe there is no suid binaries in disributions :-)).
>
> If word() or fword() are broken they should be fixed. The solution is
> not to introduce yet another string parsing function.
>
Agreed, but getword already there.
I will look at this functions, hopefully do something.
>
> Finally, in case you haven't been discouraged entirely, one more thing.
> Please do not use C++ style "//" comments. I believe there are still
> compilers out there which don't like that.
>
> /Kim
>
So then i regenerate my patch, here it is:
--- ./etalon/e16/e/src/E.h 2004-08-31 21:24:39.0 +0400
+++ ./hacked/e16/e/src/E.h 2004-09-07 11:26:12.433520592 +0400
@@ -2083,7 +2110,7 @@
char *usershell(int uid);
const char *atword(const char *s, int num);
const char *atchar(const char *s, char c);
-char *getword(char *s, int num);
+char *getword(char *s,int num);
voidword(const char *s, int num, char *wd);
int canread(const char *s);
int canwrite(const char *s);
--- ./etalon/e16/e/src/file.c 2004-07-13 03:33:15.0 +0400
+++ file.c 2004-09-07 11:38:43.320368440 +0400
@@ -510,74 +510,155 @@
EDBUG_RETURN(NULL);
}
-char *
-getword(char *s, int num)
-{
+/**
+ * Search for a [num]-th word in a string s and return it in
+ * allocated buffer.
+ *
+ * Parameters :
+ * s- pointer to a null terminating string.
+ * Punctuation signs treats as whitespaces.
+ * num - number of a token to search.
+ *
+ * Return value :
+ * NULL - if (num < 1) or (num > number of tokens in s)
+ * or if s contained unquoted symbols other than
+ * spaces, digits and symbols.
+ * Otherwise, pointer to null terminated string returned.
+ * WARNING : Caller must free the buffer.
+ *
+ * Example : " {..\" Alice \" wants more \"cookies4 grandma \""
+ * Token 1 = Alice
+ * Token 2 = wants
+ * Token 3 = more
+ * Token 4 = cookies4 grandma
+ */
+char *
+getword(const char *s, int num)
+{
+ int cnt = 1 ;
+
+ ptrdiff_t char_count = 0 ;
+
+ char *start = NULL ;
+ char *word = NULL ;
+ char *tmp_copy = NULL ;
- /* *FIXME**
-* This function is broken but it isn't in use so I'll fix it later
-* (DO NOT USE UNTIL FIXED
-*/
- int cnt, i;
- char *start, *finish, *ss, *w;
- char *wd = NULL;
+ const char quota_sym = '"' ;
+
+ EDBUG(9,"getword") ;
- EDBUG(9, "getword");
if (!s)
- EDBUG_RETURN(NULL);
- if (!wd)
- EDBUG_RETURN(NULL);
+ {
+goto fail ;
+ }
+
+ /*counting from 1, negative index is mistake */
if (num <= 0)
- {
- *wd = 0;
- EDBUG_RETURN(NULL);
- }
- cnt = 0;
- i = 0;
- start = NULL;
- finish = NULL;
- ss = NULL;
- w = wd;
-
- while (s[i])
- {
- if ((cnt == num) && ((s[i] == ' ') || (s[i] == '\t')))
- {
-finish = &s[i];
-break;
- }
- if ((s[i] != ' ') && (s[i] != '\t'))
- {
-if (i == 0)
- {
- cnt++;
- if (cnt == num)
-start = &s[i];
- }
-else if ((s[i - 1] == ' ') || (s[i - 1] == '\t'))
- {
- cnt++;
- if (cnt == num)
-start = &s[i];
- }
- }
- i++;
- }
- if (cnt == num)
- {
- if ((start) && (finish))
- {
-for (ss
Re: [E-devel] e16 : file.c
Vadik Mironov wrote:
óÒÅÄÁ 01 óÅÎÔÑÂÒØ 2004 20:32, Kim Woelders ÎÁÐÉÓÁÌ:
Vadik Mironov wrote:
Good day, everybody !
These two patches replace broken getword function in file.c with the
not broken one (I hope). Also i made small warkaround about one bug,
and i submit another patch later, but i need this function, so it
will be really cool, if somebody merge these patches to the CVS.
Thank you in advance.
The getword() function is used only two places, in a function which
probably never is called. As it is marked broken as well it is very
close to elimination.
If you need the quotation handling you may be able to use fword() or
field(), otherwise maybe word() or simply sscanf().
Also, please use char* (not unsigned) as the simple C string type like
throughout the rest of the code.
/Kim
Then i try to clarify why i wrote this and submit.
There is a bug in IPC_Debug (eesh , then simply "debug"), here is the patch:
--- ./e/etalon/e16/e/src/ipc.c 2004-08-21 01:13:56.0 +0400
+++ ./e/hacked/e16/e/src/ipc.c 2004-09-02 10:03:22.716571976 +0400
@@ -4432,6 +4432,11 @@
charparam[1024];
int l;
const char *p;
+
+ if(NULL==params)
+ {
+ return ;
+ }
p = params;
l = 0;
Thanks, fixed :)
I worked on a fullscreen applications problems under e (i had some troubles
with fullscreened reeaders and ooffice), but this happens not all the time,
and hopefully on the weekend i will be able to track this down.
I discovered this IPC_Debug null pointer and try to use eesh to get desired
info writing some more code to this function. When i looked in other
functions , i saw something like this:
---
static void
IPC_Cursor(const char *params, Client * c)
{
charbuf[FILEPATH_LEN_MAX];
buf[0] = 0;
if (params)
{
charparam1[FILEPATH_LEN_MAX];
charparam2[FILEPATH_LEN_MAX];
charparam3[FILEPATH_LEN_MAX];
param1[0] = 0;
param2[0] = 0;
param3[0] = 0;
word(params, 1, param1);
there is two considerations on that code:
1. The allocation on the stack around 16 kilobytes is not a god idea, because
stack was invented for function calls and recursion , but not for storing
data. And this could theoretically lead to a problem in general case, if
system cannot allocate such a big chunk on the stack.
We may be using a bit more stack space than necessary, but I doubt this
causes any problems in E in real life. This construction is used
throughout E's parsing code and recursion is never very deep.
It is also vastly faster and less error prone than allocating memory
dynamically.
I don't agree with the "storing data" issue. The stack is the perfect
place to store (moderate amounts of) data temporarily.
2. I don't know yet what happens with e, if i run something like this:
#!/bin/sh
eesh cursor "2K of some trash withoun \n"
or something like :
name[0] = "/usr/X11R6/bin/eesh" ;
name[1] = "button";
name[3] = NULL ;
name[2] = malloc(2);
int i = 0 ;
for(i=0;i<2;i++)
{
name[2][i] = 'A';
}
execve(name[0],name,NULL) ;
hopefully nothing, but still this two functions word and fword are not safe,
scanf("%n") is better, but it is not an easy tool for string parsing.
If somebody protected eesh from buffer smashing, then it is fine, but still
i'm suspicious in using word and fword anyway (and of course i believe there
is no suid binaries in disributions :-)).
If word() or fword() are broken they should be fixed. The solution is
not to introduce yet another string parsing function.
The last point, that i would like to mention is unsigned char :
In that function i heavily used standard functions from ctype.h.
ISO/IEC 9899:1999 (C language) page 181.
I'll just say again, in E (and I believe everywhere else too) the simple
C string pointer type is char*. If you want to cast pointers inside some
function I have no objections, but the API must use only (const) char
pointers to strings.
Please configure with --enable-gcc-warnings and make sure there are no
warnings.
BTW, another thing, if i declare the DEBUG macro through compiler option then
i cannot build eesh because there is only external declarations of the
call_level in E.h.
Thanks, fixed :)
Finally, in case you haven't been discouraged entirely, one more thing.
Please do not use C++ style "//" comments. I believe there are still
compilers out there which don't like that.
/Kim
---
This SF.Net email is sponsored by BEA Weblogic Workshop
FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Work
Re: [E-devel] e16 : file.c
Среда 01 Сентябрь 2004 20:32, Kim Woelders написал:
> Vadik Mironov wrote:
> > Good day, everybody !
> >
> > These two patches replace broken getword function in file.c with the
> > not broken one (I hope). Also i made small warkaround about one bug,
> > and i submit another patch later, but i need this function, so it
> > will be really cool, if somebody merge these patches to the CVS.
> > Thank you in advance.
>
> The getword() function is used only two places, in a function which
> probably never is called. As it is marked broken as well it is very
> close to elimination.
> If you need the quotation handling you may be able to use fword() or
> field(), otherwise maybe word() or simply sscanf().
> Also, please use char* (not unsigned) as the simple C string type like
> throughout the rest of the code.
>
> /Kim
Then i try to clarify why i wrote this and submit.
There is a bug in IPC_Debug (eesh , then simply "debug"), here is the patch:
--- ./e/etalon/e16/e/src/ipc.c 2004-08-21 01:13:56.0 +0400
+++ ./e/hacked/e16/e/src/ipc.c 2004-09-02 10:03:22.716571976 +0400
@@ -4432,6 +4432,11 @@
charparam[1024];
int l;
const char *p;
+
+ if(NULL==params)
+ {
+ return ;
+ }
p = params;
l = 0;
I worked on a fullscreen applications problems under e (i had some troubles
with fullscreened reeaders and ooffice), but this happens not all the time,
and hopefully on the weekend i will be able to track this down.
I discovered this IPC_Debug null pointer and try to use eesh to get desired
info writing some more code to this function. When i looked in other
functions , i saw something like this:
---
static void
IPC_Cursor(const char *params, Client * c)
{
charbuf[FILEPATH_LEN_MAX];
buf[0] = 0;
if (params)
{
charparam1[FILEPATH_LEN_MAX];
charparam2[FILEPATH_LEN_MAX];
charparam3[FILEPATH_LEN_MAX];
param1[0] = 0;
param2[0] = 0;
param3[0] = 0;
word(params, 1, param1);
there is two considerations on that code:
1. The allocation on the stack around 16 kilobytes is not a god idea, because
stack was invented for function calls and recursion , but not for storing
data. And this could theoretically lead to a problem in general case, if
system cannot allocate such a big chunk on the stack.
2. I don't know yet what happens with e, if i run something like this:
#!/bin/sh
eesh cursor "2K of some trash withoun \n"
or something like :
name[0] = "/usr/X11R6/bin/eesh" ;
name[1] = "button" ;
name[3] = NULL ;
name[2] = malloc(2);
int i = 0 ;
for(i=0;i<2;i++)
{
name[2][i] = 'A';
}
execve(name[0],name,NULL) ;
hopefully nothing, but still this two functions word and fword are not safe,
scanf("%n") is better, but it is not an easy tool for string parsing.
If somebody protected eesh from buffer smashing, then it is fine, but still
i'm suspicious in using word and fword anyway (and of course i believe there
is no suid binaries in disributions :-)).
The last point, that i would like to mention is unsigned char :
In that function i heavily used standard functions from ctype.h.
ISO/IEC 9899:1999 (C language) page 181.
BTW, another thing, if i declare the DEBUG macro through compiler option then
i cannot build eesh because there is only external declarations of the
call_level in E.h.
--
With best regards,
Vadik Mironov
<[EMAIL PROTECTED]>
---
This SF.Net email is sponsored by BEA Weblogic Workshop
FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Workshop 8.1 today.
http://ads.osdn.com/?ad_idP47&alloc_id808&op=click
___
enlightenment-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] e16 : file.c
Vadik Mironov wrote: Good day, everybody ! These two patches replace broken getword function in file.c with the not broken one (I hope). Also i made small warkaround about one bug, and i submit another patch later, but i need this function, so it will be really cool, if somebody merge these patches to the CVS. Thank you in advance. The getword() function is used only two places, in a function which probably never is called. As it is marked broken as well it is very close to elimination. If you need the quotation handling you may be able to use fword() or field(), otherwise maybe word() or simply sscanf(). Also, please use char* (not unsigned) as the simple C string type like throughout the rest of the code. /Kim --- This SF.Net email is sponsored by BEA Weblogic Workshop FREE Java Enterprise J2EE developer tools! Get your free copy of BEA WebLogic Workshop 8.1 today. http://ads.osdn.com/?ad_id=5047&alloc_id=10808&op=click ___ enlightenment-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] e16 : file.c
Среда 01 Сентябрь 2004 19:01, Michael Jennings написал:
> On Wednesday, 01 September 2004, at 12:14:40 (+0400),
>
> Vadik Mironov wrote:
> > These two patches replace broken getword function in file.c with the
> > not broken one (I hope). Also i made small warkaround about one bug,
> > and i submit another patch later, but i need this function, so it
> > will be really cool, if somebody merge these patches to the CVS.
>
> Well, if someone had really wanted to fix this, they probably would've
> just stolen the code from libast:
>
> http://www.eterm.org/libast/html/strings_8c-source.html#l00340
>
> No offense, but your code is...well, iffy. Your use of goto's and
> labels is completely unnecessary.
>
> Michael
Well, character handling not an easy task and it doesnt matter how the
code looks like. It should be robust, understandable (try to imagine how your
function will be executed under various circumstances), simple and well
documented. I would like to have an simple function in library with clear
docs and visible path of execution.
--
With best regards,
Vadik Mironov
<[EMAIL PROTECTED]>
PS. No defense :-)
Could you please tell me what happened in this function with such a little
program?
int main(int argn,char *argc[])
{
get_word(1, "\"Anne got cr");
return 0;
}
---
This SF.Net email is sponsored by BEA Weblogic Workshop
FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Workshop 8.1 today.
http://ads.osdn.com/?ad_idP47&alloc_id808&op=click
___
enlightenment-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] e16 : file.c
On Wednesday, 01 September 2004, at 12:14:40 (+0400), Vadik Mironov wrote: > These two patches replace broken getword function in file.c with the > not broken one (I hope). Also i made small warkaround about one bug, > and i submit another patch later, but i need this function, so it > will be really cool, if somebody merge these patches to the CVS. Well, if someone had really wanted to fix this, they probably would've just stolen the code from libast: http://www.eterm.org/libast/html/strings_8c-source.html#l00340 No offense, but your code is...well, iffy. Your use of goto's and labels is completely unnecessary. Michael -- Michael Jennings (a.k.a. KainX) http://www.kainx.org/ <[EMAIL PROTECTED]> n + 1, Inc., http://www.nplus1.net/ Author, Eterm (www.eterm.org) --- "I'll be leaving soon; it's hard to say when I'll return, and I don't want to lead you on. So if you feel the need, close your eyes and share this dream. It will be Eternity." -- Blessid Union of Souls, "Forever for Tonight" --- This SF.Net email is sponsored by BEA Weblogic Workshop FREE Java Enterprise J2EE developer tools! Get your free copy of BEA WebLogic Workshop 8.1 today. http://ads.osdn.com/?ad_id=5047&alloc_id=10808&op=click ___ enlightenment-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
[E-devel] e16 : file.c
Good day, everybody !
These two patches replace broken getword function in file.c with the not
broken one (I hope). Also i made small warkaround about one bug, and i submit
another patch later, but i need this function, so it will be really cool, if
somebody merge these patches to the CVS.
Thank you in advance.
--
With best regards,
Vadik Mironov
<[EMAIL PROTECTED]>
--- ./e/etalon/e16/e/src/E.h2004-08-31 21:24:39.0 +0400
+++ ./e/hacked/e16/e/src/E.h2004-09-01 12:00:40.485221968 +0400
@@ -2083,7 +2083,7 @@
char *usershell(int uid);
const char *atword(const char *s, int num);
const char *atchar(const char *s, char c);
-char *getword(char *s, int num);
+unsigned char *getword(unsigned char *s,int num);
voidword(const char *s, int num, char *wd);
int canread(const char *s);
int canwrite(const char *s);
--- ./e/etalon/e16/e/src/file.c 2004-07-13 03:33:15.0 +0400
+++ ./e/hacked/e16/e/src/file.c 2004-09-01 12:03:04.695298704 +0400
@@ -510,74 +510,153 @@
EDBUG_RETURN(NULL);
}
-char *
-getword(char *s, int num)
-{
+/**
+ * Search for a [num]-th word in a string s and return it in
+ * allocated buffer.
+ *
+ * Parameters :
+ * s- pointer to a null terminating string.
+ * Punctuation signs treats as whitespaces.
+ * num - number of a token to search.
+ *
+ * Return value :
+ * NULL - if (num < 1) or (num > number of tokens in s)
+ * or if s contained unquoted symbols other than
+ * spaces, digits and symbols.
+ * Otherwise, pointer to null terminated string returned.
+ * WARNING : Caller must free the buffer.
+ *
+ * Example : " {..\" Alice \" wants more \"cookies4 grandma \""
+ * Token 1 = Alice
+ * Token 2 = wants
+ * Token 3 = more
+ * Token 4 = cookies4 grandma
+ */
+unsigned char *
+getword(unsigned char *s, int num)
+{
+ int cnt = 1 ;
+
+ ptrdiff_t char_count = 0 ;
+
+ unsigned char *start = NULL ;
+ unsigned char *word = NULL ;
+ unsigned char *tmp_copy = NULL ;
- /* *FIXME**
-* This function is broken but it isn't in use so I'll fix it later
-* (DO NOT USE UNTIL FIXED
-*/
- int cnt, i;
- char *start, *finish, *ss, *w;
- char *wd = NULL;
+ const unsigned char quota_sym = '"' ;
- EDBUG(9, "getword");
if (!s)
- EDBUG_RETURN(NULL);
- if (!wd)
- EDBUG_RETURN(NULL);
+ {
+goto fail ;
+ }
+
+ //counting from 1, negative index is mistake
if (num <= 0)
- {
- *wd = 0;
- EDBUG_RETURN(NULL);
- }
- cnt = 0;
- i = 0;
- start = NULL;
- finish = NULL;
- ss = NULL;
- w = wd;
-
- while (s[i])
- {
- if ((cnt == num) && ((s[i] == ' ') || (s[i] == '\t')))
- {
-finish = &s[i];
-break;
- }
- if ((s[i] != ' ') && (s[i] != '\t'))
- {
-if (i == 0)
- {
- cnt++;
- if (cnt == num)
-start = &s[i];
- }
-else if ((s[i - 1] == ' ') || (s[i - 1] == '\t'))
- {
- cnt++;
- if (cnt == num)
-start = &s[i];
- }
- }
- i++;
- }
- if (cnt == num)
- {
- if ((start) && (finish))
- {
-for (ss = start; ss < finish; ss++)
- *wd++ = *ss;
- }
- else if (start)
- {
-for (ss = start; *ss != 0; ss++)
- *wd++ = *ss;
- }
- *wd = 0;
- }
- EDBUG_RETURN(wd);
+ {
+goto fail ;
+ }
+
+ //skip all whitespaces at first
+ //except double quotas since they are meaningful
+ //also let us ignore other garbage
+ while(!isalnum(*s) && quota_sym != *s)
+ {
+ s++ ;
+ }
+
+ //main cycle till the end
+ while ('\0'!=*s)
+ {
+ //we are standing at the start of quotes
+ if (quota_sym == *s)
+ {
+ //step over quota
+ s++ ;
+ //save start in case of word
+ if(cnt == num)
+ {
+ start = s;
+ }
+ //it is possible to write inside quotes any trash
+ //even bells and whistles :-)
+ while (quota_sym != *s)
+ {
+ //only one quote sign
+ if ('\0' == *s)
+ {
