c - 我的 recovery.c 代码正确恢复 jpeg，但未通过 cs50 检查

Question

这是代码。我是 C 语言的初学者，因此任何缩短我的代码的建议将不胜感激。我检查了所有 50 张图像，它们看起来很完美，但代码未通过 cs50 检查。

int main(void)
{
    FILE* source = fopen("card.raw", "r");



    uint8_t jpg[512];
    int direct = 0;
    int jpgcounter = 0;
    uint8_t checkjpg[4];
    FILE* outputfile ;
    char filename[7] ;

    while(feof(source) == 0)
    {
        fread(jpg,512,1,source);

        for (int i = 0 ; i < 4 ; i++)
        {
            checkjpg[i] = jpg[i];
        }

        if( checkjpg[0] == 0xff && checkjpg[1] == 0xd8 && checkjpg[2] == 0xff && checkjpg[3] >= 0xe0 && checkjpg[3] <= 0xef )
        {

            if ( direct == 0 )
            {
              sprintf( filename, "%03d.jpg" , jpgcounter);
              outputfile = fopen(filename, "w");
              fwrite(jpg,512,1,outputfile);
              direct = 1;
            }
            else
            {
                fclose(outputfile);
                jpgcounter++;
                sprintf( filename, "%03d.jpg" , jpgcounter);
              outputfile = fopen(filename, "w");
              fwrite(jpg,512,1,outputfile);
            }

        }
        else
        {
           fwrite(jpg,512,1,outputfile) ;
        }
    }
    fclose(outputfile);
    fclose(source);
    return 0;
}

现在主要的问题是它没有通过cs50检查，所以一定有一些我在card.raw或其他东西上遗漏的数据，人眼无法检测到图像中的这些错误，但计算机可以。

Answer 1

我想我知道问题出在哪里。您永远不会初始化outputfile。作为初学者，您应该在声明变量时始终对其进行初始化。从来不写

int i;

写

int i = ...;

并给它一个初始值(0、-1、INT_MAX，无论你喜欢什么)。这对于指针来说非常重要。如果你写

FILE * outputfile;

那个指针指向哪里？嗯，这是随机的。它可能指向任何地方，也可能指向任何地方。这是一个“无效”指针，在任何情况下您都必须立即使用 outputfile，因为任何使用的结果都是未定义的。但你怎么知道它已经初始化了呢？好吧，你不能!您不能，因为您从未为其分配任何可以检查的值。

更好的代码

FILE * outputfile = NULL;

现在outputfile有一个定义的值，它是NULL，这意味着您可以测试它是否已初始化

if (outputfile == NULL) {
    // Not initialized
} else {
    // Initialized
}

查看您的代码并考虑以下内容:
如果循环第一次运行并且第一个 512 字节 block 与 0xffd8ff.. 的 if 测试不匹配，会发生什么情况？然后你以 else-case 结束，这个 case 执行以下操作

fwrite(jpg,512,1,outputfile) ;

但是这里的outputfile有什么值(value)呢？没有值(value)，它完全未定义。您访问任何地址的内存，这很可能会使您的应用程序崩溃，这正是您所发生的情况。如果您已将 outputfile 初始化为 NULL，则正确的代码将是:

} else if (outputfile != NULL) {
    fwrite(jpg,512,1,outputfile);
}

这里是一个 super 美化、清理的代码版本。该代码未经测试，我只知道它可以编译。我知道它也变得更大，但请考虑还有大量的注释，并且代码正在检查和处理所有预期的错误，甚至打印到 STDERR 出了什么问题。如果我愿意，我可以轻松压缩它 down to 58 lines ，这仅比问题中的代码多了 7 行，但您的代码没有捕获或打印所有这些错误:

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

int main ( void ) {
    FILE * inputFile = fopen("card.raw", "r");
    if (!inputFile) { // Same as "if (inputFile == NULL)"
        fprintf(stderr, "Cannot open input file!\n");
        // Don't close it, it didn't open!
        return 1;
    }

    // Always declare variables in the smallest possible scope!
    // Don't declare anything here you only need in the loop and
    // whose value doesn't need to survive a loop iteration.
    int fileCounter = 0;
    FILE * outputFile = NULL;
    bool writeError =  false;

    for (;;) { // Endless loop, will never terminate on its own
        uint8_t cluster[512];
        // It will read one cluster or nothing at all.
        if (fread(cluster, sizeof(cluster), 1, inputFile) != 1) {
            // If we have an open output file, close it.
            if (outputFile) {
                fclose(outputFile);
                outputFile = NULL; // Not required but good style.
            }
            break; // Terminates the loop!
            // Not reached, code flow continues after the loop.
        }

        // Check if start of new _or first_ JPG file.
        if (cluster[0] == 0xFF && cluster[1] == 0xd8
            && cluster[2] == 0xFF && cluster[3] >= 0xE0 && cluster[3] <= 0xEF
        ) {
            char filename[8];
            snprintf(filename, sizeof(filename), "%03d.jpg", fileCounter++);

            // Start nof an new JPG file.
            // If we have an "old" one, time to close it
            if (outputFile) {
                fclose(outputFile);
            }
            // Open new file
            outputFile = fopen(filename, "w");
            if (!outputFile) {
                // Cannot create output file.
                writeError = true;
                break; // Terminates the loop!
                // Not reached, code flow continues after the loop.
            }
        }

        // If we have an output file, write the cluster to it.
        if (outputFile) {
            if (fwrite(cluster, sizeof(cluster), 1, outputFile) != 1) {
                // Write error.
                writeError = true;
                // Close the file.
                fclose(outputFile);
                break; // Terminates the loop!
                // Not reached, code flow continues after the loop.
            }
        }
    }

    // If we end up here, we ran into one of the "breaks"
    // and now need to find out which one.
    bool exitWithError = false;
    if (writeError) {
        exitWithError = true;
        fprintf(stderr, "Counldn't create/write to output file!\n");
    } else if (ferror(inputFile) != 0) {
        exitWithError = true;
        fprintf(stderr, "Counldn't read from input file file!\n");
    }
    // Otherwise input file was just at the end.

    // Final clanup:
    fclose(inputFile);

    return (exitWithError ? 1 : 0);
}

我与您分享此代码是因为通过查看其他人如何编写代码来学习某些编码概念可能是最简单的。