[comp.lang.c] Memory Fault

gh@S5000.UUCP (Sys Admin) (07/19/90)

I got this code from sactoh0!jak (Jay A. Konigsberg)
he wrote the simped editor

when I run simped and try to put any lines in to the file or read any lines
in a existing file I get a core dump

This is the Stack Trace from sdb
------------------------------------------------------------------------
Core file 'core' Memory Fault (11) at 
14 files, 14 procedures.
0x8319c in strdup:23:       buffer[stringinx] = string[stringinx];
>strdup(string="Now is the time.")  [strdup.c:23]
allocate()
addlines(text=0x092d44,overflow=Data address not found
>
------------------------------------------------------------------------
This is the code for strdup.c
any help is appreciated

/*
 * strdup - duplicate a string using malloc
 * CODE NOT TESTED, HOWEVER IT SHOULD WORK FINE FOR simped.
 * ===============
 */
#include "simped.h"

char *strdup(string)
char *string;
{
/*static char *buffer;*/
char *buffer, *malloc();

int stringinx;

if (buffer = (char *) malloc(strlen(string)) == NULL)
    {
    fprintf(stderr, "malloc: in strdup, error\n");
    exit(2);
    }

for (stringinx=0; stringinx <= strlen(string); ++stringinx)
    buffer[stringinx] = string[stringinx];

return(buffer);

}

_____________________________________Glen_______________________________________
______________________________Lexington__Oklahoma_______________________________
______________________{uunet uunet.uu.net}!uokmax!S5000!gh______________________
-- 

_____________________________________Glen_______________________________________
______________________________Lexington__Oklahoma_______________________________
______________________{uunet uunet.uu.net}!uokmax!S5000!gh______________________

barmar@think.com (Barry Margolin) (07/22/90)

In article <189@S5000.UUCP> gh@S5000.UUCP (Sys Admin) writes:
> * CODE NOT TESTED, HOWEVER IT SHOULD WORK FINE FOR simped.

Famous last words...

>if (buffer = (char *) malloc(strlen(string)) == NULL)

Common error: it should be strlen(string)+1.  Strlen() doesn't include the
trailing '\0' in its result, but you need to allocate room for it.
--
Barry Margolin, Thinking Machines Corp.

barmar@think.com
{uunet,harvard}!think!barmar

henry@zoo.toronto.edu (Henry Spencer) (07/22/90)

In article <189@S5000.UUCP> gh@S5000.UUCP (Sys Admin) writes:
> * CODE NOT TESTED, HOWEVER IT SHOULD WORK FINE FOR simped.

"Code not tested" means "code contains bugs".  Always.

>if (buffer = (char *) malloc(strlen(string)) == NULL)

Try

if ((buffer = (char *) malloc(strlen(string)+1)) == NULL)

instead.  The turkey has forgotten that == binds tighter than =, and that
strlen does not count the terminating NUL.

While you're at it, you might change that ridiculous final loop to
something like "strcpy(buffer, string);", which would be clearer and
much faster.

If this is the standard of quality of the rest of the code, I strongly
urge that you never use this editor for anything important.  The author
clearly does not know C.
-- 
NFS:  all the nice semantics of MSDOS, | Henry Spencer at U of Toronto Zoology
and its performance and security too.  |  henry@zoo.toronto.edu   utzoo!henry

robert@cs.arizona.edu (Robert J. Drabek) (07/22/90)

> ... [Jay A. K.] he wrote the simped editor
The code is really bogus.  Sorry.

>  * CODE NOT TESTED, HOWEVER IT SHOULD WORK FINE FOR simped.
We've heard that before.

> int stringinx;
This is not needed; see below.

> if (buffer = (char *) malloc(strlen(string)) == NULL)
  if ((buffer = (char *) malloc(strlen(string) + 1)) == NULL)
      ^                                        ^^^ ^
      1                                        222 1
1.  Missing set of parenthesis; without them buffer is (normally) being
    set to `1'.
2.  The `+ 1' must be there since you need space for an end-of-string
    marker.

> for (stringinx=0; stringinx <= strlen(string); ++stringinx)
>     buffer[stringinx] = string[stringinx];
Use `strcpy(buffer, string);' instead.  The code you (or Jay) gave is Order n
squared, i.e., very inefficient.

> return(buffer);
        ^      ^
Just write `return buffer;', no parenthesis necessary.  (Just a style
point; no further comments solicited.)

For a final remark, the last two lines could actually be collapsed as
  `return strcpy(buffer, string);'

-- 
Robert J. Drabek                            robert@cs.Arizona.EDU
Department of Computer Science              uunet!arizona!robert
The University of Arizona                   602 621 4326
Tucson, AZ  85721

karl@haddock.ima.isc.com (Karl Heuer) (07/23/90)

>if (buffer = (char *) malloc(strlen(string)) == NULL)

Henry has already pointed out the two bugs here; I'd like to note that, as
written, this code is assigning an integer expression to a pointer variable.
Shame on you for ignoring the compiler warning, or shame on the compiler for
silently accepting it.  (This isn't the VMS compiler again, is it?)

Karl W. Z. Heuer (karl@kelp.ima.isc.com or ima!kelp!karl), The Walking Lint

gh@S5000.UUCP (Sys Admin) (07/26/90)

Thanks to all who responded

here's the code that worked
My system is a AT&T 3b1 System V 3.51m

#include "simped.h"

char *strdup(string)
char *string;
{
/*static char *buffer;*/
char *buffer, *malloc();

int stringinx;

if ((buffer = (char *) malloc(strlen(string) + 1)) == NULL)
    {
    fprintf(stderr, "malloc: in strdup, error\n");
    exit(2);
    }

   return strcpy(buffer, string);
}
-- 

_____________________________________Glen_______________________________________
______________________________Lexington__Oklahoma_______________________________
______________________{uunet uunet.uu.net}!uokmax!S5000!gh______________________