Linked list's asignment help..

[Simon Mansfield]

Im trying to read in a list of employee's from a file, in the format:

01:SJMrogrammer:481 (<- £4.81 an hr)
02:AMGython Scripter:512
:
:
etc.

When assigning to the "new" pointer it brings up an assignment error on line
33 and 34 of Staff.c, now these are the two string fields.. But I dont see
why it is doing it!? Anyone?

So I have created this file Staff.c:

#include "main.h"

struct employee {
int pid;
char name[3];
char job[40];
int hourlyRate;
char *appointments[7];
struct employee *next;
};

PTR read_in_staff() {

// Create head and new pointers.
PTR head = NULL;
PTR new = NULL;

FILE *input; // File to read employee's from.
char filename[40]; // String to store file path.
PERSON store; // A PERSON struct to store the read in data.

// Prompt user for file path.
printf("\nPlease enter the path of the input file: ");
gets(filename);

if((input = fopen(filename, "r")) != NULL) { //Opens the file in read
mode.
while(feof(input) == 0) { // While the end of file has not been
reached.
fscanf(input, "%d:%s:%s:%d", store.pid, store.name, store.job,
store.hourlyRate); // Get the employee fields and put in "store" struct.
new = (PTR)malloc(sizeof(PERSON)); // Create an instance of struct
PERSON and allocate space.
new->next = head; // Make the new node link to the first node.
head = new; // Make head now point to the new element.
new->pid = store.pid; // Fill values with data.
new->name = store.name;
new->job = store.job;
new->hourlyRate = store.hourlyRate;
}
} else {
printf("Error opening this file"); // Error produced on incorrect file
entry.
}
return head;
}


And a header file Staff.h:

/* staff.h - Header file for staff.c */

#ifndef _STAFF_H
#define _STAFF_H 1

/* The function prototypes */

typedef struct employee PERSON;
typedef PERSON *PTR;

PTR read_in_staff();

#endif

 

[Richard Bos]

#include "main.h"

It would be nice to have this "main.h", especially since...

struct employee {
int pid;
char name[3];
char job[40];
int hourlyRate;
char *appointments[7];
struct employee *next;
};

PTR read_in_staff() {

....we have no idea what a PTR is.

// Create head and new pointers.
PTR head = NULL;
PTR new = NULL;

FILE *input; // File to read employee's from.

Read the employee's _what_? Or do you perhaps mean "employees"?

char filename[40]; // String to store file path.
PERSON store; // A PERSON struct to store the read in data.

Your indentation is iffy. You may run into trouble with that some day,
where you think something belongs to a different level than it really
does.

// Prompt user for file path.

A pretty obvious comment, isn't it? It's not a good idea to write
comments that don't tell one anything new, since after a few pages of
this, one starts skipping the comments entirely as being not worth
reading.

printf("\nPlease enter the path of the input file: ");
gets(filename);

Never, ever, _EVER_ use gets(). It is a buffer overflow error waiting to
happen, a backdoor in any program that uses it. And the worst is, it
cannot be repaired. There is no way to stop gets() from clobbering over
the end of your array when you enter more than, in this case, 40 chars.
Use fgets() instead.
Or, if you want to use a cookbook solution without understanding what it
really does or having something flexible enough to deal with varying-
length data, use scanf(). Clearly I don't recommend that, either; but it
can be made to serve by the real SM experts. No, your best bet would be
fgets().

if((input = fopen(filename, "r")) != NULL) { //Opens the file in read
mode.

Oops... your comment has leaked. It's not wise to use // comments in a
usenet post.

while(feof(input) == 0) { // While the end of file has not been
reached.

This is a bad way of reading a file. Read the FAQ:

fscanf(input, "%d:%s:%s:%d", store.pid, store.name, store.job,
store.hourlyRate); // Get the employee fields and put in "store" struct.

This is not the right way to use scanf() with integers. You need
pointers for those.
Neither is it the right way to read strings. You won't need a pointer in
that case, but you do need to limit the length of your input fields to
the length of your variables (good luck if those are of varying length,
btw), especially since your name field is only three chars long.
And I hope you realise that you're going to have trouble entering a job
description of "Assistant sales manager", since scanf("%s") stops at the
first whitespace...

new = (PTR)malloc(sizeof(PERSON)); // Create an instance of struct
PERSON and allocate space.

Don't cast malloc(). It is completely unnecessary if you have a proper
declaration of malloc() in scope; it hides the error of _not_ having a
proper declaration of malloc() in scope; and worst, it degrades the
warning value of all casts, even the necessary ones.
OTOH, _do_ #include <stdlib.h> if you use malloc(), and <stdio.h> if you
use printf(), fgets(), etcetera. Calling most library functions without
a declaration causes undefined behaviour, and including the correct
headers is the easiest way to get those declarations. BTW, since it
seems that your compiler passed this by completely for printf(), you
want to jack up its warning level.

new->next = head; // Make the new node link to the first node.
head = new; // Make head now point to the new element.
new->pid = store.pid; // Fill values with data.
new->name = store.name;
new->job = store.job;

You cannot assign arrays like that.

I think it would be a very good idea if you went back and made sure you
understand the basics of pointers, arrays, and the difference between
them, before you attempt to use them in a larger program.

new->hourlyRate = store.hourlyRate;
}
} else {
printf("Error opening this file"); // Error produced on incorrect file
entry.
}
return head;
}

And a header file Staff.h:

Which you never #include in the code you show, so what's it got to do
with anything?

/* staff.h - Header file for staff.c */

#ifndef _STAFF_H

This identifier is not in your namespace. It is reserved for use by the
implementation; any use by you invokes undefined behaviour.

Richard

 

[Simon Mansfield]

Ok, first off, thank you very much for all your comments, I will try to
consider as many as possible. But, would it be possible to give an example
of how one might implement somthing which does the same thing as I am trying
to do in the (sloppy) code below? Because the first 3 lines compile fine..
However its only the strings that do not..

You cannot assign arrays like that.

Also, on these lines you say that I should use pointers instead of normal
variables.. And that "%s" will end the string at the first whitespace... Is
there anyway to get round this?? And what should it look like?

fscanf(input, "%d:%s:%s:%d", store.pid, store.name, store.job,
store.hourlyRate); // Get the employee fields and put in "store" struct.

Thanks again,

Simon

 

[CBFalconer]

Im trying to read in a list of employee's from a file, in the
format:

01:SJMrogrammer:481 (<- £4.81 an hr)
02:AMGython Scripter:512
:
:
etc.

When assigning to the "new" pointer it brings up an assignment
error on line 33 and 34 of Staff.c, now these are the two string
fields.. But I dont see why it is doing it!? Anyone?

So I have created this file Staff.c:

Try again. First, format your program properly, and ensure it is
complete and compilable. Do not use // comments, they wrap around
and convert compilable source into nonsense. Mark the lines 33 and
34 (or whatever) so we can tell where the error occurs, and give
the full error message. Do not use any non-standard features.
Limit your line length to 72, or even better 65.

If you make it relatively easy to examine your code, we are much
more likely to do so.

 

[Michael Mair]

Ok, first off, thank you very much for all your comments, I will try to
consider as many as possible. But, would it be possible to give an example
of how one might implement somthing which does the same thing as I am trying
to do in the (sloppy) code below? Because the first 3 lines compile fine..
However its only the strings that do not..

As Richard already told you: Read up on certain topics.
Read the sections 4 to 6 (especially 6) in the comp.lang.c
FAQ ( http://www.eskimo.com/~scs/C-faq/top.html ) to understand
the issues at hand. The short of it is: Arrays cannot be assigned
in one go (exception: initialization at declaration).

Another thing: Do not top-post. Put your answer _after_ the
stuff you are referring to.
Then you would have seen that Richard writes the same thing
as I do.
And one more: You snipped the attributions. This is considered
_very_ rude. Here we go:


In this vein: http://learn.to/quote

Yep.

[Corrected order:]

Also, on these lines you say that I should use pointers instead of normal
variables.. And that "%s" will end the string at the first whitespace... Is
there anyway to get round this?? And what should it look like?

Yes. Have a look at the documentation of fscanf and look for
scansets. Or google for "scanset" in the group comp.lang.c
restricted to this year.


-Michael

 

[Richard Bos]

Ok, first off, thank you very much for all your comments, I will try to
consider as many as possible. But, would it be possible to give an example
of how one might implement somthing which does the same thing as I am trying
to do in the (sloppy) code below? Because the first 3 lines compile fine..
However its only the strings that do not..

So RTFM. This is very basic array handling indeed.

Also, on these lines you say that I should use pointers instead of normal
variables..

No, I don't. One, pointers _are_ normal variables. Two, the types you
use in these lines are fine (arrays of char), but what you do with them
is not (direct assignment). Again, RTFM. Three, I never told you to use
anything _here_.

And that "%s" will end the string at the first whitespace...

In the scanf() family, yes.

Is there anyway to get round this?? And what should it look like?

There are two ways to get around this:
- use the [] field specifier, which is inflexible and looks like line
noise (a.k.a. Perl)
- use a function which is designed for, and suitable for, random string
input from human sources. Such as fgets().
As I told you in my previous post, in fact.

Do not expect to be spoon-fed. I will not help you become a mediocre
programmer with no real understanding. Engage your own brain.

Richard