Problem in C, pointers galore

[Krakerjak]

Okay, i have a small problem with my code.
First ill give a rundown on what is supposed to be happening.

The program accepts an input of name as a character string, and a date (YYYY MM DD).
The date is copied into 3 variables year, month, day.
This is input in a function iohandling, and sent back to main.

Next, the function make_date takes this data and inputs it all into a DATE structure.

The next function date2str accepts this structure and converts the month value into a string referring to the # in month.
It also takes the struct and inputs ALL the data into a single string in the form:

NAME - "MONTH STRING", DD. YYYY.

This program will accept names and corresponding dates until an invalid date is entered or nothing is entered for name or date.

I have watched all the variables, and stepped throuugh the program and have found that the program works right, for a single input.
Then to get the data printed you have to enter null data, or invalid data.
The program will overwrite the structure of good data the array WAS pointing to with the null data....as well as fill the new structure the array is NOW pointing to with the null data.

If this is confusing....ill post the code here as well as an example output

if i enter 5 dates, say

Ryan
2000 05 05
Bill
2000 06 06
Jim
2000 07 07
Tom
2000 08 08
Joe
2000 09 09
Then nothing (NULL)
and nothing

I get printed

- May, 5. 2000
- June, 6. 2000
- July, 7. 2000
- August, 8. 2000
- September, 9. 2000

OR alternately

Ryan
2000 05 05
Bill
2000 06 06
Jim
2000 07 07
Tom
2000 08 08
Joe
2000 13 09 (cant have 13th month)

I get printed

Joe - May, 5. 2000
Joe - June, 6. 2000
Joe - July, 7. 2000
Joe - August, 8. 2000
Joe - September, 9. 2000

/edit
oop, my code is set up to print
Joe - 9, September. 2000
no biggie
edit/

Link to code

 

[TheDiggler]

Yes, post your code if you want help figuring out your bug. Preferably place the code on an HTTP or FTP site somewhere. If you throw it up on an HTTP site, append a ".txt" file extension to the file so that a web browser knows to interpret the file as text. Of course if you do throw it up on an HTTP/FTP server, make sure you provide us w/ details on where to access the code. (Worst case scenario, post it in here. All the formatting will be lost though when viewing the code in these forums).

*EDIT* Ha, you posted a link to your code while I was typing this response! 😛

 

[TheDiggler]

Ok.. I haven't gone through your entire code yet; however, a preliminary scan shows the following serious bugs:

In make_date, you're using the wrong OR operator to check for "[condition A] OR [condition B]". You're using a SINGLE PIPE (|) for OR (which does a BITWISE OR check, not a LOGICAL OR that you want to check). You need to use a DOUBLE PIPE (||).

Furthermore, it's safer to put each "condition" within its own set of parenthesis.

Example:
if ( (day > 31) || (day < 1) )

BTW, I think your entire make_date function can be SIMPLIFIED as follows:

DATE* make_date(int year, int month, int day, char *name)
{
DATE *pdate = NULL;

if ( (strlen(name) > 0) &&
(day >= 1) && (day <= 31) &&
(month >= 1) && (month <= 12) &&
(year >= 1) )
{
pdate = (DATE*) malloc(sizeof(DATE));

if (pdate != NULL)
{
pdate->year = year;
pdate->month = month;
pdate->day = day;
pdate->name = strdup(name); /* strdup mallocs memory for and copies a string */

if (pdate->name == NULL)
{
printf("\nError allocating space for name!!!");
free(pdate);
pdate = NULL;
}
}
else printf ("\nError allocating space for DATE structure!!!");
}

return pdate;
}


Click Quote to see a FORMATTED VERSION of the above code.

BTW, in your original code, you had these logic errors in the make_date function:

1) pdate->name = (char*) malloc(sizeof(name));
[*]This allocates enough memory to store a POINTER. name is a char *, and sizeof(name) simply returns the size of a char * (which is a pointer).
[*] The correct way to allocate enough memory is:
pdate->name = (char *) malloc( strlen(name) + 1); /* where the "+ 1" is space for the NULL TERMINATOR

2) pdate->name = name;
[*]This doesn't COPY THE STRING. It simply assigns the pointer pdate->name to the memory location of name
[*]Assuming you did step #1 (above) correctly, the correct way to copy the contents is:
strcpy(pdate->name, name);

3) Note: Both steps 1 & 2 above were (in my recommended replacement make_date routine) replaced by a single call to strdup() (which does the malloc and strcpy in one call).

*EDIT* After further review, if you go w/ your original code but change the OR's to || (and put parenthesis around EACH SIDE of each OR CONDITION) and correct your "pdate->name = name" logic, your original code should function properly. Of course if you use the replacement make_date function I showed you, your code should work too.

 

[Krakerjak]

Very good.

Its working now. Thx for the assistance. 🙂

The main problem was the way i had used malloc for my name string
creating space for the pointer, taking that out and using the strdup() method was the solution. (didnt know that function yet)
I also changed the OR operator syntax.

You version of make_date is better, less code, more compact. I'll probably change that too.

You forgot yer brackets around pdate 😉 😛 😉 😛 😉

else printf ("\nError allocating space for DATE structure!!!");
}

return (pdate);
}

 

[TheDiggler]

You're welcome for the assistance, and parenthesis aren't required for the return command. Try it w/o 'em 😀.