Velocity Reviews

Velocity Reviews (http://www.velocityreviews.com/forums/index.php)
-   C Programming (http://www.velocityreviews.com/forums/f42-c-programming.html)
-   -   SEGFAULT problem (http://www.velocityreviews.com/forums/t952442-segfault-problem.html)

difeiz@gmail.com 09-20-2012 04:13 AM

SEGFAULT problem
 
Greetings all,

I am working on a program and encountered a problem recently. The program is simplified (but the problem is still reproduceable):

Code:

#define _GNU_SOURCE
#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>

void
read_in_rates(const char *filename, double **rates)
{
int i, lines = 0;
char ch;

FILE *rates_file = fopen(filename, "r");
do {
ch = fgetc(rates_file);
if (ch == '\n') lines++;
} while (ch != EOF );

*rates = realloc(*rates, lines * sizeof(**rates));

rewind(rates_file);
for(i = 0; i < lines; i++) {
fscanf(rates_file, "%lf", &(*rates)[i]);
printf("%lf\n", *rates[i]);
}
}

int
main(void)
{
int i;
double *rates_1 = malloc(sizeof(*rates_1));

read_in_rates("./test", &rates_1);
for(i = 0; i <= 80; i++)
printf("%i\t%lf\n", i, rates_1[i]);

exit(EXIT_SUCCESS);
}

test file can be generated by:
~$ seq 1 100 > test

In main function I malloced a double pointer (rates_1) with a size of a double type, then I passed a pointer (&rates_1) to this double pointer (rates_1) to the function read_in_rates.

The read_in_rates function counted the lines in test file and trying to reallocate the pointer, access to the pointer (*rates[i]) will guarantee a segfault when i >=1 on my machine.

I'm running this program on linux on virtualbox, memory spaces are plenty, I have several other malloc and realloc for some structure types and they are working fine, could anyone point out where the problem is? Thanks in advance.

Cheers,
Difei

Barry Schwarz 09-20-2012 05:53 AM

Re: SEGFAULT problem
 
On Wed, 19 Sep 2012 21:13:39 -0700 (PDT), difeiz@gmail.com wrote:

>Greetings all,
>
> I am working on a program and encountered a problem recently. The program is simplified (but the problem is still reproduceable):
>
>
Code:

>#define _GNU_SOURCE
>#include <unistd.h>
>#include <stdlib.h>
>#include <stdio.h>
>
>void
>read_in_rates(const char *filename, double **rates)
>{
>    int i, lines = 0;
>    char ch;
>
>    FILE *rates_file = fopen(filename, "r");

Code:


Did fopen succeed?

>    do {
>        ch = fgetc(rates_file);


Is char signed or unsigned on your system?  fgetc returns an int.  You
should honor that interface.

>        if (ch == '\n') lines++;
>    } while (ch != EOF );
>
>    *rates = realloc(*rates, lines * sizeof(**rates));


Did realloc succeed?

>    rewind(rates_file);
>    for(i = 0; i < lines; i++) {
>        fscanf(rates_file, "%lf", &(*rates)[i]);
>        printf("%lf\n", *rates[i]);


What you have coded will be parsed as *(rates[i]).  When i is not 0,
rates[i] doesn't exist.  I think you want (*rates)[i].

>    }
>}
>
>int
>main(void)
>{
>    int i;
>    double *rates_1 = malloc(sizeof(*rates_1));


Did malloc succeed?

>
>    read_in_rates("./test", &rates_1);
>    for(i = 0; i <= 80; i++)
>        printf("%i\t%lf\n", i, rates_1[i]);


How do you know that read_in_rates processed at least 80 lines.  It
would be better for the function to return an int telling you how many
lines.

>
>    exit(EXIT_SUCCESS);
>}
>


>
>test file can be generated by:
>~$ seq 1 100 > test
>
>In main function I malloced a double pointer (rates_1) with a size of a double type, then I passed a pointer (&rates_1) to this double pointer (rates_1) to the function read_in_rates.


Better terminology is pointer to double. Many take the phrase "double
pointer" to mean type** for some type.

You passed the address of rates_1. This is a pointer value and not a
pointer object.

>The read_in_rates function counted the lines in test file and trying to reallocate the pointer, access to the pointer (*rates[i]) will guarantee a segfault when i >=1 on my machine.


As noted above, the object you are dereferencing does not exist.

>I'm running this program on linux on virtualbox, memory spaces are plenty, I have several other malloc and realloc for some structure types and they are working fine, could anyone point out where the problem is? Thanks in advance.


You need to look at operator precedence. See comment above.

--
Remove del for email

Ike Naar 09-20-2012 06:00 AM

Re: SEGFAULT problem
 
On 2012-09-20, difeiz@gmail.com <difeiz@gmail.com> wrote:
>
Code:

> #define _GNU_SOURCE
> #include <unistd.h>

Code:


The two lines above are unnecessary for this program.

> #include <stdlib.h>
> #include <stdio.h>
>
> void
> read_in_rates(const char *filename, double **rates)
> {
>    int i, lines = 0;
>    char ch;


ch is used to store the result of fgetc, which is an int
because fgetc can return EOF which might be outside the
range of a char. So use 'int ch;',

>    FILE *rates_file = fopen(filename, "r");


You should check if fopen succeeds (i.e. returns
a non-null pointer-to-FILE).

>    do {
>        ch = fgetc(rates_file);
>        if (ch == '\n') lines++;
>    } while (ch != EOF );
>
>    *rates = realloc(*rates, lines * sizeof(**rates));


You should check if realloc succeeds (i.e. returns a
non-null pointer).

>    rewind(rates_file);
>    for(i = 0; i < lines; i++) {
>        fscanf(rates_file, "%lf", &(*rates)[i]);
>        printf("%lf\n", *rates[i]);


*rates[i] parses as *(rates[i]), but what is intended
here is certainly (*rates)[i].

>    }


Forgot to fclose(rates_file);

> }
>
> int
> main(void)
> {
>    int i;
>    double *rates_1 = malloc(sizeof(*rates_1));


You should check if malloc succeeds (i.e. returns a
non-null pointer).

>    read_in_rates("./test", &rates_1);
>    for(i = 0; i <= 80; i++)
>        printf("%i\t%lf\n", i, rates_1[i]);


If the input file has less that 81 lines, the call
to read_in_rates will have resized rates_1 to less
than 81 elements, and you are accessing out-of-bounds
elements of rates_i in the printing loop.

>    exit(EXIT_SUCCESS);
> }
>


>
> test file can be generated by:
> ~$ seq 1 100 > test


This is very strange, test is used both for input and for output.
If the file is opened for output before it is opened for input
(which is most probably the case here) then the contents of the
file are lost before they are used.

difeiz@gmail.com 09-20-2012 06:06 AM

Re: SEGFAULT problem
 
Hi Barry,

The program is simplified for reading so rellocs and mallocs checking are removed, the "80" thing is just a placeholder, as you pointed out, the operator precedence caused the SEGFAULT problem, thank you very much for your help.

On Thursday, September 20, 2012 1:53:31 PM UTC+8, Barry Schwarz wrote:
> On Wed, 19 Sep 2012 21:13:39 -0700 (PDT), difeiz@gmail.com wrote:
>
>
>
> >Greetings all,

>
> >

>
> > I am working on a program and encountered a problem recently. The program is simplified (but the problem is still reproduceable):

>
> >

>
> >
Code:

>
> >#define _GNU_SOURCE

>
> >#include <unistd.h>

>
> >#include <stdlib.h>

>
> >#include <stdio.h>

>
> >

>
> >void

>
> >read_in_rates(const char *filename, double **rates)

>
> >{

>
> >    int i, lines = 0;

>
> >    char ch;

>
> >

>
> >    FILE *rates_file = fopen(filename, "r");

>
>
>
> Did fopen succeed?
>
>
>
> >    do {

>
> >        ch = fgetc(rates_file);

>
>
>
> Is char signed or unsigned on your system?  fgetc returns an int.  You
>
> should honor that interface.
>
>
>
> >        if (ch == '\n') lines++;

>
> >    } while (ch != EOF );

>
> >

>
> >    *rates = realloc(*rates, lines * sizeof(**rates));

>
>
>
> Did realloc succeed?
>
>
>
> >    rewind(rates_file);

>
> >    for(i = 0; i < lines; i++) {

>
> >        fscanf(rates_file, "%lf", &(*rates)[i]);

>
> >        printf("%lf\n", *rates[i]);

>
>
>
> What you have coded will be parsed as *(rates[i]).  When i is not 0,
>
> rates[i] doesn't exist.  I think you want (*rates)[i].
>
>
>
> >    }

>
> >}

>
> >

>
> >int

>
> >main(void)

>
> >{

>
> >    int i;

>
> >    double *rates_1 = malloc(sizeof(*rates_1));

>
>
>
> Did malloc succeed?
>
>
>
> >

>
> >    read_in_rates("./test", &rates_1);

>
> >    for(i = 0; i <= 80; i++)

>
> >        printf("%i\t%lf\n", i, rates_1[i]);

>
>
>
> How do you know that read_in_rates processed at least 80 lines.  It
>
> would be better for the function to return an int telling you how many
>
> lines.
>
>
>
> >

>
> >    exit(EXIT_SUCCESS);

>
> >}

>
> >


>
> >

>
> >test file can be generated by:

>
> >~$ seq 1 100 > test

>
> >

>
> >In main function I malloced a double pointer (rates_1) with a size of a double type, then I passed a pointer (&rates_1) to this double pointer (rates_1) to the function read_in_rates.

>
>
>
> Better terminology is pointer to double. Many take the phrase "double
>
> pointer" to mean type** for some type.
>
>
>
> You passed the address of rates_1. This is a pointer value and not a
>
> pointer object.
>
>
>
> >The read_in_rates function counted the lines in test file and trying to reallocate the pointer, access to the pointer (*rates[i]) will guarantee a segfault when i >=1 on my machine.

>
>
>
> As noted above, the object you are dereferencing does not exist.
>
>
>
> >I'm running this program on linux on virtualbox, memory spaces are plenty, I have several other malloc and realloc for some structure types and they are working fine, could anyone point out where the problem is? Thanks in advance.

>
>
>
> You need to look at operator precedence. See comment above.
>
>
>
> --
>
> Remove del for email


Ike Naar 09-20-2012 06:12 AM

Re: SEGFAULT problem
 
On 2012-09-20, Ike Naar <ike@sverige.freeshell.org> wrote:
> On 2012-09-20, difeiz@gmail.com <difeiz@gmail.com> wrote:
>> ~$ seq 1 100 > test

>
> This is very strange, test is used both for input and for output.
> If the file is opened for output before it is opened for input
> (which is most probably the case here) then the contents of the
> file are lost before they are used.


Sorry, forget this one, I was confused about the 'seq 1 100 > test'
command and thought it was a run of the program you showed.
It appears, however, that 'seq' is a Unix utility.

Ian Collins 09-20-2012 07:48 AM

Re: SEGFAULT problem
 
On 09/20/12 04:13 PM, difeiz@gmail.com wrote:
> Greetings all,
>
> I am working on a program and encountered a problem recently. The program is simplified (but the problem is still reproduceable):
>
> [code]
> #define _GNU_SOURCE


Why?

> #include<unistd.h>
> #include<stdlib.h>
> #include<stdio.h>
>
> void
> read_in_rates(const char *filename, double **rates)
> {
> int i, lines = 0;
> char ch;
>
> FILE *rates_file = fopen(filename, "r");
> do {
> ch = fgetc(rates_file);
> if (ch == '\n') lines++;
> } while (ch != EOF );
>
> *rates = realloc(*rates, lines * sizeof(**rates));
>
> rewind(rates_file);
> for(i = 0; i< lines; i++) {
> fscanf(rates_file, "%lf",&(*rates)[i]);
> printf("%lf\n", *rates[i]);


This is probably your problem, you are indexing then dereferencing
rather than dereferencing then indexing. See the line above for the
correct form.

--
Ian Collins


All times are GMT. The time now is 10:36 AM.

Powered by vBulletin®. Copyright ©2000 - 2014, vBulletin Solutions, Inc.
SEO by vBSEO ©2010, Crawlability, Inc.