-1

My code compiles for a few test cases, however for ./a.out copy-ppm color.ppm copy.ppm I get a -6 SIGABRT error, and for every other test case I get a -11 SIGSEGV error.

Below I am going to post my code , image,c image.h and interface.c

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

#include "image.h"

// open the file, create an ImagePPM, and return the pointer
// return NULL if the file cannot be opened

ImagePPM *readPPM(char *filename)
{
    FILE *inFile = NULL;
    inFile = fopen(filename,"r");
    if (inFile == NULL){
        return NULL;
    }
    //made
    
    ImagePPM *image = malloc(sizeof(struct _imagePPM));
    fscanf(inFile, "%*s");
    
    int numRows, numCols, limit = 0;
    fscanf(inFile, "%d %d %d", &numRows , &numCols, &limit);
    image -> pixels = (struct _pixel **)malloc(numRows*sizeof(struct _pixels *));
    for (int i = 0 ;i < numRows; i++){
        image->pixels[i] = (struct _pixel*)malloc(numCols*sizeof(struct _pixel));

    }
    for (int i = 0; i < numRows; i++){
        for (int j = 0; j < numCols; j++){
            int red, green, blue = 0;
            fscanf(inFile, "%d %d %d", &red, &green, &blue);
            (&(image->pixels[i][j]))->red = red;
            (&(image->pixels[i][j]))->green = green;
            (&(image->pixels[i][j]))->blue = blue;
        }
    }

    strcpy(image->magic, "P3");
    image -> maxVal = limit;
    image -> numRows = numRows;
    image -> numCols = numCols;
    
    fclose(inFile);

return image;
}
// open the file and write the ImagePPM to the file
// return 1 on success
// return 0 if the file cannot be opened

int writePPM(ImagePPM *pImagePPM, char *filename)
{
    FILE *outFile = fopen(filename,"w");
    if (outFile == NULL){
        return 0;
    }

    fprintf(outFile, "%s\n%d\t%d\n%d\n", pImagePPM -> magic, pImagePPM ->numRows, pImagePPM -> numCols , pImagePPM -> maxVal);
    for (int i = 0; i < pImagePPM -> numCols; i++){
        for (int j = 0; j < pImagePPM -> numRows; j++){
            Pixel *PH = &(pImagePPM -> pixels[i][j]);
            fprintf(outFile, "%d %d %d ", PH -> red , PH -> green , PH -> blue);
            free(Pixel *PH);
        }
        fprintf(outFile, "\n");
        //fprintf(outFile,"made it here");

    }

    fclose(outFile);


return 1;
}

// free the ImagePPM and its pixels
// everything with a malloc needs a free

void freePPM(ImagePPM *pImagePPM)
{
    int length = pImagePPM -> numCols;
    for(int i = length -1 ; i >= 0; i--){
        free(pImagePPM -> pixels[i]);
    }
    free(pImagePPM);
    return;
}

// open the file, create an ImagePGM, and return the pointer
// return NULL if the file cannot be opened

ImagePGM *readPGM(char *filename)
{

    FILE *inFile = NULL;
    inFile = fopen(filename, "r");
    if (inFile == NULL){
        return NULL;
    }
    ImagePGM *image = malloc(sizeof(struct _imagePGM));
    fscanf(inFile, "%*s");

    int numRows, numCols , limit = 0;
    fscanf(inFile, "%d %d %d", &numRows , &numCols , &limit);

    image -> pixels = (int **)malloc(numRows*sizeof(int*));
    for(int i = 0; i <numRows; i++){
        image -> pixels[i] = (int*)malloc(numCols*sizeof(int));
    }
    for (int i = 0; i < numRows; i++){
        for (int j = 0; j < numCols; j++){
            int total = 0;
            fscanf(inFile, "%d", &total);
            image -> pixels[i][j] = total;
        }
    }

    strcpy (image -> magic, "P2");
    image -> maxVal = limit;
    image -> numRows = numRows;
    image -> numCols = numCols;

    fclose(inFile);

    return image;
}

// open the file and write the ImagePGM to the file
// return 1 on success
// return 0 if the file cannot be opened

int writePGM(ImagePGM *pImagePGM, char *filename)
{
    FILE *outFile = NULL; 
    outFile = fopen(filename, "w");
    if (outFile == NULL){
        return 0;
    }
    fprintf(outFile, "%s\n%d\t%d\n%d\n",
    pImagePGM->magic, pImagePGM->numRows,
    pImagePGM->numCols, pImagePGM->maxVal);

    for (int i = 0 ; i < pImagePGM -> numCols; i++){
        for (int j = 0; j < pImagePGM -> numRows; j++){
            fprintf(outFile, "%d ", pImagePGM -> pixels[i][j]);
        }
        fprintf(outFile, "\n");

    }
    fclose(outFile);
return 1;
}

// free the ImagePGM and its pixels
// everything with a malloc needs a free

void freePGM(ImagePGM *pImagePGM)
{
    int numRows = pImagePGM -> numCols;
        for (int i = numRows -1; i >=0; i ++){
            free(pImagePGM->pixels[i]);
         }
         free(pImagePGM);
    return ; // maybe NULL or zero
}

ImagePGM *convertToPGM(ImagePPM *pImagePPM)
{
    int x ,y , max;
    x = pImagePPM -> numCols;
    y = pImagePPM -> numRows;
    max = pImagePPM -> maxVal;
                                            // space error 
    ImagePGM *image = malloc(sizeof(struct _imagePPM));
    image -> pixels = (int **)malloc(x*sizeof(int*));
    for (int i = 0; i < x; i++){
        image -> pixels[i] = (int*)malloc(y*sizeof(int));
        // error for struct needs to be fixed


    }
    strcpy(image -> magic, "P2");
    image -> numRows = x;
    image -> numCols = y;
    image -> maxVal = max;

    for (int i = 0; i < x; i++){
        for (int j = 0; j < y; j++){
            int total = 0;
            total += pImagePPM->pixels[i][j].red;
            total += pImagePPM->pixels[i][j].green;
            total += pImagePPM->pixels[i][j].blue;
            total /=3; // converting to grey scale by / itself and 3
            image -> pixels [i][j] = total;

        }
    }
    return image;
}

ImagePGM *shrinkPGM(ImagePGM *pImagePGM)
{
    int oldRow , oldCol, x, y, max;
    oldRow = pImagePGM -> numRows;
    oldCol = pImagePGM -> numCols;
    x = oldRow/2;
    y = oldCol/2;
    max = pImagePGM -> maxVal;
    ///////////
    //printf("Made it here");

    ImagePGM *image = malloc(sizeof(struct _imagePPM));
    image -> pixels = (int **)malloc(x*sizeof(int*));
    for (int i = 0; i < x; i++){
        image -> pixels[i] = (int*)malloc(y*sizeof(int));

    }
    
    strcpy(image -> magic, "P2");
    image -> numRows = x;
    image -> numCols = y;
    image -> maxVal = max; // could be limit instead of max or max val


    for(int i = 0; i < x; i++){
        for(int j = 0; j < y; j++){
            int total = 0;
            total += pImagePGM->pixels[i*2][j*2];
            total += pImagePGM->pixels[i*2][j*2+1];
            total += pImagePGM->pixels[i*2+1][j*2];
            total += pImagePGM->pixels[i*2+1][j*2+1];
            total /= 4;
            image->pixels[i][j] = total;
        }
    }
    return image;
}

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

#include "image.h"

int main(int argc, char **argv)
{
    if (argc != 4)
    {
        printf("Usage: %s copy-ppm input.ppm output.ppm\n", argv[0]);
        printf("       %s copy-pgm input.pgm output.pgm\n", argv[0]);
        printf("       %s grayscale input.ppm output.pgm\n", argv[0]);
        printf("       %s shrink input.pgm output.pgm\n", argv[0]);
        return 1;
    }

    char *command = argv[1];
    char *inputFilename = argv[2];
    char *outputFilename = argv[3];

    if (strcmp(command, "copy-ppm") == 0)
    {
        ImagePPM *pImagePPM = readPPM(inputFilename);
        if (pImagePPM == NULL)
        {
            printf("Unable to read the PPM file: %s\n", inputFilename);
            return 2;
        }
        int success = writePPM(pImagePPM, outputFilename);
        if (!success)
        {
            printf("Unable to write to the file: %s\n", outputFilename);
            freePPM(pImagePPM);
            return 3;
        }
        freePPM(pImagePPM);
    }
    else if (strcmp(command, "copy-pgm") == 0)
    {
        ImagePGM *pImagePGM = readPGM(inputFilename);
        if (pImagePGM == NULL)
        {
            printf("Unable to read the PGM file: %s\n", inputFilename);
            return 4;
        }
        int success = writePGM(pImagePGM, outputFilename);
        if (!success)
        {
            printf("Unable to write to the file: %s\n", outputFilename);
            freePGM(pImagePGM);
            return 5;
        }
        freePGM(pImagePGM);
    }
    else if (strcmp(command, "grayscale") == 0)
    {
        ImagePPM *pImagePPM = readPPM(inputFilename);
        if (pImagePPM == NULL)
        {
            printf("Unable to read the PPM file: %s\n", inputFilename);
            return 6;
        }
        ImagePGM *pImagePGM = convertToPGM(pImagePPM);
        int success = writePGM(pImagePGM, outputFilename);
        if (!success)
        {
            printf("Unable to write to the file: %s\n", outputFilename);
            freePPM(pImagePPM);
            freePGM(pImagePGM);
            return 7;
        }
        freePPM(pImagePPM);
        freePGM(pImagePGM);
    }
    else if (strcmp(command, "shrink") == 0)
    {
        ImagePGM *pOrig = readPGM(inputFilename);
        if (pOrig == NULL)
        {
            printf("Unable to read the PGM file: %s\n", inputFilename);
            return 8;
        }
        ImagePGM *pShrink = shrinkPGM(pOrig);
        int success = writePGM(pShrink, outputFilename);
        if (!success)
        {
            printf("Unable to write to the file: %s\n", outputFilename);
            freePGM(pOrig);
            freePGM(pShrink);
            return 9;
        }
        freePGM(pOrig);
        freePGM(pShrink);
    }
    else
    {
       printf("Unrecognized command\n");
       return 10;
    }

    return 0;
}

I am honestly lost on what I should do to fix it, I have an idea that it has something to do with with magic other than that I do not know.

  • 1
    Have you tried to use a [*debugger*](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) to catch the crashes in your code, and see where in your code they happen? And then examine variables and their values to see if they could help you figure out the problem? – Some programmer dude Nov 28 '22 at 17:10
  • 1
    On a different note, why e.g. `(&(image->pixels[i][j]))->red` instead of `image->pixels[i][j].red`? – Some programmer dude Nov 28 '22 at 17:12
  • 1
    Also, `int red, green, blue = 0;` only initializes `blue`. – Some programmer dude Nov 28 '22 at 17:13
  • 1
    And why do you ignore the input PPM format specifier? And hardcode it to `"P3"`? – Some programmer dude Nov 28 '22 at 17:14
  • Ignoring the return value of `fscanf` and failing to check that the return value of `malloc` is not `NULL` are two sure-fire ways of operating in an indeterminate state, and may lead to invoking [Undefined Behaviour](https://en.cppreference.com/w/c/language/behavior). One of the first things you should do when a program is not working as expected is to confirm these function calls (and any other function call that can *fail*) succeed in the way you expect them to. – Oka Nov 28 '22 at 17:16
  • 1
    Oh, and `free(Pixel *PH);` is wrong. First of all it's a *declaration* for the `free` function. Secondly, you don't allocate individual `Pixel` structures, but an *array* of `Pixel` structures. And of course the array of pointers to `Pixel`. – Some programmer dude Nov 28 '22 at 17:17
  • I recommend you build with extra warnings enabled, as some of the problems you have will be detectable by the compiler. And treat all warnings as errors. If you use GCC build with `-Wall -Wextra -Werror`. – Some programmer dude Nov 28 '22 at 17:19

1 Answers1

0

The problem could be this:

In the readPPM function you read the pixels using the loops:

for (int i = 0; i < numRows; i++){
    for (int j = 0; j < numCols; j++){

Then in writePPM you write the pixels in the loops:

for (int i = 0; i < pImagePPM -> numCols; i++){
    for (int j = 0; j < pImagePPM -> numRows; j++){

You have mixed numRows and numCols between reading and writing. The code will only work when numRows == numCols.

If numRows != numCols then you will go out of bounds of one of the arrays, leading to undefined behavior.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • Okay so I changed what you suggested, my -6 SIGABRT case passed , my output is fine but I am still getting a -11 SIGSEGV error. – Peytonsalv Nov 28 '22 at 18:08