Linked List simple problem

[Valerio Daelli]

Hello everybody
I have a problem with a linked list.
I just get segmentation faults.
Could anyone please help me?
Thanks, this is the code
__________________________


#include <stdio.h>
#include <stdlib.h>
#include <iostream.h>
using namespace std;

struct TCell {
char* NumeroTel;
TCell* Next;
};

struct TList {
unsigned Size;
TCell* First;
};

int main()
{
TList List;
List.Size=0;
int FloatToRead;
char* testo[100];
char* buffer;
int nbytes=100;
int i=0;
int a=0;
FILE *filePtr;
filePtr = fopen("input","r");
buffer=(char *)malloc((nbytes+1)*(sizeof(char)));
while (fgets(buffer,nbytes,filePtr) != NULL)
{
TCell* Temp=List.First;
List.First=new TCell;
List.First->NumeroTel=(char *)malloc(sizeof(buffer)+1);
sprintf(List.First->NumeroTel,buffer);
i++;
printf("Letti %d valori: %s - %s",i,buffer,List.First->NumeroTel);
List.First->Next=Temp;
++List.Size;
}
fclose(filePtr);
for(int j=0;j<List.Size;++j) {
printf("Valore: %s \n",List.First->NumeroTel);
TCell* Temp=List.First;
List.First=List.First->Next;
cout << "Distruzione della cella..." << endl;
delete Temp;
}
}

 

[Mike Wahler]

Hello everybody
I have a problem with a linked list.
I just get segmentation faults.
Could anyone please help me?
Thanks, this is the code
__________________________


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

There's no such header as <iostream.h> in C.
That's an old prestandard C++ header. C++
is not topical here.

using namespace std;

Another C++ construct. You need to decide whether
you're writing C or C++.

struct TCell {
char* NumeroTel;
TCell* Next;
};

struct TList {
unsigned Size;
TCell* First;
};

int main()
{
TList List;
List.Size=0;
int FloatToRead;
char* testo[100];
char* buffer;
int nbytes=100;
int i=0;
int a=0;
FILE *filePtr;
filePtr = fopen("input","r");
buffer=(char *)malloc((nbytes+1)*(sizeof(char)));

1. Don't cast the return value from 'malloc()'
2. sizeof(char) is one by definition.

while (fgets(buffer,nbytes,filePtr) != NULL)
{
TCell* Temp=List.First;
List.First=new TCell;
List.First->NumeroTel=(char *)malloc(sizeof(buffer)+1);

sizeof(buffer) isn't what you probably think it is.
Print it out or look at it in a debugger

sprintf(List.First->NumeroTel,buffer);

Because of the above 'sizeof' issue, 'sprintf()' is very likely
overflowing the memory pointed to by 'List.First->NumeroTel',
which would give undefined behavior, possibly manifested as a
'seg fault'.

i++;
printf("Letti %d valori: %s - %s",i,buffer,List.First->NumeroTel);
List.First->Next=Temp;
++List.Size;
}
fclose(filePtr);
for(int j=0;j<List.Size;++j) {
printf("Valore: %s \n",List.First->NumeroTel);
TCell* Temp=List.First;
List.First=List.First->Next;
cout << "Distruzione della cella..." << endl;
delete Temp;
}
}

I didn't try to analyze your logic, so I won't comment
on it.

-Mike

 

[Dave Vandervies]

Hello everybody
I have a problem with a linked list.
I just get segmentation faults.
Could anyone please help me?
Thanks, this is the code
__________________________

[With much snippage --DV]

#include <iostream.h>

This doesn't look like a C header.
comp.lang.c++ is <---thataway---, but they'll tell you it's not a C++
header either.
Best to decide which language you're using before you start writing code,
and ask questions about it in an appropriate place.

struct TList {
unsigned Size;
TCell* First;
};

int main()
{
TList List;
List.Size=0;

[Other declarations and initializations, but this is all that involves
List. --DV]

buffer=(char *)malloc((nbytes+1)*(sizeof(char)));

This is bad code in both of the languages you look like you might be
trying to use. The C people will tell you not to cast the pointer you
get from malloc, and the C++ people will tell you to use new instead.
(Actually, in this case they'll probably tell you to use a std::string
instead of allocating your own memory, but the principle still holds.)

while (fgets(buffer,nbytes,filePtr) != NULL)
{
TCell* Temp=List.First;

What's in List.first the first time the loop runs? (Hint: Probably not
what you expect.)

List.First=new TCell;
List.First->NumeroTel=(char *)malloc(sizeof(buffer)+1);
sprintf(List.First->NumeroTel,buffer);

sizeof(buffer) is the size of the pointer, not the size of what it's
pointing at. I suspect this may be what's causing your segfault, but
there are enough other problems with your code that I can't be sure.

List.First->Next=Temp;
++List.Size;
}

for(int j=0;j<List.Size;++j) {
TCell* Temp=List.First;
List.First=List.First->Next;

delete Temp;

You're leaking the memory you malloc'd when you were building the
list here.
(You do actually manage to avoid following the bad pointer from the last
link in the list, but that doesn't look intentional.)

}
}

dave

 

[m]

Hello everybody
I have a problem with a linked list.
I just get segmentation faults.
Could anyone please help me?
Thanks, this is the code
__________________________


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

There's no such header as <iostream.h> in C.
That's an old prestandard C++ header. C++
is not topical here.

using namespace std;

Another C++ construct. You need to decide whether
you're writing C or C++.

struct TCell {
char* NumeroTel;
TCell* Next;
};

struct TList {
unsigned Size;
TCell* First;
};

int main()
{
TList List;
List.Size=0;
int FloatToRead;
char* testo[100];
char* buffer;
int nbytes=100;
int i=0;
int a=0;
FILE *filePtr;
filePtr = fopen("input","r");
buffer=(char *)malloc((nbytes+1)*(sizeof(char)));

1. Don't cast the return value from 'malloc()'

These casts are not necessary under an ANSI C compiler (because malloc
returns void * which the compiler converts automatically). Are there non
ANSI-C compilers around? If there arent then theres no need to typecast,
else there is rt?

 

[Dave Vandervies]

A rather long article, quoted in its entirety with one paragraph of new
material in the middle.

Please learn to snip. (At least you didn't top-post.)

Mike Wahler wrote:

These casts are not necessary under an ANSI C compiler (because malloc
returns void * which the compiler converts automatically). Are there non
ANSI-C compilers around? If there arent then theres no need to typecast,
else there is rt?

You are unlikely to want to compile new code with a pre-ANSI compiler,
and if you do want to do such a thing you'll need to worry about more
than just whether you're casting a void *. Therefore, you need another
good reason to want to cast a void * to another pointer type in C code.
(Such reasons do exist, but they're Very Rare. You probably don't
have one. If you have to ask, you almost definitely don't have one.)

The OP most likely wasn't casting it for this reason; confusion about
what language he was using is more likely.


dave

 

[m]

You are unlikely to want to compile new code with a pre-ANSI compiler,
and if you do want to do such a thing you'll need to worry about more
than just whether you're casting a void *. Therefore, you need another
good reason to want to cast a void * to another pointer type in C code.
(Such reasons do exist, but they're Very Rare. You probably don't
have one. If you have to ask, you almost definitely don't have one.)

my qustion was are there non-ANSI c compilers around. not if op is
working on a pre-ANSI C compiler. i am new to C 2, so I will appreciate
this answer.

 

[Mike Wahler]

my qustion was are there non-ANSI c compilers around.

Sure there are (as a matter fact, I own a few) -- C is much
older than the first ANSI standard. The vendors of most/all
C compilers that do (more or less) conform to the standard will
state this in their advertising and packaging.

not if op is
working on a pre-ANSI C compiler.
i am new to C 2, so I will appreciate
this answer.

I recommend you learn standard C (aka 'ISO C' or 'ANSI C').
Standard C does not require a cast for conversion from 'void*'
to any other (object) pointer type. A C compiler that requires
such a cast is not conforming (note that this is one of the
many ways in which C and C++ differ) -- perhaps OP was actually
using a C++ compiler, or maybe he wrote the cast from ignorance,
I don't know.


-Mike

 

[Lawrence Kirby]

my qustion was are there non-ANSI c compilers around.

Sure there are. But there is very rarely a good reason to use une these
days, especially for new code.

not if op is
working on a pre-ANSI C compiler. i am new to C 2, so I will appreciate
this answer.

In comp.lang.c the assumption is that you are using a standard C compiler
unless explicitly stated otherwise.

Lawrence

 

[becker]

Hi, how are you?
I haven't really looked at your code closely, but
is there any reason to why you use 'new' inside of the C code?
Have a good day.

 

[Mark McIntyre]

Hi, how are you?
I haven't really looked at your code closely, but
is there any reason to why you use 'new' inside of the C code?

because he's writing C++ (to an old standard as well), as these snippets
show. Hence he's posting in the wrong group...