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

你的代码有很多问题。如果 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 标记的部分是死代码。

Answer 2

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

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()returns一个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);

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

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

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

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

Segmentation fault in CS50 (2020) recovery program

c

segmentation-fault

cs50