Code keeps crashing (Week 4.2 has stopped working

Question

I have a program that has to get the longest sentence from a file. To achieve this, I am putting the first sentence in an array and then comparing future sentences to the currently largest sentence's size.

However the act of comparing both array's eludes me. The array's current_sentence and longest_sentence are both 80 characters long, but I wish to know which actually contains the longest sentence (which can be up to 80 characters long).

I have already attempted many different solutions (through google, most of which were stackoverflow results) but every time I make an attempt the returned value is the first sentence in the file, which makes me believe that either the check itself fails entirely, or the length of both array's is returned as 80.

These attempts include (but are not limited to):

if((sizeof(current_sentence) / sizeof(char)) < (sizeof(longest_sentence) / sizeof(char))

if(sizeof(current_sentence) / sizeof(current_sentence[0])) < (sizeof(longest_sentence) / sizeof(longest_sentence[0]))

Here is my code:

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

char *langste_regel(char *);

int main(void) {
    /*
     * stdout and stderr required for proper output
     */
    setvbuf(stdout, NULL, _IONBF, 0);
    setvbuf(stderr, NULL, _IONBF, 0);

    char *s = langste_regel("vb1.txt");
    if(s != NULL) {
        printf("\nde langste regel is: %s\n", s);
        free(s);
    }

    return 0;
}

char *langste_regel(char *filename) {
    FILE *file;
    file = fopen(filename, "r");

    if(file == NULL) {
        fprintf(stderr, "Kan bestand niet %s openen", filename);
    }

    char current_sentence[80];
    int len = 2;
    char *longest_sentence = (char *)malloc(sizeof(char) * len);

    fgets(longest_sentence, 80, file);

    while(fgets(current_sentence, 80, file)) {
        if(sizeof(current_sentence) < sizeof(longest_sentence)) {
            strncpy(longest_sentence, current_sentence, 80);
        }
    }

    fclose(file);

    return longest_sentence;
}

Answer 1

You want to use strlen() to get length of the line(s), not sizeof which returns the number of bytes the objects occupy.

Change

    if(sizeof(current_sentence) < sizeof(longest_sentence)) {

to

    if(strlen(current_sentence) < strlen(longest_sentence)) {

Another problem I see is that you are allocating only 2 bytes but reading upto 80 bytes here:

  char *longest_sentence = (char *)malloc(sizeof(char) * len);

  fgets(longest_sentence, 80, file);

Answer 2

The code

sizeof(current_sentence)

isn't doing what you think. To find the length of a null terminated string, use

strlen(current_sentence)

Answer 3

There are a number of problems with your code.

  file = fopen(filename, "r");

   if(file == NULL) {
       fprintf(stderr, "Kan bestand niet %s openen", filename);
   }

You check if the fopen() call is unsuccessful, but then you just move on and use the file pointer anyway. You should return an error indication to the caller.

---

  char current_sentence[80];
  int len = 2;
  char *longest_sentence = (char *)malloc(sizeof(char) * len);

  fgets(longest_sentence, 80, file);

You allocate 2 bytes to longest_sentence and then you try to read as much as 80 bytes into the buffer. You should allocate 80 bytes.

If you meant to dynamically grow the buffer as needed, you need a much more elaborate solution. You need to:

• Try to allocate 80 bytes.
• Try to read 80 bytes.
• Check if the string ends with a newline (\n).
• If not, try to realloc() the buffer to a larger size.

• Continue reading and re-allocating until

  • a newline has been found, or
  • you've reached end-of-file, or
  • a read error occurs, or
  • the re-allocation fails, or
  • you reach a pre-defined max length.

You also don't check if the string was read successfully. The fgets() function will return NULL on end-of-file or if a read error has occurred. You should return an error indicator to the caller. Eg:

if (!fgets(longest_sentence, 80, file)) {
  free (longest_sentence);
  return NULL:
}

---

  while(fgets(current_sentence, 80, file)) {
      if(sizeof(current_sentence) < sizeof(longest_sentence)) {
          strncpy(longest_sentence, current_sentence, 80);
      }
  }

The result of the sizeof operator is the size of the type of the operand, not the length of the strings. You should use strlen() (and reverse the comparison, as you have noted elsewhere).

while(fgets(current_sentence, 80, file)) {
    if(strlen(current_sentence) > strlen(longest_sentence)) {
        strncpy(longest_sentence, current_sentence, 80);
    }
}

The use of strncpy() is usually questionable. The call above will always write 80 bytes, regardless of the length of current_sentence. In general, it will not zero-terminate the output string if no zero is found within the first 80 bytes of the input string. It will in this case however, because fgets() guarantees that there is a zero byte within those 80 characters.

A simple strcpy() would be more straightforward here (in my opinion).