我的 recover.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;

那个指针指向哪里？嗯，这是随机的。它可能指向任何地方或任何地方。这是一个 "invalid" 指针，在任何情况下您现在都不能使用 outputfile 因为任何用法的结果都是未定义的。但是你怎么知道它已经初始化了呢？好吧，你不能！你不能，因为你从来没有给它分配任何你可以检查的值。

更好的代码是

FILE * outputfile = NULL;

因为现在 outputfile 有一个定义值，它是 NULL 这意味着你可以测试它是否被初始化

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

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

fwrite(jpg,512,1,outputfile) ;

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

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

这是一个超级美化、清理过的代码版本。代码未经测试，我只知道它可以编译。我知道它也变大了很多，但请考虑还有大量的评论，代码正在检查和处理所有预期的错误，甚至打印到 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);
}

我与您分享这段代码，因为学习某些编码概念可能最简单，只需看看其他人是如何编写代码的。

我的 recover.c 代码可以正确恢复 jpeg，但没有通过 cs50 检查

My code for recover.c recovers jpegs correctly but does not pass cs50 check

c

cs50