Hello all,

I am trying to read a file into some allocated memory as part of a
program and I am running into a problem. The program gets a
segmentation fault during fread. I have previously used this code on
64-bit RHEL4 without any problems but I am not having this issue on 32-
bit RHEL5. I have simplified the code to it's most basic form and I
am still seeing the issue. Below is a full program that causes the
error. If anybody knows what may be causing the problem, would
be greatly appreciated....

#include <stdio.h>
#include <fcntl.h>

int main()
{
int data_size = 1024*1024;
u_char *data_buf;
FILE *data_file;
int data_num;

data_buf = (unsigned char *) valloc(data_size);
printf("Memory allocated.\n");
data_file = fopen("data_file","rb");
printf("File opened.");
data_num = fread(data_buf, sizeof(u_char), data_size, data_file);
printf("File read.");
return 0;
}

Thanks for your !

paragon.john@gmail.com wrote:
> I am trying to read a file into some allocated memory as part of a
> program and I am running into a problem. The program gets a
> segmentation fault during fread. I have previously used this code on
> 64-bit RHEL4 without any problems but I am not having this issue on 32-
> bit RHEL5. I have simplified the code to it's most basic form and I
> am still seeing the issue. Below is a full program that causes the
> error. If anybody knows what may be causing the problem, would
> be greatly appreciated....

> #include <stdio.h>
> #include <fcntl.h>

<fcntl.h> doesn't seem to be needed at all.

> int main()
> {
> int data_size = 1024*1024;

Why not make that a 'size_t' so that you can be sure that
the value actually fits in? An int doesn't need to be
able to hold more than 16 bits, which wouldn't be enough.

> u_char *data_buf;

What's wrong with plain old 'unsigned char'?

> FILE *data_file;
> int data_num;

Also this value should be a 'size_t', that's the type
that fread() returns.

> data_buf = (unsigned char *) valloc(data_size);

Rather likely the problem is here. First of all, valloc()
isn't a standard C function - and even the Linux man page
describes it as "obsolete", so why don't you use malloc()?

But the real problem is rather likely that there's no
declaration for the valloc() function in scope since you
didn't include <stdlib.h>. By using the cast in front of
valloc() you silenced the compiler but you didn't solve
the underlying problem: since there's no declaration the
compiler assumes that valloc() returns an int an will treat
the return value accordingly - it may store it somewhere
where an int but not a pointer fits (or, if you're on a
machine with dedicated data and address registers, the
return value gets passed back via a address register but
the caller, expecting an int, i.e. data, tries to pull it
from a data register). And this crippled or just random
value is then converted back into a pointer which doesn't
point to the memory that was allocated...

So never cast the return value of malloc() (and other
functions) unless you have a very good reason. While you
can silence the compiler that way you just keep it from
giving you valuable hints.

> printf("Memory allocated.\n");

You can't be sure since you didn't check the return value
of valloc().

> data_file = fopen("data_file","rb");
> printf("File opened.");

That's also something you can't be sure about since you
also don't test the return value of fopen().

> data_num = fread(data_buf, sizeof(u_char), data_size, data_file);
> printf("File read.");

And again it would make sense to check the return value of
fread().

> return 0;
> }
Regards, Jens
--
\ Jens Thoms Toerring ___ jt@toerring.de
\__________________________ http://toerring.de

Hi

On May 6, 8:09 pm, paragon.j...@gmail.com wrote:
> The program gets a segmentation fault during fread.
> #include <stdio.h>
> #include <fcntl.h>
>
> int main()
> {
> int data_size = 1024*1024

* sizeof(u_char);


> u_char *data_buf;
> FILE *data_file;
> int data_num;
>
> data_buf = (unsigned char *) valloc(data_size);

if( data_buf )

> printf("Memory allocated.\n");

else
printf("Memory NOT allocated.\n");

> data_file = fopen("data_file","rb");
> printf("File opened.");
> data_num = fread(data_buf, sizeof(u_char), data_size, data_file);
> printf("File read.");
> return 0;
> }

You need to include the size of whatever u_char is. I guess you think
that it is 1, but perhaps it isn't. You haven't included how you
typedef it.

You also must always check the return value of every function that can
fail. I've done the one that is most likely to cause segfault.

Also, valloc is documented as 'obsolete'. Use posix_memalign() if
your c library supports it.

HTH

viza

On May 6, 3:09 pm, paragon.j...@gmail.com wrote:
> Hello all,
>
> I am trying to read a file into some allocated memory as part of a
> program and I am running into a problem. The program gets a
> segmentation fault during fread. I have previously used this code on
> 64-bit RHEL4 without any problems but I am not having this issue on 32-
> bit RHEL5. I have simplified the code to it's most basic form and I
> am still seeing the issue. Below is a full program that causes the
> error. If anybody knows what may be causing the problem, would
> be greatly appreciated....
>
> #include <stdio.h>
> #include <fcntl.h>
>
> int main()
> {
> int data_size = 1024*1024;
> u_char *data_buf;
> FILE *data_file;
> int data_num;
>
> data_buf = (unsigned char *) valloc(data_size);
> printf("Memory allocated.\n");
> data_file = fopen("data_file","rb");
> printf("File opened.");
> data_num = fread(data_buf, sizeof(u_char), data_size, data_file);
> printf("File read.");
> return 0;
>
> }
>
> Thanks for your !

Thank you for the guys. I've figured it out. I feel dumb for
not checking the return values on the functions.

paragon.john@gmail.com wrote:
>
> I am trying to read a file into some allocated memory as part of a
> program and I am running into a problem. The program gets a
> segmentation fault during fread. I have previously used this code on
> 64-bit RHEL4 without any problems but I am not having this issue on 32-
> bit RHEL5. I have simplified the code to it's most basic form and I
> am still seeing the issue. Below is a full program that causes the
> error. If anybody knows what may be causing the problem, would
> be greatly appreciated....
>
> #include <stdio.h>
> #include <fcntl.h>

There is no such header file in standard C.
>
> int main()

Better would be "int main(void)

> {
> int data_size = 1024*1024;

There is no guarantee that an int can hold this number. Use a
long, or possibly a size_t.

> u_char *data_buf;

There is no such type as u_char.

> FILE *data_file;
> int data_num;
>
> data_buf = (unsigned char *) valloc(data_size);

There is no such function as valloc. If you meant malloc, don't
cast the result, which hides the error of failing to "#include
<stdlib.h>". The size parameter should be a size_t type.
> printf("Memory allocated.\n");

How do you know?

> data_file = fopen("data_file","rb");
> printf("File opened.");

How do you know?

> data_num = fread(data_buf, sizeof(u_char), data_size, data_file);
> printf("File read.");

How do you know?

> return 0;
> }

--
[mail]: Chuck F (cbfalconer at maineline dot net)
[page]: <http://cbfalconer.home.att.net>
Try the download section.


** Posted from http://www.teranews.com **

Jens Thoms Toerring wrote:
> paragon.john@gmail.com wrote:
> > #include <stdio.h>
> > #include <fcntl.h>
>
> <fcntl.h> doesn't seem to be needed at all.

It's not a standard C header either.

> > int main()
> > {
> > int data_size = 1024*1024;
>
> Why not make that a 'size_t' so that you can be sure that
> the value actually fits in?

There is no guarantee that 1048576 will fit into a size_t.

> An int doesn't need to be able to hold more than 16 bits,
> which wouldn't be enough.

Since 1024 is just as int, then 1024*1024 can overflow,
irrespective of what you assign it to.

> > data_buf = (unsigned char *) valloc(data_size);
>
> Rather likely the problem is here. First of all, valloc()
> isn't a standard C function - and even the Linux man page
> describes it as "obsolete", so why don't you use malloc()?
>
> But the real problem is rather likely that there's no
> declaration for the valloc() function in scope since you
> didn't include <stdlib.h>.

Note that a mere delcaration is not sufficient. You may
need a prototype, especially if you're passing an int
argument for a size_t parameter.

<snip>
> So never cast the return value of malloc() (and other
> functions) unless you have a very good reason.

Prevention is better than cure. Requiring prototypes to
be in scope is a better suggestion IMO. Sadly, the option
actually breaks implementation conformance, although
I find the cost to be well worth it.

<snip>

--
Peter