Segmentation fault

**Andrew Poelstra** · Sep 6 '06, 04:15 AM

Re: Segmentation fault

"SP" <polifemo@comca st.netwrites:

The following code compiles with no errors, but fails with
"Segmentati on 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 "
>

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 */
};

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\ \?\\\'\"!%^&*() =+/<>,.|[]{}#~";

You have "\\" in there twice, just so you know.

char *line = NULL;
size_t size = 0;
>
image_fp = fopen("chess.pg m", "r");
if(!image_fp)
{
fprintf(stderr, "unable to open file\n");
return 1;

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);

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] != '#')

this is your problem.

[remaining code snipped]

--
Andrew Poelstra <http://www.wpsoftware. net/projects>
To reach me by email, use `apoelstra' at the above domain.
"Do BOTH ends of the cable need to be plugged in?" -Anon.

**M.B** · Sep 6 '06, 05:15 AM

Re: Segmentation fault

SP wrote:

The following code compiles with no errors, but fails with
"Segmentati on 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.pg m", "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(l ine)+1 *
sizeof(image.ty pe_p));
if(image.type_p )
{
strcpy(image.ty pe_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_r ow,
&image.max_col) ;
if ( args == 2) success = 1;
}
if(i == 2)
{
args = sscanf(line ,"%d", &image.max_v al );
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(i mage.pixel_p) * image.max_row) ;

u allocate image.pixel_p as array of image.max_row of type
"sizeof(image.p ixel_p" !!!!!!!???
strange .
sizeof(image.pi xel_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++)

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(i mage.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_c ol; 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.pixe l_p[row]);
}
free(image.pixe l_p);
return EXIT_SUCCESS;
}

**valis.eric.ykchan@gmail.com** · Sep 6 '06, 05:25 AM

Re: Segmentation fault

/* Compare row to max_col? */
for( row = 0; row <= image.max_col; row++)
{
image.pixel_p[row] = malloc(sizeof(i mage.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_c ol; 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]);
}
}
}

SP · Sep 6 '06, 12:15 PM

Re: Segmentation fault

valis.eric.ykch an@gmail.com wrote:

/* Compare row to max_col? */
for( row = 0; row <= image.max_col; row++)
{
image.pixel_p[row] = malloc(sizeof(i mage.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_c ol; 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]);
}
}
}

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(i mage.pixel_p) *
image.max_col);
if( image.pixel_p == NULL)
{
fprintf(stderr, "Memory allocation failed\n");
return EXIT_FAILURE;
}
}

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.

**Chris Torek** · Sep 6 '06, 05:05 PM

Re: Segmentation fault

In article <1157511396.082 724.215760@m73g 2000cwd.googleg roups.com>
SP <polifemo@comca st.netwrote:

>The following code compiles with no errors, but fails with
>"Segmentatio n Fault" when I run it .

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(l ine)+1 * sizeof(image.ty pe_p));

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(i mage.pixel_p) * image.max_row) ;

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(i mage.pixel_p) * image.max_col);

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

image.pixel_p = malloc(image.ma x_row * sizeof *image.pixel_p) ;

and:

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

respectively. (It is safe to write sizeof *a[i] instead of sizeof
*(a[i]), 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.

SP · Sep 8 '06, 01:05 AM

Re: Segmentation fault

Chris Torek wrote:

In article <1157511396.082 724.215760@m73g 2000cwd.googleg roups.com>
SP <polifemo@comca st.netwrote:

The following code compiles with no errors, but fails with
"Segmentati on Fault" when I run it .

>
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(l ine)+1 * sizeof(image.ty pe_p));

>
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(i mage.pixel_p) * image.max_row) ;

>
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(i mage.pixel_p) * image.max_col);

>
And this is yet another repeat of the same problem. These two lines
should read:
>
image.pixel_p = malloc(image.ma x_row * sizeof *image.pixel_p) ;
>
and:
>
image.pixel_p[row] = malloc(image.ma x_col * sizeof *image.pixel_p[row]);
>
respectively. (It is safe to write sizeof *a[i] instead of sizeof
*(a[i]), 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.

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

Segmentation fault

Segmentation fault

Comment

Comment

Comment

Comment

Comment

Comment