0

I am working on the CS50 Intro to CS course--Lab 4: Volume. And I think I have some misunderstanding with the molloc function. At first I used the malloc function to allocate space for the header part as below:

    // TODO: Copy header from input file to output file
    uint8_t *header = malloc(HEADER_SIZE);
    if (header == NULL)
    {
        return 2;
    }
    fread(header, sizeof(header), 1, input);
    fwrite(header, sizeof(header), 1, output);
    free(header);

The program managed to compile but I think it didn't copy the header properly. Then I tried this:

    // TODO: Copy header from input file to output file
    uint8_t header[HEADER_SIZE];
    fread(header, sizeof(header), 1, input);
    fwrite(header, sizeof(header), 1, output);

And that worked, but I don't know what changes I have made by doing so. I just see this two kind of codes as having the same effect. Can somebody tell me the difference and what mistakes I made? I mean if I really want to use malloc function, what the codes should be like? Below is my full successful code for volumn.c:

    // Modifies the volume of an audio file

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

// Number of bytes in .wav header
const int HEADER_SIZE = 44;

int main(int argc, char *argv[])
{
    // Check command-line arguments
    if (argc != 4)
    {
        printf("Usage: ./volume input.wav output.wav factor\n");
        return 1;
    }

    // Open files and determine scaling factor
    FILE *input = fopen(argv[1], "r");
    if (input == NULL)
    {
        printf("Could not open file.\n");
        return 1;
    }

    FILE *output = fopen(argv[2], "w");
    if (output == NULL)
    {
        printf("Could not open file.\n");
        return 1;
    }

    float factor = atof(argv[3]);

    // TODO: Copy header from input file to output file
    uint8_t header[HEADER_SIZE];
    fread(header, sizeof(header), 1, input);
    fwrite(header, sizeof(header), 1, output);

    // TODO: Read samples from input file and write updated data to output file
    int16_t buffer;

    while (fread(&buffer, sizeof(buffer), 1, input))
    {
        buffer *= factor;
        fwrite(&buffer, sizeof(buffer), 1, output);
    }

    // Close files
    fclose(input);
    fclose(output);
}
Bruno He
  • 3
  • 2
  • 1
    Welcome to SO. You should print `sizeof(header)` in both cases. In one case, `header` is a pointer and that means it only has the size of a pointer. – Gerhardh Feb 22 '22 at 07:10
  • 1
    Tof fix this, use the size of allocated memory. – Gerhardh Feb 22 '22 at 07:29
  • Note that is impossible in C to find the size of an object from a pointer to it. It isn't `malloc` that you have misunderstood, but `sizeof`. Use `fread(header, HEADER_SIZE, 1, input);`. This is particularly common with an array as a function argument, for example in `func(char arr[10][10])` the `sizeof arr` is 4 or 8, not 100, because arrays are passed to a function as a pointer. – Weather Vane Feb 22 '22 at 08:25
  • @Gerhardh I see where my problem is. Thx!!! – Bruno He Feb 22 '22 at 14:59
  • @WeatherVane You let me understand my problem better. Thank u as well!!! – Bruno He Feb 22 '22 at 14:59
  • @Gerhardh Thank u for the welcome. I am glad to be a part of this great community – Bruno He Feb 22 '22 at 15:03

1 Answers1

-2

While working with this lab I also encountered similar problem. I was able to solve it without using malloc but I wanted to write another solution incorporating it.

The problem with your malloc is that you are not providing the correct argument. It is given that each byte of header is an uint8_t (8-bit unsigned int). So the size should be sizeof(uint8_t) * HEADER_SIZE. Check out an example here.

Therefore the correct way is:

uint8_t *header = malloc(sizeof(uint8_t) * HEADER_SIZE);

instead of uint8_t *header = malloc(HEADER_SIZE);

Also you should not check for header == NULL in the next line as we just initialized this pointer and it doesn't matter what's present there.

Thanks for the question and hope my answer helps!

fazer
  • 1
  • 1
  • 2
    `sizeof(uint8_t)` cannot be anything else than 1 (https://stackoverflow.com/questions/48655310/can-i-assume-that-sizeofuint8-t-1), and that check after malloc is not optional. malloc can fail. – Mat May 21 '22 at 05:08
  • @Mat Thanks for sharing. Now I see that my answer doesn't help at all :(. – fazer May 21 '22 at 05:28
  • Does that mean the original line works as intended? Because my output seems to be just fine. – fazer May 21 '22 at 05:30
  • the malloc is ok either way. problem in the question is misuse of `sizeof` – Mat May 21 '22 at 05:33