Issue with memory allocation and file reading

Discussion in 'C Programming' started by paragon.john@gmail.com, May 6, 2008.

  1. Guest

    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, help 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 help!
     
    , May 6, 2008
    #1
    1. Advertising

  2. 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, help 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 ___
    \__________________________ http://toerring.de
     
    Jens Thoms Toerring, May 6, 2008
    #2
    1. Advertising

  3. viza Guest

    Hi

    On May 6, 8:09 pm, 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
     
    viza, May 6, 2008
    #3
  4. Guest

    On May 6, 3:09 pm, 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, help 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 help!


    Thank you for the help guys. I've figured it out. I feel dumb for
    not checking the return values on the functions.
     
    , May 6, 2008
    #4
  5. Jens Thoms Toerring wrote:
    > 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
     
    Peter Nilsson, May 7, 2008
    #5
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. s.subbarayan

    Dynamic memory allocation and memory leak...

    s.subbarayan, Mar 18, 2005, in forum: C Programming
    Replies:
    10
    Views:
    754
    Eric Sosman
    Mar 22, 2005
  2. Rodrigo Dominguez

    memory allocation and freeing memory

    Rodrigo Dominguez, Jun 13, 2005, in forum: C Programming
    Replies:
    11
    Views:
    632
    Jean-Claude Arbaut
    Jun 15, 2005
  3. Henk
    Replies:
    4
    Views:
    867
  4. Ken
    Replies:
    24
    Views:
    3,942
    Ben Bacarisse
    Nov 30, 2006
  5. chris
    Replies:
    6
    Views:
    1,031
    chris
    Oct 28, 2005
Loading...

Share This Page