C fgets question

Question

struct DVDInfo  *ReadStruct( void ) {
    struct DVDInfo  *infoPtr;
    int             num;
    char            line[ kMaxLineLength ];
    char            *result;

    infoPtr = malloc( sizeof( struct DVDInfo ) );

    if ( NULL == infoPtr ) {
        printf( "Out of memory!!!  Goodbye!\n" );
        exit( 0 );
    }

    printf( "Enter DVD Title:  " );
    result = fgets( line, kMaxLineLength, stdin );
    line[ strlen( line ) - 1 ] = '\0';
    infoPtr->title = MallocAndCopy( line );

    printf( "Enter DVD comment:  " );
    result = fgets( line, kMaxLineLength, stdin );
    line[ strlen( line ) - 1 ] = '\0';
    infoPtr->comment = MallocAndCopy( line );

    do {
        printf( "Enter DVD Rating (1-10):  " );
        scanf( "%d", &num );
        Flush();
    }
    while ( ( num < 1 ) || ( num > 10 ) );

    infoPtr->rating = num;

    printf( "\n----------\n" );

    return( infoPtr );
}

为什么要在上面使用变量“result”？它并没有被使用。从fgets返回的指针被存储在其中，但它没有任何作用。

- startuprob

---

此外，为什么要使用 line[strlen(line)-1] = '\0'; 呢？fgets 会自动将终止符附加到字符串末尾。这不是画蛇添足吗？ - startuprob

如果 fgets 失败（注意 result 从未被检查），则说 line[sizeof(line)-1]=0 将会提高安全性。将其与 strlen 一起使用是无意义的，不是因为 fgets 总是附加一个终止零（在失败的情况下可能不会），而是因为在任何情况下，只要 strlen 不会出现未定义行为，指定的字节将为零，无论 fgets 是否对此有所作为。 - supercat

Answer 1

你应该测试结果是否为NULL，以检查EOF条件或错误，而不是忽略它。此外，如果不检查结果，你将在行上执行strlen，这可能会导致未初始化的数据，因为fgets失败了。实际上，在fgets之后，你应该：

if (!result)
{
  free(infoPtr); // To not leak the object allocated at the start
  return NULL; // Function failed
}

如果第一个fgets成功而第二个失败，则可能仍然存在泄漏，因为结构体的指针成员有额外的分配。不幸的是，由于该结构体未被初始化为零，您无法检查那些指针是否为NULL。所以，也许使用calloc而不是malloc，或者至少将所有结构体指针成员初始化为NULL，可能是更好的选择。

- Michael Goldshteyn

Answer 2

看起来好像有人 开始 实现错误检查，但最终搞砸了。应该将返回值与 NULL 进行比较，如果相等则报告错误。

- Ignacio Vazquez-Abrams

Answer 3

很可能，编译器会发出警告，告知函数返回值已被忽略。程序员并不关心fgets的返回值，只是简单地添加了result = 以使编译器停止催促。正确的解决方案应该是检查返回值，以确保函数成功完成。

- bta