CS50 PSet4 Recover程序无内存错误但Valgrind测试失败求助

我正在完成CS50 PSet4的Recover题目，遇到Valgrind测试失败的错误，提示：

program is free of memory errors
valgrind tests failed; see log for more information.

Valgrind指出错误出在output_file = fopen(filename, "w");这一行，试过调试器但没找到问题，代码如下：

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


typedef uint8_t BYTE;

int main(int argc, char *argv[])
{
    // Check number of arguments
    if (argc != 2)
    {
        printf("Only one argument should be used\n");
        return 1;
    }

    // Open reading file
    FILE *input_file = fopen(argv[1], "r");
    if (input_file == NULL)
    {
        printf("Could not open file.\n");
        return 2;
    }

    // Store blocks of 512 bytes in an array
    BYTE buffer[512];

    // Keep track of number of images generated
    int count_image = 0;

    // File pointer for recovered images
    FILE *output_file = NULL;

    // Char filename
    char *filename = malloc (8 * sizeof(char));

    // Read the blocks of 512 bytes
    while (fread(buffer, sizeof(char), 512, input_file))
    {
        // Check if the bytes indicate start of JPEG
        if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
        {
            // Write the JPEG filenames
            sprintf(filename, "%03i.jpg", count_image);

            // Open output file for writing
            output_file = fopen(filename, "w");

            // Count number of images found
            count_image++;
        }

        if (output_file != NULL)
        {
            fwrite(buffer, sizeof(char), 512, output_file);
        }
    }

    free(filename);
    fclose(output_file);
    fclose(input_file);

    return 0;
}

错误原因及修复方案 #

1. 未关闭已打开的文件句柄 #

每次检测到新JPEG头时，你直接给output_file重新赋值新的文件指针，但没有关闭之前打开的文件。这会导致文件句柄泄漏，Valgrind会捕捉到这个资源泄漏问题。

• 修复：打开新文件前，先关闭之前的输出文件（如果存在）：

  if (output_file != NULL)
  {
      fclose(output_file);
  }
  output_file = fopen(filename, "w");
  

2. 未检查fopen的返回值 #

如果fopen失败（比如磁盘空间不足、权限不够），output_file会变为NULL，后续操作会引发未定义行为，Valgrind也会标记潜在错误。

• 修复：添加fopen失败的判断逻辑：

  output_file = fopen(filename, "w");
  if (output_file == NULL)
  {
      fclose(input_file);
      free(filename);
      printf("无法创建输出文件\n");
      return 3;
  }
  

3. 可能关闭NULL指针 #

如果输入文件中没有任何JPEG数据，output_file会一直是NULL，最后执行fclose(output_file)会触发未定义行为，Valgrind会报错。

• 修复：关闭output_file前先判断是否为NULL：

  if (output_file != NULL)
  {
      fclose(output_file);
  }
  

4. 文件名内存分配可优化（非必须但更规范） #

你用malloc分配8字节存储文件名，虽然空间足够（%03i.jpg生成的字符串是7个字符+1个终止符），但可以直接用栈上数组替代，避免手动内存管理的麻烦：

char filename[8];

这样就不需要调用free(filename)了。

修复后的完整代码 #

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

typedef uint8_t BYTE;

int main(int argc, char *argv[])
{
    // 检查参数数量
    if (argc != 2)
    {
        printf("仅需传入一个参数\n");
        return 1;
    }

    // 打开输入文件
    FILE *input_file = fopen(argv[1], "r");
    if (input_file == NULL)
    {
        printf("无法打开文件\n");
        return 2;
    }

    BYTE buffer[512];
    int count_image = 0;
    FILE *output_file = NULL;
    char filename[8]; // 用栈数组替代malloc，简化内存管理

    while (fread(buffer, sizeof(BYTE), 512, input_file))
    {
        // 检测JPEG起始标识
        if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0)
        {
            // 关闭之前的输出文件（如果存在）
            if (output_file != NULL)
            {
                fclose(output_file);
            }

            // 生成文件名
            sprintf(filename, "%03i.jpg", count_image);

            // 打开新的输出文件并检查是否成功
            output_file = fopen(filename, "w");
            if (output_file == NULL)
            {
                fclose(input_file);
                printf("无法创建输出文件\n");
                return 3;
            }

            count_image++;
        }

        // 如果当前有打开的输出文件，写入数据
        if (output_file != NULL)
        {
            fwrite(buffer, sizeof(BYTE), 512, output_file);
        }
    }

    // 关闭最后一个输出文件（如果存在）
    if (output_file != NULL)
    {
        fclose(output_file);
    }
    fclose(input_file);

    return 0;
}

内容的提问来源于stack exchange，提问作者santa_cloudy