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.000000000 +0400
+++ ./e/hacked/e16/e/src/ipc.c 2004-09-02 10:03:22.716571976 +0400
@@ -4432,6 +4432,11 @@
char param[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)
{
   char                buf[FILEPATH_LEN_MAX];

   buf[0] = 0;

   if (params)
     {
        char                param1[FILEPATH_LEN_MAX];
        char                param2[FILEPATH_LEN_MAX];
        char                param3[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 "20000K of some trash withoun \n"

        or something like :

    name[0] = "/usr/X11R6/bin/eesh"   ;
    name[1] = "button"                ;
    name[3] = NULL              ;
    name[2] =  malloc(20000)    ;
    int i = 0                   ;
    for(i=0;i<20000;i++)
    {
        name[2][i] = 'A'        ;
    }
    execve(name[0],name,NULL)   ;

hopefully nothing, but still this two functions word and fword are not safe,
scanf("%<num>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 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

Reply via email to