Help with structure->char_string

[SP]

I am writing a function to read data into a structure, but I seem to be
messing up the character string in the fopen() call.
When the program runs it does not open the file and fopen() returns a
NULL value.
I put in a few fprintf() calls to verify that the file name & path is
being set properly.

?? am I referencing the structure element correctly in the freadpng()
function ??

Thanks
Sal Polifemo


file png.h
=======
struct png_file{
char *name; /* name of image file */
FILE *filep; /* file pointer */
char *type; /* 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; /* pointer to memory space to store
image */
};

file freadpng.c
==========
#include <stdio.h>
#include "png.h"

int freadpng( struct png_file * image)
{
FILE *file_ptr;
fprintf(stderr,"\n#### file name: %s\n", &image->name);
file_ptr = fopen(image->name, "r");
if( !file_ptr )
{
fprintf(stderr,"\n#### error opening file \n");
return -1;
}
fclose(file_ptr);
return 1;
}

file main.c
=======
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "fgetdata.h"
#include "png.h"

int freadpng( struct png_file *image);

int main(int argc, char * argv[])
{
struct png_file image_in;
int result;

image_in.name = malloc( ( strlen(argv[1]) + 1 ) * sizeof
*image_in.name );

if(image_in.name)
{
strcpy(&image_in.name, argv[1]);
}
else
{
fprintf(stderr,"Failed to copy filename to variable");
}
fprintf(stderr," \n#### image.name_p: %s\n", &image_in.name);

result = freadpng( &image_in);
if( result == 1)
{
fprintf(stderr,"\n#### back in main.c ####\n");
}
else
{
fprintf(stderr," \n#### function call error\n");
}


return EXIT_SUCCESS;
}

 

[Joe Estock]

I am writing a function to read data into a structure, but I seem to be
messing up the character string in the fopen() call.
When the program runs it does not open the file and fopen() returns a
NULL value.
I put in a few fprintf() calls to verify that the file name & path is
being set properly.

?? am I referencing the structure element correctly in the freadpng()
function ??

Thanks
Sal Polifemo


file png.h
=======
struct png_file{
char *name; /* name of image file */
FILE *filep; /* file pointer */
char *type; /* 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; /* pointer to memory space to store
image */
};

file freadpng.c
==========
#include <stdio.h>
#include "png.h"

int freadpng( struct png_file * image)
{
FILE *file_ptr;
fprintf(stderr,"\n#### file name: %s\n", &image->name);

&image->name? I think you mean image->name here. &image is a pointer to
type char * (char **). I'm surprised that your compiler doesn't print a
diagnostic.

file_ptr = fopen(image->name, "r");
if( !file_ptr )
{
fprintf(stderr,"\n#### error opening file \n");
return -1;
}
fclose(file_ptr);
return 1;
}

file main.c
=======
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "fgetdata.h"
#include "png.h"

int freadpng( struct png_file *image);

int main(int argc, char * argv[])
{
struct png_file image_in;
int result;

image_in.name = malloc( ( strlen(argv[1]) + 1 ) * sizeof
*image_in.name );

if(image_in.name)
{
strcpy(&image_in.name, argv[1]);
}
else
{
fprintf(stderr,"Failed to copy filename to variable");

You really should bail here. The next line will invoke UB if your call
to malloc fails.

}
fprintf(stderr," \n#### image.name_p: %s\n", &image_in.name);

Again, you specified that you are printing a string in the format
parameter. In this scenario &image_in.name is not a string.

 

[SP]

&image->name? I think you mean image->name here. &image is a pointer to
type char * (char **). I'm surprised that your compiler doesn't print a
diagnostic.

when I remove the & operator the program crashes with a segfault error

the problem was fixed by adding the & operator
file_ptr = fopen(&image->name, "r");

if( !file_ptr )
{
fprintf(stderr,"\n#### error opening file \n");
return -1;
}
fclose(file_ptr);
return 1;
}

file main.c
=======
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "fgetdata.h"
#include "png.h"

int freadpng( struct png_file *image);

int main(int argc, char * argv[])
{
struct png_file image_in;
int result;

image_in.name = malloc( ( strlen(argv[1]) + 1 ) * sizeof
*image_in.name );

if(image_in.name)
{
strcpy(&image_in.name, argv[1]);
}
else
{
fprintf(stderr,"Failed to copy filename to variable");

You really should bail here. The next line will invoke UB if your call
to malloc fails.

}
fprintf(stderr," \n#### image.name_p: %s\n", &image_in.name);

Again, you specified that you are printing a string in the format
parameter. In this scenario &image_in.name is not a string.

result = freadpng( &image_in);
if( result == 1)
{
fprintf(stderr,"\n#### back in main.c ####\n");
}
else
{
fprintf(stderr," \n#### function call error\n");
}


return EXIT_SUCCESS;
}

I am using the gcc compiler if that makes any difference.

 

[Fred Kleinschmidt]

I am writing a function to read data into a structure, but I seem to be
messing up the character string in the fopen() call.
When the program runs it does not open the file and fopen() returns a
NULL value.
I put in a few fprintf() calls to verify that the file name & path is
being set properly.

?? am I referencing the structure element correctly in the freadpng()
function ??

Thanks
Sal Polifemo


file png.h
=======
struct png_file{
char *name; /* name of image file */
FILE *filep; /* file pointer */
char *type; /* 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; /* pointer to memory space to store
image */
};

file freadpng.c
==========
#include <stdio.h>
#include "png.h"

int freadpng( struct png_file * image)
{
FILE *file_ptr;
fprintf(stderr,"\n#### file name: %s\n", &image->name);

fprintf(stderr,"\n#### file name: %s\n", image->name);

file_ptr = fopen(image->name, "r");
if( !file_ptr )
{
fprintf(stderr,"\n#### error opening file \n");
return -1;
}
fclose(file_ptr);
return 1;
}

file main.c
=======
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "fgetdata.h"
#include "png.h"

int freadpng( struct png_file *image);

int main(int argc, char * argv[])
{
struct png_file image_in;
int result;

image_in.name = malloc( ( strlen(argv[1]) + 1 ) * sizeof
*image_in.name );

Before you do the above, you should first check whether argv[1] exists.

if(image_in.name)
{
strcpy(&image_in.name, argv[1]);

strcpy( image_in.name, argv[1] );

Or, in this particular case, since argv[1] isn't going away,
you could dispense with the malloc and copy, and just use:
image_in.name = argv[1];

}
else
{
fprintf(stderr,"Failed to copy filename to variable");
}
fprintf(stderr," \n#### image.name_p: %s\n", &image_in.name);

fprintf(stderr," \n#### image.name_p: %s\n", image_in.name);

result = freadpng( &image_in);
if( result == 1)
{
fprintf(stderr,"\n#### back in main.c ####\n");
}
else
{
fprintf(stderr," \n#### function call error\n");
}

If you malloc'd name, you should free it.

 

[Barry Schwarz]

when I remove the & operator the program crashes with a segfault error

That is because your call to strcpy is also wrong.

the problem was fixed by adding the & operator

No the problem wasn't fixed. You simply replaced one kind of
undefined behavior with another. Apply the fixes people have told
you.

file_ptr = fopen(&image->name, "r");

If your compiler did not produce a diagnostic here, you have real
problems. fopen requires a char*. You provided a char**. There is
no implicit conversion between the two. This is a violation requiring
a diagnostic.

if( !file_ptr )
{
fprintf(stderr,"\n#### error opening file \n");
return -1;
}
fclose(file_ptr);
return 1;
}

file main.c
=======
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "fgetdata.h"
#include "png.h"

int freadpng( struct png_file *image);

int main(int argc, char * argv[])
{
struct png_file image_in;
int result;

image_in.name = malloc( ( strlen(argv[1]) + 1 ) * sizeof
*image_in.name );

if(image_in.name)
{
strcpy(&image_in.name, argv[1]);

You need to remove this &. This also requires a diagnostic as
discussed above.

You want to copy the data into the area name points to, not into name
itself. This will also invoke undefined behavior whenever
strlen(argv1) >= sizeof(struc png_file). Even when it doesn't invoke
undefined behavior, it causes a memory leak since you have lost the
pointer to the allocated memory.

I am using the gcc compiler if that makes any difference.

Nope. But you may want to invoke it in compliant mode with maximum
warning level.


Remove del for email

 

[SP]

That is because your call to strcpy is also wrong.

Thanks for pinting this out, it has taken me a while to understand why
this was a problem.

No the problem wasn't fixed. You simply replaced one kind of
undefined behavior with another. Apply the fixes people have told
you.

I went back and appyed the fixes and it all works now.

file_ptr = fopen(&image->name, "r");

If your compiler did not produce a diagnostic here, you have real
problems. fopen requires a char*. You provided a char**. There is
no implicit conversion between the two. This is a violation requiring
a diagnostic.

if( !file_ptr )
{
fprintf(stderr,"\n#### error opening file \n");
return -1;
}
fclose(file_ptr);
return 1;
}

file main.c
=======
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "fgetdata.h"
#include "png.h"

int freadpng( struct png_file *image);

int main(int argc, char * argv[])
{
struct png_file image_in;
int result;

image_in.name = malloc( ( strlen(argv[1]) + 1 ) * sizeof
*image_in.name );

if(image_in.name)
{
strcpy(&image_in.name, argv[1]);

You need to remove this &. This also requires a diagnostic as
discussed above.

You want to copy the data into the area name points to, not into name
itself. This will also invoke undefined behavior whenever
strlen(argv1) >= sizeof(struc png_file). Even when it doesn't invoke
undefined behavior, it causes a memory leak since you have lost the
pointer to the allocated memory.

I am using the gcc compiler if that makes any difference.

Nope. But you may want to invoke it in compliant mode with maximum
warning level.


Remove del for email