Free memory cause segmentation fault.

H

Hon Seng Phuah

Hi all,

I do not understand why my code can cause segmentation fault in Unix Gcc.

#include <stdlib.h>
#include <stdio.h>

int main()
{
char *directory,
*pos,
file_name[] = "Geco/ich7/A0/abc/cell_name/mail/good.geco";
int count = 0;

pos = strtok(file_name, "/");
directory = (char *) malloc(sizeof(char) * (strlen(file_name) + 1));
directory[0] = '\0';
while (pos)
{
strcat(directory, pos);
pos = strtok(NULL, "/");
}
free(directory); /* segmentation faults occurs here */
return 0;
}

If you know what I did wrong, please point out for me. Thanks.
 
J

Jack Klein

Hi all,

I do not understand why my code can cause segmentation fault in Unix Gcc.

Have you read and understood the operation of the strtok() function?
Are you aware that it overwrites the character terminating each token
with a '\0', thus truncating the string?
#include <stdlib.h>
#include <stdio.h>

int main()
{
char *directory,
*pos,
file_name[] = "Geco/ich7/A0/abc/cell_name/mail/good.geco";
int count = 0;

pos = strtok(file_name, "/");

After this call, the file_name array contains:

"Geco" "\0" "ich7/A0/abc/cell_name/mail/good.geco"

And you should never use the pointer returned by strtok(), even the
first call, without checking it for NULL.
directory = (char *) malloc(sizeof(char) * (strlen(file_name) + 1));

First, never cast the return value of malloc() in C, do a Google
groups search on this group to see the reason why. If your compiler
complains about this even though you have included <stdlib.h>, you are
not using C but some other language.

Secondly, never multiply by sizeof(char) as this is 1 by definition,
always has been, and always will be.

Thirdly, never use a pointer returned by malloc(), calloc(), or
realloc() without checking it for NULL.

But your problem is caused by the fact that, as I have shown above,
your first call to strtok() has overwritten the fifth character in the
original string, the first '/' character, with a null byte, so
strlen() will now return 4, and you will allocate 5 bytes for
directory.
directory[0] = '\0';
while (pos)
{
strcat(directory, pos);

The first pass through this loop, you copy the four characters "Geco"
plus a '\0' into directory. You have now used all of the memory you
allocated.

On the second and following repetitions of the loop, you copy 31 more
characters past the end of allocated memory.
pos = strtok(NULL, "/");
}
free(directory); /* segmentation faults occurs here */
return 0;
}

If you know what I did wrong, please point out for me. Thanks.

Writing past the end of allocated memory produces undefined behavior,
often causing problems that show up on later calls to memory
allocation functions.

But very seriously reconsider your use of strtok() until you are sure
you understand its operation.
 
A

Allin Cottrell

Hon said:
Hi all,

I do not understand why my code can cause segmentation fault in Unix Gcc.

#include <stdlib.h>
#include <stdio.h>

#include said:
int main()
{
char *directory,
*pos,
file_name[] = "Geco/ich7/A0/abc/cell_name/mail/good.geco";
int count = 0;

unused variable count.
pos = strtok(file_name, "/");
directory = (char *) malloc(sizeof(char) * (strlen(file_name) + 1));

You obviously have not read many postings to comp.lang.c.
(1) sizeof(char) == 1 by definition.
(2) You should not cast the return of malloc(); there's no advantage,
and it can mask failure to #include <stdlib.h> (although you did
remember this).

directory = malloc(strlen(file_name) + 1); /* nicer, isn't it? */

By the way, you should, in general, check that malloc didn't return
NULL.
directory[0] = '\0';
while (pos)
{
strcat(directory, pos);
pos = strtok(NULL, "/");
}
free(directory); /* segmentation faults occurs here */
return 0;
}

I don't get a segfault after the above corrections.

If you use gcc, be aware that you can get good error and warning
messages if you use the right flags, for example

gcc -O -W -Wall

With these flags, your original source file produced this:

myrtle:~$ gcc -O -W -Wall foo.c
foo.c: In function `main':
foo.c:11: warning: implicit declaration of function `strtok'
foo.c:11: warning: assignment makes pointer from integer without a cast
foo.c:13: warning: implicit declaration of function `strlen'
foo.c:18: warning: implicit declaration of function `strcat'
foo.c:19: warning: assignment makes pointer from integer without a cast
foo.c:9: warning: unused variable `count'

Allin Cottrell
 
A

Allin Cottrell

I said:
#include <string.h> /* required for prototypes of str* functions */

Yes, but Jack Klein in this thread has spotted the real reason for
your segmentation fault, which I missed. Mea culpa.

Allin Cottrell.
 
M

Mike Wahler

Hon Seng Phuah said:
Hi all,

I do not understand why my code can cause segmentation fault in Unix Gcc.

#include <stdlib.h>
#include <stdio.h>

int main()
{
char *directory,
*pos,
file_name[] = "Geco/ich7/A0/abc/cell_name/mail/good.geco";
int count = 0;

pos = strtok(file_name, "/");
directory = (char *) malloc(sizeof(char) * (strlen(file_name) + 1));
directory[0] = '\0';
while (pos)
{
strcat(directory, pos);
pos = strtok(NULL, "/");
}
free(directory); /* segmentation faults occurs here */
return 0;
}

If you know what I did wrong, please point out for me. Thanks.

I've modified your code to detect and indicate the problem, as well
as to fix a few other errors:

#include <stdlib.h>
#include <stdio.h>
#include <string.h>

int main()
{
char *directory,
*pos,
file_name[] = "Geco/ich7/A0/abc/cell_name/mail/good.geco";

int count = 0;
size_t alloc_size = 0;
size_t sum = 0;

pos = strtok(file_name, "/"); /* 1) */
alloc_size = strlen(file_name + 1); /* 2) */

directory = malloc(alloc_size);
if(!directory)
{
printf("Cannot allocate memory\n");
return EXIT_FAILURE;
}

printf("alloc_size == %lu\n", (unsigned long)alloc_size);
printf("Press ENTER to continue:");
fflush(stdout);
getchar();
putchar('\n');

directory[0] = '\0';

while (pos)
{
printf("directory == %s\n"
"strlen(directory) == %lu\n",
directory, (unsigned long)strlen(directory));

printf("pos == %s\n"
"strlen(pos) == %lu\n",
pos, (unsigned long)strlen(pos));

sum = strlen(directory) + strlen(pos) + 1;

printf("%lu + %lu + %d == %lu",
(unsigned long)strlen(directory),
(unsigned long)strlen(pos),
1,
(unsigned long)sum);

if(sum > alloc_size)
printf(" [greater than %lu, not enough room for strcat()!]",
alloc_size);
else
{
printf(" [Sufficient room, concatenating.]");
strcat(directory, pos);
}

putchar('\n');
printf("Press ENTER to continue:");
fflush(stdout);
getchar();
putchar('\n');
pos = strtok(NULL, "/");
}

printf("directory == %s\n", directory);
printf("strlen(directory) == %lu\n",
(unsigned long)strlen(directory));

free(directory); /* segmentation faults occurs here (NOT ANY MORE!) */
return 0;
}


1. After you compile and run this, reverse the order of the lines I've
marked with /* 1) */ and /* 2) */, then compile and run it again.

2. Carefully re-read the documentation for 'strtok()'.


HTH,
-Mike

-Mike
 
E

Emmanuel Delahaye

In said:
Hi all,

I do not understand why my code can cause segmentation fault in Unix Gcc.

#include <stdlib.h>
#include <stdio.h>

int main()
{
char *directory,
*pos,
file_name[] = "Geco/ich7/A0/abc/cell_name/mail/good.geco";
int count = 0;

pos = strtok(file_name, "/");
directory = (char *) malloc(sizeof(char) * (strlen(file_name) + 1));
directory[0] = '\0';
while (pos)
{
strcat(directory, pos);
pos = strtok(NULL, "/");
}
free(directory); /* segmentation faults occurs here */
return 0;
}

If you know what I did wrong, please point out for me. Thanks.

Thanks you system for having catched the Undefined Behaviour. Mine was silent
about it.

You don't allocate space enough. It was a little tricky to figure out. I have
instrumented your code:

#include <stdlib.h>
#include <stdio.h>
#include <string.h>

#include "ed/inc/sys.h"

int main ()
{
char file_name[] = "Geco/ich7/A0/abc/cell_name/mail/good.geco";
char *pos = strtok (file_name, "/");
size_t const size = strlen (file_name) + 1;
char *directory = malloc (size);

if (directory != NULL)
{
LIM_PTR (directory, size);

directory[0] = '\0';
while (pos)
{
strcat (directory, pos);
printf ("'%s'\n", directory);
CHK_PTR (directory, size);

pos = strtok (NULL, "/");
}
free (directory); /* segmentation faults occurs here */
}
return 0;
}

missing code at
http://mapage.noos.fr/emdel/clib.htm
Module SYS

And I got:

D:\CLC\H\HONG>bc proj.prj
'Geco'
'Gecoich7'
SYS: assertion '(directory)[(size) -1]==0' failed at MAIN.C:23
SYS: SYS_print_stack() : Undefined in ISO-C
SYS: *** EXIT ***

After further investigation, I have discovered that you called strtok()
before calling strlen(), hence the length was wrong (too short) and you
didn't asked space enough to malloc().

The fix: Invert the order of two lines:

size_t const size = strlen (file_name) + 1;
char *pos = strtok (file_name, "/");

Now, I've got:

D:\CLC\H\HONG>bc proj.prj
'Geco'
'Gecoich7'
'Gecoich7A0'
'Gecoich7A0abc'
'Gecoich7A0abccell_name'
'Gecoich7A0abccell_namemail'
'Gecoich7A0abccell_namemailgood.geco'

Sounds good to me.
 
H

Hon Seng Phuah

Allin Cottrell said:
Yes, but Jack Klein in this thread has spotted the real reason for
your segmentation fault, which I missed. Mea culpa.

Allin Cottrell.


Thanks for point out my problem. I did not notice that my malloc is
after strtok. Why I did not check for null after calling malloc
because I was just testing my small algorithm and did not expect my
computer has empty heap.
 
M

Mike Wahler

Hon Seng Phuah said:
Allin Cottrell <[email protected]> wrote in message


Thanks for point out my problem. I did not notice that my malloc is
after strtok.

The problem wasn't directly caused by 'malloc()', but by modifying the
string
length of 'file_name' with 'strtok()' before submitting it to 'strlen()'
This caused 'strlen()' to report a truncated string length, which you
passed to 'malloc()'.
Why I did not check for null after calling malloc
because I was just testing my small algorithm and did not expect my
computer has empty heap.

When writing code, I advise you to treat assumptions as deadly diseases.
They *will* get you. When you least expect it. Trust me. :)

-Mike
 
O

Old Wolf

I do not understand why my code can cause segmentation fault in Unix Gcc.

#include <stdlib.h>
#include <stdio.h>

int main()
{
char *directory,
*pos,
file_name[] = "Geco/ich7/A0/abc/cell_name/mail/good.geco";

pos = strtok(file_name, "/");
directory = (char *) malloc(sizeof(char) * (strlen(file_name) + 1));

directory = malloc(sizeof file_name);

The result of 'sizeof' isn't affected by strtok, and doesn't incur
any runtime overhead of parsing the string.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top