reading strings from a file in c

Question

I am reading a string from a file in C. The string is supposed to have a specific length and start with thisisnumbr. If both requirements are met, then something else is supposed to happen. Furthermore, I want to prevent that anything unexpected in the file might cause a crash.

My code:

#define MYSTRING "thisisnumb-"

void read_mystring()
{
  int c, i = 0, len =0 ;
  char input[sizeof( MYSTRING)+2] ;
  char check[] =  MYSTRING ;
  FILE *file ;
  file = fopen("/pathto/myfile", "r") ;
  if (file) {
      while ((c = getc(file)) != EOF)
      {
          input[i] = c ;
          i++ ;
          if (i > sizeof(input))
          {      
            len = 1 ;
            break ;
          }
      }
      fclose(file) ;
  }
  if(strncmp(input,check,sizeof(check)-1) == 0  && len == 0)
  {
   //do something
  }
}

So input has the size of MYSTRING plus 2 more characters (supposed to be 2 digits.

In the while loop I am reading myfile and storing it in input. With

if (i > sizeof(input))
{      
   len = 1 ;
   break ;
}

I make sure that the string reading stops if the string in the file appears to be longer than expected.

Then I compare the beginning of the string with strncmp and check if len==0 to make sure the string starts with MYSTRING AND also has the correct length.

If so, something else happens.

This works, meaning that I don't get an Segmentation fault if there is no file, the string in file is too long, or the string in the file doesn't start with MYSTRING.

I was wondering, if there is anything else that might break my program?

And also, when I do printf("input=%s\n",input) at the end of my function, I get my string but also an additional line with garbage?

Any ideas?

Answer 1

There are a number of things you need to look at. Foremost sizeof MYSTRING includes the storage size required for the nul-byte. It is strlen + 1. You must be very careful mixing sizeof string (on a char array) and string length.

Next, if you call this function more than once throughout your code, it may be better to fopen the file in the caller and pass a FILE* parameter to your function. (it's up to you) I would do:

/* open file in caller to prevent repeatedly opening and closing file */
FILE *fp = fopen (fname, "r");

if (!fp) {  /* validate file open for reading */
    fprintf (stderr, "error: file open failed '%s'.\n", fname);
    exit (EXIT_FAILURE);
}

Next, there are many ways to handle your function itself. As mentioned in the comment, you are better served by providing a buffer large enough to handle long strings in the file (and even then you need to validate a complete line read occurred) Reading with fgets the '\n' is read and included in the resulting buffer, so you will need to remove the trailing '\n' by overwriting with a nul-byte, e.g.

    char buf[BUFSZ] = "";
    while (fgets (buf, BUFSZ, fp)) {
        size_t len = strlen (buf);
        if (len > 0 && buf[len - 1] == '\n')
            buf[--len] = 0;
        else {
            /* handle more chars remain in line than buf can hold */
        }

After validating your line read, you simply need to check the length against your requirement and then check that the last two characters are digits, e.g.

        if (len != sizeof MYSTRING + 1) {
            /* not right length - handle error */
        }

        if (strncmp (buf, MYSTRING, sizeof MYSTRING - 1) == 0 &&
            isdigit (buf[sizeof MYSTRING - 1]) &&
            isdigit (buf[sizeof MYSTRING]))
        {
            /* string matches criteria -- do something */
        }
        else {
            /* doesn't meet conditon -- handle error */
        }

Putting it altogether, and adding a moretoread flag to read until the end of a long line if it exceeds BUFSZ, you would have something similar to the following:

void read_mystring (FILE *fp)
{
    char buf[BUFSZ] = "";
    int moretoread = 0;

    while (fgets (buf, BUFSZ, fp)) {
        size_t len = strlen (buf);
        if (len > 0 && buf[len - 1] == '\n') { /* check for newline */
            buf[--len] = 0;                    /* overwrite with nul-byte */
            moretoread = 0;                    /* reset moretoread flag */
        }
        else {
            /* handle more chars remain in line than buf can hold */
            moretoread = 1;
        }
        if (moretoread)    /* you are way over your wanted length */
            continue;      /* just read until newline encountered */
        if (len != sizeof MYSTRING + 1) {
            /* not right length - handle error */
        }

        /* check prefix followed by two digits */
        if (strncmp (buf, MYSTRING, sizeof MYSTRING - 1) == 0 &&
            isdigit (buf[sizeof MYSTRING - 1]) &&
            isdigit (buf[sizeof MYSTRING]))
        {
            /* string matches criteria -- do something */
        }
        else {
            /* doesn't meet conditon -- handle error */
        }
    }
}

Include ctype.h for isdigit().

Like I said, there are many, many different approaches you can take, these are just thoughts based on your conditions and one way of doing it.