pointer changes value between calls, why/how?

[dtschoepe]

Hi all,

Working on homework again...

I've got a weird problem, I've been banging my head against the wall
on what is causing it. I have a pointer to a typdef named Person. At
one point in the code the pointer changes from storing the memory
location of the Person and contains the value '1'.

I have commented out several pieces of code in order to try to
eliminate the cause but no success so far. I have also inserted in the
code comments where the problem or interesting areas are located. The
code consists of 4 files, I will list their contents here....

matchmaker.c
functions.c
functions.h
input.txt

What happens is that the program reads in the file "input.txt". I have
limited it to reading one set of data until I fix the seg fault issue
(commented out while loop). After it reads in the 1 set of data I have
a line that is supposed to print the first name of the "Person" using
a pointer points to firstname call in matchmaker.c (after the
commented while loop). I have commented the line right before where
the seg fault happens and where the pointer has somehow gotten the
value '1' assigned into it.

Hopefully I can learn something from someone here tonight.

#include "functions.h"

int main(int argc, char * argv[])
{

char *filename;
FILE *fin;
FILE *fout;
Person *memberptr;
Person member;
int error, count=0;;

filename = getFileName();
fin = openFile(filename, "r");
fout = openFile("HW3output.txt", "w");
printf("\nMemberPTR contents: %d\n", *memberptr);
printf("MemberPTR address: %x\n\n", &memberptr);

/* get member details and create nodes */
//while (!feof(fin))
//{
printf("count value: %d\n", count);
memberptr = createMember(fin);
printf("Name %s\n", memberptr->firstName);

// Here *memberptr has the value of a memory location
printf("MemberPTR contents: %x\n", *memberptr);
printf("MemberPTR address: %x\n", &memberptr);
//addMember(memberptr);
printf("Name %s\n", memberptr->firstName);
count++;
printf("member added, count value: %d\n", count);
//}

// Here is where *memberptr == 1
// Why would it change from a memory address to 1 here?
printf("out of while loop\n");
printf("MemberPTR contents: %x\n", *memberptr);
printf("MemberPTR address: %x\n", &memberptr);

// Here we get a seg fault because memberptr == 1 and not a valid
memory location
printf("Name %s\n", memberptr->firstName);
printMember(memberptr);
sortMembers();
matchMembers();

/* free memory */
deleteAll();

/* close files */
error = closeFile(fin);
error = closeFile(fout);

return EXIT_SUCCESS;
}


/* functions.c */
#include "functions.h"

Node *head = NULL;

Person *createMember(FILE *infile)
{
char fname[30];
char lname[30];
char sex[10];
char age[10];
char color[20];
char hobby[30];
char *colorS;
Person member;
Person *memberptr;

if (!feof(infile))
{
fgets(fname, 30, infile);
fgets(lname, 30, infile);
fgets(sex, 10, infile);
fgets(age, 10, infile);
fgets(color, 20, infile);
fgets(hobby, 30, infile);
}

else
return NULL;

/* read member attributes from file */
if (fname[strlen(fname)-1] == '\n')
fname[strlen(fname)-1] = '\0';
if (lname[strlen(lname)-1] == '\n')
lname[strlen(lname)-1] = '\0';
if (color[strlen(color)-1] == '\n')
color[strlen(color)-1] = '\0';
if (hobby[strlen(hobby)-1] == '\n')
hobby[strlen(hobby)-1] = '\0';

/* create memory allocation for member attributes */
member.firstName = calloc(strlen(fname)+1, 1);
member.lastName = malloc(sizeof(lname));
member.hobby = malloc(sizeof(hobby));
colorS = malloc(sizeof(color));

/* store member attributes */
strcpy(member.firstName, fname);
strcpy(member.lastName, lname);
sscanf(sex, "%c", &member.sex);
sscanf(age, "%d", &member.age);
strcpy(colorS, color);
member.favColor = readColor(colorS);
strcpy(member.hobby, hobby);

memberptr = &member;
printf("memberptr: %x\n\n", *memberptr);
return memberptr;
}


enum color readColor(char *theFavColor)
{
enum colors { COLORS };
enum colors theColor;
if ( strcmp(theFavColor, "red") == 0 )
theColor = red;
else if ( strcmp(theFavColor, "blue") == 0)
theColor = blue;
else if ( strcmp(theFavColor, "green") == 0)
theColor = green;
else if ( strcmp(theFavColor, "yellow") == 0)
theColor = yellow;
else if ( strcmp(theFavColor, "black") == 0)
theColor = black;
else if ( strcmp(theFavColor, "purple") == 0)
theColor = purple;
else if ( strcmp(theFavColor, "pink") == 0)
theColor = pink;
else
theColor = none;

return theColor;
}

void printMember(Person *memberptr)
{
char color[10];
switch(memberptr->favColor)
{
case red :
strcpy(color, "red");
break;
case blue :
strcpy(color, "blue");
break;
case green :
strcpy(color, "green");
break;
case yellow :
strcpy(color, "yellow");
break;
case black :
strcpy(color, "black");
break;
case purple :
strcpy(color, "purple");
break;
case pink :
strcpy(color, "pink");
break;
default:
strcpy(color, "N/A");
break;
}

printf("starting to print person\n");
printf("Name: %s %s\n", memberptr->firstName, memberptr->lastName);
printf("Sex: %c\n", memberptr->sex);
printf("Age: %d\n", memberptr->age);
printf("Favorite Color: %s\n", color);
printf("Hobby: %s\n\n", memberptr->hobby);
}

int addMember(Person *member) {

Node *cur = NULL;

cur = (Node *)malloc(sizeof(Node));

if(cur == NULL)
return 0;

cur -> client = member;
cur -> next = NULL;

if(head == NULL)
head = cur;
else
{
cur -> next = head;
head = cur;
}

return 1;
}

void deleteAll() {

}

void sortMembers() {

}

void matchMembers() {

}

/* ask user for filename and return char pointer */
char *getFileName()
{
char temp[100];
char *filename;
int len = 0;
printf("Enter a filename to open: ");
fgets(temp, sizeof(temp), stdin);
len = strlen(temp);
temp[len-1] = '\0';
len = strlen(temp);
filename = (char *)malloc(sizeof(char) * (len - 1));
strcpy(filename, temp);
return filename;
}

FILE *openFile(char *filename, char *mode)
{
FILE *fp;

while ((fp = fopen(filename, mode)) == NULL)
{
printf("The file '%s' was not found. Enter a file to open.\n",
filename);
printf("Or enter 'q' to quit: ");
filename = getFileName();
if (filename[0] == 'q' && strlen(filename) == 1)
break;
}
return fp;
}

int closeFile(FILE *file)
{
int error = 0;
if (file != NULL)
fclose(file);
else
error = 1;
return error;
}

/* functions.h */

#ifndef FUNCTIONS_H
#define FUNCTIONS_H

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

#define COLORS red, blue, green, yellow, black, purple, pink, none


enum color { COLORS };
typedef enum color Color;

typedef struct person
{
char *firstName;
char *lastName;
char *hobby;
Color favColor;
char sex;
int age;
} Person;

struct node
{
struct node *next;
Person *client;
};
typedef struct node Node;


/* Node functions */
int addMember(Person *member);
void deleteAll();

/* Person functions */
Person *createMember(FILE *infile);
enum color readColor(char *theFavColor);
void printPerson(Person *member);
void sortMembers();
void matchMembers();


/* I/O functions */
char *getFileName();
FILE *openFile(char *filename, char *mode);
int closeFile(FILE *file);


#endif


/* input.txt */

bubba
von bigbelly
m
44
purple
watching pokemon
sally sue
sullivan
f
40
purple
working out
faith
hill
f
32
green
writing songs




David

 

[Gordon Burditt]

Working on homework again...

I've got a weird problem, I've been banging my head against the wall
on what is causing it. I have a pointer to a typdef named Person. At

typedefs don't have addresses. Variables do.

one point in the code the pointer changes from storing the memory
location of the Person and contains the value '1'.

Person *createMember(FILE *infile)
{ ....
Person member;
Person *memberptr; ....
memberptr = &member; ....
return memberptr;

You are returning a pointer to an auto variable. That's a lot
like sending postal mail from a house being demolished, and
expecting an answer to be deliverable. This is an EXTREME no-no.

 

[davenet]

typedefs don't have addresses. Variables do.





You are returning a pointer to an auto variable. That's a lot
like sending postal mail from a house being demolished, and
expecting an answer to be deliverable. This is an EXTREME no-no.

Enlighten me Gordon, what is an auto variable? I am not clear as to
what that is even though you snipped my code to make that point.

Thanks,

David

 

[davenet]

typedefs don't have addresses. Variables do.





You are returning a pointer to an auto variable. That's a lot
like sending postal mail from a house being demolished, and
expecting an answer to be deliverable. This is an EXTREME no-no.

OK, I'm sort of getting it. Are you referring here to the variable
having local scope and disappearing after the function call is done? I
was under the impression that memory allocated on the heap would not
go away until you free it? So here I am making a Person variable
inside this function and then returning a pointer to that address. Is
that a wrong understanding of what I can do?

David

 

[davenet]

OK, I figured it out...

Gordon, thanks for taking the time to get me going in the right
direction.

David

 

[Gordon Burditt]

one point in the code the pointer changes from storing the memory

OK, I'm sort of getting it. Are you referring here to the variable
having local scope and disappearing after the function call is done? I

Yes. This is an automatic variable. You declare them inside a function
definition without the 'static' keyword. They go away when the function
returns.

was under the impression that memory allocated on the heap would not

What memory allocated on the heap? (Not 'member' - it's an automatic
variable).

go away until you free it?

The only sensible definition of "heap" in this newsgroup (since the
C standard does not define that term, nor does it define 'staque'
nor anything about memory being divided into 'smegments') is "that
place from which malloc() and family get their memory". The trouble
here is, the variable 'member' was *NOT* allocated with malloc().
It is an automatic variable. It goes away when the function returns.

(Maybe it should be allocated with malloc(). Or you could declare
it static. Both have their problems. If you allocate it with
malloc(), you need to remember to free() it. If you make it static,
you need to *copy* it before calling the function again or it will
get overwritten.)

So here I am making a Person variable
inside this function and then returning a pointer to that address. Is
that a wrong understanding of what I can do?

A pointer to memory that has gone away isn't particularly useful
for anything. It's a disaster waiting to happen. Don't do that.
Return a pointer to dynamically allocated memory (malloc()), or
a static variable. Or pass in a pointer to memory to put the result in.

 

[Barry Schwarz]

Hi all,

Working on homework again...

I've got a weird problem, I've been banging my head against the wall
on what is causing it. I have a pointer to a typdef named Person. At
one point in the code the pointer changes from storing the memory
location of the Person and contains the value '1'.

I have commented out several pieces of code in order to try to
eliminate the cause but no success so far. I have also inserted in the
code comments where the problem or interesting areas are located. The
code consists of 4 files, I will list their contents here....

matchmaker.c
functions.c
functions.h
input.txt

What happens is that the program reads in the file "input.txt". I have
limited it to reading one set of data until I fix the seg fault issue
(commented out while loop). After it reads in the 1 set of data I have
a line that is supposed to print the first name of the "Person" using
a pointer points to firstname call in matchmaker.c (after the
commented while loop). I have commented the line right before where
the seg fault happens and where the pointer has somehow gotten the
value '1' assigned into it.

Hopefully I can learn something from someone here tonight.

#include "functions.h"

int main(int argc, char * argv[])
{

char *filename;
FILE *fin;
FILE *fout;
Person *memberptr;
Person member;
int error, count=0;;

filename = getFileName();
fin = openFile(filename, "r");
fout = openFile("HW3output.txt", "w");
printf("\nMemberPTR contents: %d\n", *memberptr);

memberptr was never given a value. Attempting to dereference it
invokes undefined behavior. Unless Person is a synonym for int, %d is
the wrong format for it.

printf("MemberPTR address: %x\n\n", &memberptr);

%x is the wrong format for an address argument. Use %p and cast the
corresponding argument to void*.

/* get member details and create nodes */
//while (!feof(fin))

If you uncomment this, it will not do what your want.

//{
printf("count value: %d\n", count);
memberptr = createMember(fin);

See the note in createMember. memberptr now contains an invalid
address. Any attempt to use it invokes undefined behavior.

printf("Name %s\n", memberptr->firstName);

// Here *memberptr has the value of a memory location
printf("MemberPTR contents: %x\n", *memberptr);

No it does not. It is an expression of type Person, which happens to
be a struct. Passing a struct to printf is always wrong.

printf("MemberPTR address: %x\n", &memberptr);
//addMember(memberptr);
printf("Name %s\n", memberptr->firstName);
count++;
printf("member added, count value: %d\n", count);
//}

// Here is where *memberptr == 1

Not likely. *memberptr is still an expression of type Person.

// Why would it change from a memory address to 1 here?

Since your printf invokes undefined behavior, you have no idea what
the value of memberptr is or what it points to.

printf("out of while loop\n");
printf("MemberPTR contents: %x\n", *memberptr);
printf("MemberPTR address: %x\n", &memberptr);

The expression &memberptr is a run time constant. It is the address
of memberptr, not the address that memberptr points to.

// Here we get a seg fault because memberptr == 1 and not a valid
memory location

Which is it - is memberptr 1 or is *memberptr 1?

printf("Name %s\n", memberptr->firstName);
printMember(memberptr);
sortMembers();
matchMembers();

/* free memory */
deleteAll();

/* close files */
error = closeFile(fin);
error = closeFile(fout);

return EXIT_SUCCESS;
}


/* functions.c */
#include "functions.h"

Node *head = NULL;

Person *createMember(FILE *infile)
{
char fname[30];
char lname[30];
char sex[10];
char age[10];
char color[20];
char hobby[30];
char *colorS;
Person member;
Person *memberptr;

if (!feof(infile))
{
fgets(fname, 30, infile);
fgets(lname, 30, infile);
fgets(sex, 10, infile);
fgets(age, 10, infile);
fgets(color, 20, infile);
fgets(hobby, 30, infile);
}

else
return NULL;

/* read member attributes from file */
if (fname[strlen(fname)-1] == '\n')
fname[strlen(fname)-1] = '\0';
if (lname[strlen(lname)-1] == '\n')
lname[strlen(lname)-1] = '\0';
if (color[strlen(color)-1] == '\n')
color[strlen(color)-1] = '\0';
if (hobby[strlen(hobby)-1] == '\n')
hobby[strlen(hobby)-1] = '\0';

/* create memory allocation for member attributes */
member.firstName = calloc(strlen(fname)+1, 1);
member.lastName = malloc(sizeof(lname));
member.hobby = malloc(sizeof(hobby));
colorS = malloc(sizeof(color));

/* store member attributes */
strcpy(member.firstName, fname);
strcpy(member.lastName, lname);
sscanf(sex, "%c", &member.sex);
sscanf(age, "%d", &member.age);
strcpy(colorS, color);
member.favColor = readColor(colorS);
strcpy(member.hobby, hobby);

memberptr = &member;
printf("memberptr: %x\n\n", *memberptr);
return memberptr;

You are returning the address of an object that will cease to exist
whne the function exits.

}

snip rest of code


Remove del for email