Check for malloc fail

[GoogleCodeExporter]

Much of the code uses malloc/calloc/alloca and does not check to see if it
fails.  I wanted to start an issue where developers can post patches for this.

Attached is a patch for cmd_text.c.
copy patch to src/ directory

patch < cmd_text.c.patch

Original issue reported on code.google.com by CHWoi...@gmail.com on 23 Apr 2009 at 1:36

Attachments:

• cmd_text.c.patch

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Original comment by andre....@gmail.com on 23 Apr 2009 at 11:14

• Changed state: Accepted
• Added labels: Milestone-Release0.3.2

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I guess before I started posting these I should have asked how we should handle 
a
fail.  Currently in color.c we print out a message and exit.  Should we do the 
same
everywhere else?  For the patches above, I simply returned NULL.  In a later 
round of
code review I was going to actually check to see if any calls return an error 
such as
NULL and then maybe print an error message or let it propagate. If we decide on
non-NULL return, like printing and error message and exiting, then I will 
delete the
above comments.  The reason I would like to return NULL is that I would like to 
have
the rest of the code clean up after itself as much as possible before exiting.

Original comment by CHWoi...@gmail.com on 23 Apr 2009 at 11:31

[GoogleCodeExporter]

About the fast changes in the codebase: no problem about that - the patch 
utility is 
smart enough to find where the change happened and patch correctly. And if it 
doesn't, 
then I can do by hand.

About how to handle a fail: the best is to print a message (such as "not enough 
memory") to stderr and quit. A malloc is a event catastrophic enough to shut 
down the 
program.

Original comment by andre....@gmail.com on 23 Apr 2009 at 11:50

[GoogleCodeExporter]

Alright, I have deleted the previous comments and will submit a larger patch 
which
will patch multiple files.

Original comment by CHWoi...@gmail.com on 23 Apr 2009 at 12:10

[GoogleCodeExporter]

Should we replace calls to malloc/alloca with xsw_malloc/xsw_alloca ?  These 
calls
will then print the error and exit.  If so, should these functions take an extra
parameter that gets printed as the error? 

... = xsw_malloc(sizeof(somee struct), "Could not alloc ...\n");

or, should we have the same parameter with a default "not enough memory" error 
message?

Original comment by CHWoi...@gmail.com on 23 Apr 2009 at 12:59

[GoogleCodeExporter]

I think just printing "Not enough memory" is enough. Replacing malloc is never 
a good 
idea, it can be confusing and makes it harder for tools like efence to debug it.

Original comment by andre....@gmail.com on 23 Apr 2009 at 2:52

[GoogleCodeExporter]

Apply patch to r151.

Copy patch file to src/ directory
and apply patch it using the command: patch -p1 < xsw.malloc.patch

This patch applies something like the following to all mallocs and allocas

    if (cmd_img == NULL) 
    {
        fprintf(stderr, "Not enough memory\n");
        exit(1);        
    }

Original comment by CHWoi...@gmail.com on 26 Apr 2009 at 12:39

Attachments:

• xsw.malloc.patch

[GoogleCodeExporter]

Original comment by morganad...@gmail.com on 27 Apr 2009 at 10:36

• Changed state: InSVN

[GoogleCodeExporter]

Original comment by andre....@gmail.com on 30 Apr 2009 at 1:27

• Changed state: Fixed