Segmentation fault

S

SP

The following code compiles with no errors, but fails with
"Segmentation Fault" when I run it .
I tracked the problem down to the code which populates the
image.pixel_p array and I assume that the real problem is with the
memory allocation.

The values for the rows and columns are correct and I print them to
make sure.
I added a print statement to the code that is crashing and it gets
about 90% through the array before it fails.

Thanks in advance for your help.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "fgetdata.h"


int main( int argc, char *argv[])
{

struct png_file{
char *name_p; /* pointer to name of image file */
char *type_p; /* image type identifier */
int max_row; /* total rows of image */
int max_col; /* total columns of image */
int max_val; /* max pixel value of image */
int **pixel_p; /* pointer to 2D array space to store image */
};
struct png_file image;

FILE *image_fp;
int i, args, success;
int row, col;
char *delimiters =" \t\r\n\f\v\a\b\\?\\\'\"!%^&*()=+/<>,.|[]{}#~";
char *line = NULL;
size_t size = 0;

image_fp = fopen("chess.pgm", "r");
if(!image_fp)
{
fprintf(stderr, "unable to open file\n");
return 1;
}

/* get header info, skip comment lines */
i = 0;
success = 0;
while(i<3)
{
fgetline(&line, &size, (size_t)-1, image_fp, 0);
if(line[0] != '#')
{
if(i == 0)
{
image.type_p = malloc(strlen(line)+1 *
sizeof(image.type_p));
if(image.type_p)
{
strcpy(image.type_p, line);
}
else
{
fprintf(stderr, "Mem allocation for 'magic_str_p'
failed\n");
return 1;
}
success = 1;
}
if(i == 1)
{
args = sscanf(line ,"%d %d", &image.max_row,
&image.max_col);
if ( args == 2) success = 1;
}
if(i == 2)
{
args = sscanf(line ,"%d", &image.max_val );
if ( args == 1) success = 1;
}
if(success) i++;
success = 0;
}
}
fprintf(stderr, "Image Type: %s\n", image.type_p);
fprintf(stderr, "Image Rows x Columns: %i x %i\n", image.max_row,
image.max_col);
fprintf(stderr, "Pixel Max Value: %i\n", image.max_val);

image.pixel_p = malloc(sizeof(image.pixel_p) * image.max_row) ;
if( image.pixel_p == NULL)
{
fprintf(stderr, "Memory allocation failed\n");
return EXIT_FAILURE;
}
for( row = 0; row <= image.max_col; row++)
{
image.pixel_p[row] = malloc(sizeof(image.pixel_p) *
image.max_col);
if( image.pixel_p == NULL)
{
fprintf(stderr, "Memory allocation failed\n");
return EXIT_FAILURE;
}
}

if(image.pixel_p)
{
for( row=0; row < image.max_row; row++)
{
for( col=0; col<image.max_col; col++)
{
fgetword(&line, &size, delimiters, (size_t)-1,
image_fp, 0);
image.pixel_p[row][col] = atoi(line);
fprintf(stderr, "pixel[%i][%i] word:%s
Val:%i\n", row, col, line, image.pixel_p[row][col]);
}
}
}

fclose(image_fp);
free(image.type_p);
for( row = 0; row < image.max_col; row++)
{
free(image.pixel_p[row]);
}
free(image.pixel_p);
return EXIT_SUCCESS;
}
 
A

Andrew Poelstra

SP said:
The following code compiles with no errors, but fails with
"Segmentation Fault" when I run it .
I tracked the problem down to the code which populates the
image.pixel_p array and I assume that the real problem is with the
memory allocation.

The values for the rows and columns are correct and I print them to
make sure.
I added a print statement to the code that is crashing and it gets
about 90% through the array before it fails.

Thanks in advance for your help.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "fgetdata.h"
Click to expand...

I don't believe that you've given us the contents of fgetdata.h. Is
it possible that the problem lies in there?
int main( int argc, char *argv[])
{

struct png_file{
char *name_p; /* pointer to name of image file */
char *type_p; /* image type identifier */
int max_row; /* total rows of image */
int max_col; /* total columns of image */
int max_val; /* max pixel value of image */
int **pixel_p; /* pointer to 2D array space to store image */
};
Click to expand...

You should give your struct definitions file scope; it clutters main()
and can cause funny problems if you don't.
struct png_file image;

FILE *image_fp;
int i, args, success;
int row, col;
char *delimiters =" \t\r\n\f\v\a\b\\?\\\'\"!%^&*()=+/<>,.|[]{}#~";
Click to expand...

You have "\\" in there twice, just so you know.
char *line = NULL;
size_t size = 0;

image_fp = fopen("chess.pgm", "r");
if(!image_fp)
{
fprintf(stderr, "unable to open file\n");
return 1;
Click to expand...

This is not portable:
return EXIT_FAILURE;
would be better.
}

/* get header info, skip comment lines */
i = 0;
success = 0;
while(i<3)
{
fgetline(&line, &size, (size_t)-1, image_fp, 0);
Click to expand...

What does this function do? Since you didn't post it, I can only guess
at what problems lie within. If it's getting input from outside the
program, it must be fallable. Do you check for this anywhere? Since
you pass the address of a NULL pointer, what makes you think that the
pointer won't be NULL when it comes back out? If so,
if(line[0] != '#')
Click to expand...

this is your problem.

[remaining code snipped]
 
M

M.B

SP said:
The following code compiles with no errors, but fails with
"Segmentation Fault" when I run it .
I tracked the problem down to the code which populates the
image.pixel_p array and I assume that the real problem is with the
memory allocation.

The values for the rows and columns are correct and I print them to
make sure.
I added a print statement to the code that is crashing and it gets
about 90% through the array before it fails.

Thanks in advance for your help.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "fgetdata.h"


int main( int argc, char *argv[])
{

struct png_file{
char *name_p; /* pointer to name of image file */
char *type_p; /* image type identifier */
int max_row; /* total rows of image */
int max_col; /* total columns of image */
int max_val; /* max pixel value of image */
int **pixel_p; /* pointer to 2D array space to store image */
};
struct png_file image;

FILE *image_fp;
int i, args, success;
int row, col;
char *delimiters =" \t\r\n\f\v\a\b\\?\\\'\"!%^&*()=+/<>,.|[]{}#~";
char *line = NULL;
size_t size = 0;

image_fp = fopen("chess.pgm", "r");
if(!image_fp)
{
fprintf(stderr, "unable to open file\n");
return 1;
}

/* get header info, skip comment lines */
i = 0;
success = 0;
while(i<3)
{
fgetline(&line, &size, (size_t)-1, image_fp, 0);
if(line[0] != '#')
{
if(i == 0)
{
image.type_p = malloc(strlen(line)+1 *
sizeof(image.type_p));
if(image.type_p)
{
strcpy(image.type_p, line);
}
else
{
fprintf(stderr, "Mem allocation for 'magic_str_p'
failed\n");
return 1;
}
success = 1;
}
if(i == 1)
{
args = sscanf(line ,"%d %d", &image.max_row,
&image.max_col);
if ( args == 2) success = 1;
}
if(i == 2)
{
args = sscanf(line ,"%d", &image.max_val );
if ( args == 1) success = 1;
}
if(success) i++;
success = 0;
}
}
fprintf(stderr, "Image Type: %s\n", image.type_p);
fprintf(stderr, "Image Rows x Columns: %i x %i\n", image.max_row,
image.max_col);
fprintf(stderr, "Pixel Max Value: %i\n", image.max_val);

image.pixel_p = malloc(sizeof(image.pixel_p) * image.max_row) ;
Click to expand...

u allocate image.pixel_p as array of image.max_row of type
"sizeof(image.pixel_p" !!!!!!!???
strange .
sizeof(image.pixel_p is a pointer and in most machines sizeof lone

chack this

if( image.pixel_p == NULL)
{
fprintf(stderr, "Memory allocation failed\n");
return EXIT_FAILURE;
}
for( row = 0; row <= image.max_col; row++)
Click to expand...


one error here
i allocated max_rows (you allocated max_cpls u remeber)
cjeck this
pls change this line to
fand why that <= i should be <

{
image.pixel_p[row] = malloc(sizeof(image.pixel_p) *
image.max_col);
if( image.pixel_p == NULL)
{
fprintf(stderr, "Memory allocation failed\n");
return EXIT_FAILURE;
}
}

if(image.pixel_p)
{
for( row=0; row < image.max_row; row++)
{
for( col=0; col<image.max_col; col++)
{
fgetword(&line, &size, delimiters, (size_t)-1,
image_fp, 0);
image.pixel_p[row][col] = atoi(line);
fprintf(stderr, "pixel[%i][%i] word:%s
Val:%i\n", row, col, line, image.pixel_p[row][col]);
}
}
}

fclose(image_fp);
free(image.type_p);
for( row = 0; row < image.max_col; row++)
{
free(image.pixel_p[row]);
}
free(image.pixel_p);
return EXIT_SUCCESS;
}
Click to expand...
 
V

valis.eric.ykchan

/* Compare row to max_col? */
for( row = 0; row <= image.max_col; row++)
{
image.pixel_p[row] = malloc(sizeof(image.pixel_p) *
image.max_col);
if( image.pixel_p == NULL)
{
fprintf(stderr, "Memory allocation failed\n");
return EXIT_FAILURE;
}
}

if(image.pixel_p)
{
/* Now you compare row to max_row? */
for( row=0; row < image.max_row; row++)
{
for( col=0; col<image.max_col; col++)
{
fgetword(&line, &size, delimiters, (size_t)-1,
image_fp, 0);
image.pixel_p[row][col] = atoi(line);
fprintf(stderr, "pixel[%i][%i] word:%s
Val:%i\n", row, col, line, image.pixel_p[row][col]);
}
}
}
 
S

SP

/* Compare row to max_col? */
for( row = 0; row <= image.max_col; row++)
{
image.pixel_p[row] = malloc(sizeof(image.pixel_p) *
image.max_col);
if( image.pixel_p == NULL)
{
fprintf(stderr, "Memory allocation failed\n");
return EXIT_FAILURE;
}
}

if(image.pixel_p)
{
/* Now you compare row to max_row? */
for( row=0; row < image.max_row; row++)
{
for( col=0; col<image.max_col; col++)
{
fgetword(&line, &size, delimiters, (size_t)-1,
image_fp, 0);
image.pixel_p[row][col] = atoi(line);
fprintf(stderr, "pixel[%i][%i] word:%s
Val:%i\n", row, col, line, image.pixel_p[row][col]);
}
}
}
Click to expand...

Thanks to all for the help.

There were 2 problems
/* Compare row to max_col? */
for( row = 0; row <= image.max_col; row++)
{
image.pixel_p[row] = malloc(sizeof(image.pixel_p) *
image.max_col);
if( image.pixel_p == NULL)
{
fprintf(stderr, "Memory allocation failed\n");
return EXIT_FAILURE;
}
}
Click to expand...

I was comparing row to image.max_col and it should have been row
compared to image.max_row, as Eric pointed out

and

The "for" loop was using "<=" and it should have been "<" as M.B.
pointed out

I made these two changes and now it works as expected.

Thanks for all the help.
 
C

Chris Torek

The following code compiles with no errors, but fails with
"Segmentation Fault" when I run it .
Click to expand...

Others have already pointed out the row and column problems. Here
are additional bugs that do not actually cause breakage, but probably
should be fixed:
image.type_p = malloc(strlen(line)+1 * sizeof(image.type_p));
Click to expand...

This should read:

image.type_p = malloc((strlen(line) + 1) * sizeof *image.type_p);

There are two errors in the original. First, "var = malloc ...
sizeof var" should read "var = malloc ... sizeof *var" -- i.e.,
one more "*" on the sizeof than on the variable -- otherwise you
get the size of the pointER instead of the size of the pointEE. In
this case, you get sizeof(char *) instead of sizeof(char) (because
image.type_p has type "char *").

Second, binary "*" (multiply) binds more tightly than binary "+"
(add), so:

y + 1 * z

means:

y + (1 * z);

but you wanted:

(y + 1) * z

Since sizeof(char) is guaranteed to be equal to 1, and sizeof var
(for any var) is guaranteed to be at least 1, the difference here
is always harmless -- if sizeof(char *) is 4, you get:

y + (1 * 4)

instead of:

(y + 1) * 1

and of course multiplying by 1 leaves the original value unchanged,
so you are just adding a little too much.
image.pixel_p = malloc(sizeof(image.pixel_p) * image.max_row) ;
Click to expand...

This is a repeat of one of the two problems. The argument to
malloc() is wrong; the text above does not match the pattern:

var = malloc(N * sizeof *var);
image.pixel_p[row] = malloc(sizeof(image.pixel_p) * image.max_col);
Click to expand...

And this is yet another repeat of the same problem. These two lines
should read:

image.pixel_p = malloc(image.max_row * sizeof *image.pixel_p);

and:

image.pixel_p[row] = malloc(image.max_col * sizeof *image.pixel_p[row]);

respectively. (It is safe to write sizeof *a instead of sizeof
*(a), as the [] operator binds more tightly than the unary "*"
operator. Of course, it is also safe to parenthesize.)
 
S

SP

Chris said:
The following code compiles with no errors, but fails with
"Segmentation Fault" when I run it .
Click to expand...

Others have already pointed out the row and column problems. Here
are additional bugs that do not actually cause breakage, but probably
should be fixed:
image.type_p = malloc(strlen(line)+1 * sizeof(image.type_p));
Click to expand...

This should read:

image.type_p = malloc((strlen(line) + 1) * sizeof *image.type_p);

There are two errors in the original. First, "var = malloc ...
sizeof var" should read "var = malloc ... sizeof *var" -- i.e.,
one more "*" on the sizeof than on the variable -- otherwise you
get the size of the pointER instead of the size of the pointEE. In
this case, you get sizeof(char *) instead of sizeof(char) (because
image.type_p has type "char *").

Second, binary "*" (multiply) binds more tightly than binary "+"
(add), so:

y + 1 * z

means:

y + (1 * z);

but you wanted:

(y + 1) * z

Since sizeof(char) is guaranteed to be equal to 1, and sizeof var
(for any var) is guaranteed to be at least 1, the difference here
is always harmless -- if sizeof(char *) is 4, you get:

y + (1 * 4)

instead of:

(y + 1) * 1

and of course multiplying by 1 leaves the original value unchanged,
so you are just adding a little too much.
image.pixel_p = malloc(sizeof(image.pixel_p) * image.max_row) ;
Click to expand...

This is a repeat of one of the two problems. The argument to
malloc() is wrong; the text above does not match the pattern:

var = malloc(N * sizeof *var);
image.pixel_p[row] = malloc(sizeof(image.pixel_p) * image.max_col);
Click to expand...

And this is yet another repeat of the same problem. These two lines
should read:

image.pixel_p = malloc(image.max_row * sizeof *image.pixel_p);

and:

image.pixel_p[row] = malloc(image.max_col * sizeof *image.pixel_p[row]);

respectively. (It is safe to write sizeof *a instead of sizeof
*(a), as the [] operator binds more tightly than the unary "*"
operator. Of course, it is also safe to parenthesize.)
--
In-Real-Life: Chris Torek, Wind River Systems
Salt Lake City, UT, USA (40°39.22'N, 111°50.29'W) +1 801 277 2603
email: forget about it http://web.torek.net/torek/index.html
Reading email is like searching for food in the garbage, thanks to spammers.
Click to expand...


Chris,

Thanks for the corrections, I went back and changed the code.

As you realize I'm not a programmer, but I am interested in learning to
write decent code.
The program is an attempt to learn image processing, and my next step
is to turn this basic code into functions to help unclutter and
hopefully start building my code tool chest.

Thanks for the help.

Sal
 

Ask a Question

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

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

How to accept text and put each letter into a 2d matrix? 0
I Need Fix In Code 1
Need help finding Segmentation fault C++ 0
segmentation fault writing to array elements in structure 8
C pipe 1
self studying: this program slows down after repeated runs on my machine. 10
Need help in debugging tic tac toe (beginner) 2
help segmentation fault 6

Members online

No members online now.
Total: 30 (members: 0, guests: 30)
Robots: 180

Forum statistics

Threads
473,769
Messages
2,569,582
Members
45,066
Latest member
VytoKetoReviews

Latest Threads

Top