CS50（2020）恢复程序中的分段错误

Question

我正在尝试编写一个程序，该程序将从文件中恢复已删除的图像并将这些图像中的每一个写入它们自己的单独文件中。我已经被这个问题困扰了几天，并尽我所能自己解决它，但我现在意识到我需要一些指导。我的代码总是编译得很好，但是每次我运行我的程序时都会遇到分段错误。使用 valgrind 表明我没有任何内存泄漏。

我想我已经查明了问题所在，但我不确定如何解决。当我通过调试器运行我的程序时，它总是停在我最后一个“else”条件内的代码处（其中注释说“如果已经找到 JPEG”），并给我一条关于分段错误的错误消息。

我已经尝试在这行代码顶部打开并初始化我的文件指针 jpegn，以防止 jpegn 在运行此条件时为 NULL，但这并不能修复故障。

我对编程（和这个网站）非常陌生，所以任何建议或建议都会有所帮助。

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

typedef uint8_t BYTE;

int main(int argc, char *argv[])
{
    if(argc!=2) // Checks if the user typed in exactly 1 command-line argument
    {
        printf("Usage: ./recover image\n");
        return 1;
    }
    
    if(fopen(argv[1],"r") == NULL) // Checks if the image can be opened for reading 
    {
        printf("This image cannot be opened for reading\n");
        return 1;
    }
    
    FILE *forensic_image = fopen(argv[1],"r");  // Opens the image inputted and stores it in a new file
    
    BYTE *buffer = malloc(512 * sizeof(BYTE)); // Dynamically creates an array capable of holding 512 bytes of data
    
    if(malloc(512*sizeof(BYTE)) == NULL) // Checks if there is enough memory in the system
    {
        printf("System error\n");
        return 1;
    }
    
    // Creates a counting variable, a string and two file pointers
    
    int JPEG_num=0;
    char *filename = NULL;
   
    FILE *jpeg0 = NULL;
    FILE *jpegn = NULL;
    
    while(!feof(forensic_image))    // Repeat until end of image
    {
        fread(buffer, sizeof(BYTE), 512, forensic_image); // Read 512 bytes of data from the image into a buffer
        
        // Check for the start of a new JPEG file
        
        if(buffer[0] == 0xff & buffer[1] == 0xd8 & buffer[2] == 0xff & (buffer[3] & 0xf0) == 0xe0)
        {
            // If first JPEG
            
            if(JPEG_num == 0)
            {
                sprintf(filename, "%03i.jpg", JPEG_num);
                jpeg0 = fopen(filename, "w");
                fwrite(buffer, sizeof(BYTE), 512, jpeg0);
            }
            else    // If not first JPEG
            {
                fclose(jpeg0);
                JPEG_num++;
                
                sprintf(filename, "%03i.jpg", JPEG_num);
                jpegn = fopen(filename, "w");
                fwrite(buffer, sizeof(BYTE), 512, jpegn);
             }
            
        }
        else    // If already found JPEG
        {
            fwrite(buffer, sizeof(BYTE), 512, jpegn);
        }
        
        
    }
    
    // Close remaining files and free dynamically allocated memory

    fclose(jpegn);
    
    free(buffer);
    
}

Answer 1

编译时，始终启用警告，然后修复这些警告。

gcc -ggdb3 -Wall -Wextra -Wconversion -pedantic -std=gnu11 -c "untitled1.c" -o "untitled1.o" 

导致多个警告，例如：

untitled1.c:46:91: warning: suggest parentheses around comparison in operand of ‘&’ [-Wparentheses]

if(buffer[0] == 0xff & buffer[1] == 0xd8 & buffer[2] == 0xff & (buffer[3] & 0xf0) == 0xe0) 

注意：单个& 有点明智的AND。你真的想要一个逻辑 AND && 除了这个语句中的最后一个

关于;

FILE *forensic_image = fopen(argv[1],"r"); 

始终检查 (!=NULL) 返回值以确保操作成功。如果不成功（==NULL）则调用

perror( "fopen failed" ); 

将您的错误消息和系统认为发生错误的文本原因输出到stderr。

关于：

while(!feof(forensic_image)) 

请阅读：why while( !feof() is always wrong

关于：

FILE *forensic_image = fopen(argv[1],"r"); 

这已经在前面的代码块中完成了。绝对没有理由再次这样做，它会在代码中产生问题。建议：替换：

if(fopen(argv[1],"r") == NULL)      
{         
    printf("This image cannot be opened for reading\n");
    return 1;     
} 

与：

if( (forensic_image = fopen(argv[1],"r") ) == NULL)      
{         
    perror( "fopen for input file failed" );         
    exit( EXIT_FAILURE );     
}

关于：

BYTE *buffer = malloc( 512 * sizeof(BYTE) );

及以后：

free( buffer );

这是对代码和资源的浪费。该项目只需要一个这样的实例。建议：

#define RECORD_LEN 512 

和

unsigned char buffer[ RECORD_LEN ]; 

关于;

fread(buffer, sizeof(BYTE), 512, forensic_image); 

函数：fread() 返回一个size_t。您应该将返回的值分配给 size_t 变量并检查该值以确保操作成功。事实上，该语句应该在while() 条件中

关于;

sprintf(filename, "%03i.jpg", JPEG_num); 

这会导致未定义的行为，并可能导致段错误事件，因为指针 filename 已初始化为 NULL。建议：

char filename[20]; 

为了避免这个问题

关于：

else    // If not first JPEG             
{                 
    fclose(jpeg0); 

如果您（例如）使用第三个文件，则 jpeg0 已经关闭，从而导致运行时错误。建议删除语句：

FILE *jpeg0;

并且始终使用jpegn

关于;

else    // If already found JPEG         
{             
    fwrite(buffer, sizeof(BYTE), 512, jpegn);         
} 

在第一个输出文件中，jpegn 未设置，因此会导致崩溃。同样，所有输出文件操作只能使用jpegn。

关于：

fwrite(buffer, sizeof(BYTE), 512, jpegn); 

这会返回实际写入的（第二个参数）数量，所以应该是：

if( fwrite(buffer, sizeof(BYTE), 512, jpegn) != 512 ) { // handle error } 

发布的代码包含一些“魔术”数字，例如 512。“魔术”数字是没有基础的数字。 “神奇”数字使代码更难理解、调试等。建议使用enum 语句或#define 语句为这些“神奇”数字赋予有意义的名称，然后在整个代码中使用这些有意义的名称。

Answer 2

您的代码存在很多问题。如果 valgrind 没有发现这些，我会感到惊讶。

首先是这样的：

    if(fopen(argv[1],"r") == NULL) // Checks if the image can be opened for reading 
    {
        printf("This image cannot be opened for reading\n");
        return 1;
    }
    
    FILE *forensic_image = fopen(argv[1],"r");  // Opens the image inputted and stores it in a new file

这不是致命的，但是您打开同一个文件两次并丢弃了第一个文件指针。但是下面有类似模式的肯定是内存泄漏：

    BYTE *buffer = malloc(512 * sizeof(BYTE)); // Dynamically creates an array capable of holding 512 bytes of data
    
    if(malloc(512*sizeof(BYTE)) == NULL) // Checks if there is enough memory in the system
    {
        printf("System error\n");
        return 1;
    }

这里您分配了两次 512 字节，并且只在指针 buffer 中保留了第一个分配，而第二个分配丢失了。

然后这个：

    char *filename = NULL;

    // ...   
    
    sprintf(filename, "%03i.jpg", JPEG_num);

您正在将字符串写入未分配的内存！

还有以下几行：

        else    // If already found JPEG
        {
            fwrite(buffer, sizeof(BYTE), 512, jpegn);
        }

你如何保证jpegn 是一个有效的文件指针？可能永远不会，因为我在您的代码中看到 JPEG_num 将始终为 0。else 中由 // If not first JPEG 标记的部分是死代码。