Call to free() sefaults

C

Chris Potter

Hello everyone. I have two questions both of which regard a homework
assignment for my "Intro to C" class. The First question that i have
is that my program segfaults when i free() memory that i malloc'd and
i'm not sure why. (without free()ing the program operates as i would
expect it to) Here is the code:

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

#define MAX 75
#define MAX_NAMES 3
#define NAME_SIZE 25

struct Name {
char name[NAME_SIZE];
struct Name *next;
};

typedef struct Name NAME;

int main (void)
{

char *entry;
int pos = 0;
int count = 0;
int inword = 0;
NAME first, middle, last, *start, *list;

/* allocate space for *entry */
entry = malloc (MAX * sizeof (char));
if (entry == NULL ) {
printf ("\n malloc() failed!\n");
exit (1);
}

/* setup list */
start = &first;
first.next = &middle;
middle.next = &last;
last.next = NULL;
list = start;
/* null first elements in case user doesn't give 3 names */
first.name[0] = '\0';
middle.name[0] = '\0';
last.name[0] = '\0';

/* input to get first, middle and last name */
printf ("Please enter your full name: ");
fgets (entry, MAX, stdin);

/* split entry up into separate name fields */
while ((*entry) && (count < MAX_NAMES)) {
if (inword) {
if (isspace (*entry)) {
list->name[pos] = '\0';
list = list->next;
inword = 0;
pos = 0;
++count;
} else
list->name[pos++] = *entry;
} else
if (!isspace (*entry)) {
list->name[pos++] = *entry;
inword = 1;
}
++entry;
}

free (entry);

/* output list structure */
list = start;
printf ("Name in list structure is: ");
while (list != NULL) {
printf ("%s ", list->name);
list = list->next;
}

printf ("\n");

return 0;
}

I'm not neccesarily looking for an "answer", maybe just a nudge in the
direction of what the problem could be, and then i could root it out.

The second question is regarding the code where i put '\0' characters
into the first elements of the arrays in the NAME structures. I wanted
to just initialize the first element in the "name" array in the NAME
structure where the stuct was declared but the compiler wouldn't
accept that. Thanks in advance for any and all comment/tips/advice.

-Chris Potter
 
M

Mike Wahler

Chris Potter said:
Hello everyone. I have two questions both of which regard a homework
assignment for my "Intro to C" class. The First question that i have
is that my program segfaults when i free() memory that i malloc'd and
i'm not sure why. (without free()ing the program operates as i would
expect it to) Here is the code:
Click to expand...

The key here is that 'free()'s argument must be either
NULL, or the value returned from a called to 'malloc()',
'calloc()', or 'realloc()'. Your argument does not meet
these conditions (yes, I'm sure you did it inadvertently. :))

See below.
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <ctype.h>

#define MAX 75
#define MAX_NAMES 3
#define NAME_SIZE 25

struct Name {
char name[NAME_SIZE];
struct Name *next;
};

typedef struct Name NAME;

int main (void)
{

char *entry;
int pos = 0;
int count = 0;
int inword = 0;
NAME first, middle, last, *start, *list;

/* allocate space for *entry */
entry = malloc (MAX * sizeof (char));
if (entry == NULL ) {
printf ("\n malloc() failed!\n");
exit (1);
}
Click to expand...

Display or otherwise record the value of 'entry' here.


/* setup list */
start = &first;
first.next = &middle;
middle.next = &last;
last.next = NULL;
list = start;
/* null first elements in case user doesn't give 3 names */
first.name[0] = '\0';
middle.name[0] = '\0';
last.name[0] = '\0';

/* input to get first, middle and last name */
printf ("Please enter your full name: ");
fgets (entry, MAX, stdin);

/* split entry up into separate name fields */
while ((*entry) && (count < MAX_NAMES)) {
if (inword) {
if (isspace (*entry)) {
list->name[pos] = '\0';
list = list->next;
inword = 0;
pos = 0;
++count;
} else
list->name[pos++] = *entry;
} else
if (!isspace (*entry)) {
list->name[pos++] = *entry;
inword = 1;
}
++entry;
}

free (entry);
Click to expand...

Display or otherwise record the value of 'entry' here.

(Assuming the loop is not applied to an 'empty' string)
You'll notice that the value of 'entry' at this point
won't be the same as that above. (Because of your increment
(++entry) in the 'while' loop.

You can fix this problem by not incrementing 'entry' at all,
but accessing the array with a subscript which you'd increment
instead of the pointer:

(At top of function:)

size_t index = 0;

modified loop:)
while ((entry[index]) && (count < MAX_NAMES)) {
/* etc */
++index;
}
/* output list structure */
list = start;
printf ("Name in list structure is: ");
while (list != NULL) {
printf ("%s ", list->name);
list = list->next;
}

printf ("\n");

return 0;
}

I'm not neccesarily looking for an "answer", maybe just a nudge in the
direction of what the problem could be, and then i could root it out.
Click to expand...

See above. Also, as I've alluded, don't forget that output
of particular object values in strategic locations can be
very helpful when debugging.

The second question is regarding the code where i put '\0' characters
into the first elements of the arrays in the NAME structures. I wanted
to just initialize the first element in the "name" array in the NAME
structure where the stuct was declared but the compiler wouldn't
accept that.
Click to expand...

What statements did you use, and what exactly did the compiler say?


NAME first = {""};

/* ( first.name[0] initialized to zero, 'next' initialized to NULL) */

-Mike
 
G

Gordon Burditt

I'm not neccesarily looking for an "answer", maybe just a nudge in the
direction of what the problem could be, and then i could root it out.
Click to expand...

You allocated some memory, you MODIFIED the pointer to it by
incrementing it, and then you freed it. Does that suggest a problem
to you?

Gordon L. Burditt
 
H

Hallvard B Furuseth

Chris said:
printf ("\n malloc() failed!\n");
Click to expand...

Note: Common practice is to write error messages to stderr, not stdout.
if (isspace (*entry)) {
Click to expand...

Wrong. The ctype.h functions expect the argument to be a character
converted to the range of `unsigned char', or EOF. So use

isspace ((unsigned char) *entry)

otherwise it can break if `char' is signed and *entry is negative.
 
M

Martin Ambuhl

Chris said:
Hello everyone. I have two questions both of which regard a homework
assignment for my "Intro to C" class. The First question that i have
is that my program segfaults when i free() memory that i malloc'd and
i'm not sure why.
Click to expand...

Because that's *not* what you did.
(without free()ing the program operates as i would
expect it to) Here is the code:
Click to expand...

[... much snippage follows ...]

/* allocate space for *entry */
entry = malloc (MAX * sizeof (char));
if (entry == NULL ) {
printf ("\n malloc() failed!\n");
exit (1);
}
Click to expand...

Here you allocated the space. But note that sizeof(char)==1 by definition,
so is pointless to include, while exit(1) has no portably defined meaning
(but exit(EXIT_FAILURE) does).
[...]
++entry;
}
Click to expand...

Here you modify entry to point somewhere other than the beginning of the
space allocated
free (entry);
Click to expand...

You are now trying to free with a pointer value not returned by malloc

[...]
The second question is regarding the code where i put '\0' characters
into the first elements of the arrays in the NAME structures. I wanted
to just initialize the first element in the "name" array in the NAME
structure where the stuct was declared but the compiler wouldn't
accept that. Thanks in advance for any and all comment/tips/advice.
Click to expand...

You need to show what you are trying to do. Initializing the first element
of an array (or even no elements of a static array) is sufficient, so you
are doing something other than this if your compiler complains.
 
E

Emmanuel Delahaye

In said:
char *entry;
Click to expand...
entry = malloc (MAX * sizeof (char));
while ((*entry) && (count < MAX_NAMES)) {
++entry;
}

free (entry);
Click to expand...

You have broken a sacred rule 'the value passed to free() must be value
received from malloc().

To prevent that problem, it's good practice to define the pointer 'const':

char *const entry = malloc (MAX * sizeof (char));
while ((*entry) && (count < MAX_NAMES)) {
++entry; /* Compiler error */
}

free (entry);

A fix is to use a 'mobile pointer' to read the entry (a local poinrer
initialized with the value of 'entry'), or to use an index ('entry' is an
array of char).

The second question is regarding the code where i put '\0' characters
into the first elements of the arrays in the NAME structures. I wanted
to just initialize the first element in the "name" array in the NAME
structure where the stuct was declared but the compiler wouldn't
accept that. Thanks in advance for any and all comment/tips/advice.
Click to expand...
NAME first, middle, last, *start, *list;
Click to expand...

It's considered ugly and hard to maintain to define more that one objet per
statement. Better to stick to 'one objet, one statement' rule, and to
initialize the structure. A simple '0' is enough to force the whole structure
to 0.

NAME first = {0};
NAME middle = {0};
NAME last = {0};
NAME *start;
NAME *list;
 
B

Barry Schwarz

Hello everyone. I have two questions both of which regard a homework
assignment for my "Intro to C" class. The First question that i have
is that my program segfaults when i free() memory that i malloc'd and
i'm not sure why. (without free()ing the program operates as i would
expect it to) Here is the code:

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

#define MAX 75
#define MAX_NAMES 3
#define NAME_SIZE 25

struct Name {
char name[NAME_SIZE];
struct Name *next;
};

typedef struct Name NAME;

int main (void)
{

char *entry;
Click to expand...

entry is uninitialized.
int pos = 0;
int count = 0;
int inword = 0;
NAME first, middle, last, *start, *list;

/* allocate space for *entry */
entry = malloc (MAX * sizeof (char));
if (entry == NULL ) {
printf ("\n malloc() failed!\n");
exit (1);
}
Click to expand...

entry points to area obtained by malloc.
/* setup list */
start = &first;
first.next = &middle;
middle.next = &last;
last.next = NULL;
list = start;
/* null first elements in case user doesn't give 3 names */
first.name[0] = '\0';
middle.name[0] = '\0';
last.name[0] = '\0';

/* input to get first, middle and last name */
printf ("Please enter your full name: ");
fgets (entry, MAX, stdin);

/* split entry up into separate name fields */
while ((*entry) && (count < MAX_NAMES)) {
if (inword) {
if (isspace (*entry)) {
list->name[pos] = '\0';
list = list->next;
inword = 0;
pos = 0;
++count;
} else
list->name[pos++] = *entry;
} else
if (!isspace (*entry)) {
list->name[pos++] = *entry;
inword = 1;
}
++entry;
Click to expand...

entry no longer points to start of area obtained by malloc.
}

free (entry);
Click to expand...

Bzzt. Undefined behavior. entry does not point to start of area
obtained by malloc. The only acceptable values that may be passed to
free are:

the exact value previously returned by malloc and not yet freed
NULL
/* output list structure */
list = start;
printf ("Name in list structure is: ");
while (list != NULL) {
printf ("%s ", list->name);
list = list->next;
}

printf ("\n");

return 0;
}

I'm not neccesarily looking for an "answer", maybe just a nudge in the
direction of what the problem could be, and then i could root it out.

The second question is regarding the code where i put '\0' characters
into the first elements of the arrays in the NAME structures. I wanted
to just initialize the first element in the "name" array in the NAME
structure where the stuct was declared but the compiler wouldn't
accept that. Thanks in advance for any and all comment/tips/advice.
Click to expand...

NAME first = {""};

This will cause first.name[0] to be set to '\0'. It will also have
the side effect of setting the remaining elements of first.name to
'\0' and first.next to NULL.


<<Remove the del for email>>
 
B

Barry Schwarz

So what? It's first use is in:


Is your observation a remnant from some rule-bound course you took?
Click to expand...

No, it was simply an attempt to show the OP the status of entry from
definition to error. Given the trivial nature of the mistake in the
original post, I refrain from making assumptions about the poster's
understanding of basic concepts.


<<Remove the del for email>>
 

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

Similar Threads

Adding adressing of IPv6 to program 1
Please help with C programming to save GPS reception data in Raspberry Pi. 0
Help with EXT3 Filesystem work 1
Scanf is being prioritized over printf ? 1
Thread-Safe Generic List Queue in C 7
Fibonacci 0
Command Line Arguments 0
Problem with displaying character that code number is 219 (after SetConsoleTextAttribute)? 3

Members online

No members online now.
Total: 24 (members: 0, guests: 24)
Robots: 148

Forum statistics

Threads
473,780
Messages
2,569,611
Members
45,265
Latest member
TodLarocca

Latest Threads

Top