Anish Kashyap
Anish Kashyap

Reputation: 21

Strlen Valgrind invalid read of size 1

In this program, I am trying to take the information stored in a file and then search through it to find the number stored. I removed the part where I iterate through the file to find the number that is stored because that part works fine. However, in this section, there is an error at line 63 (the one with strlen) that causes a valgrind error to report

==4149== Memcheck, a memory error detector
==4149== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==4149== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright info
==4149== Command: main.o
==4149== 
==4149== Invalid read of size 1
==4149==    at 0x4C2EDB4: strlen (vg_replace_strmem.c:454)
==4149==    by 0x108B9E: main (main.c:63)
==4149==  Address 0x54de5f7 is 0 bytes after a block of size 23 alloc'd
==4149==    at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==4149==    by 0x108B62: main (main.c:56)
==4149== 

Here is the code:

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

int findSize(char file_name[]) { 
    // opening the file in read mode 
    FILE *fp = fopen(file_name, "r"); 

    // checking if the file exist or not 
    if (fp == NULL) { 
        printf("File Not Found!\n"); 
        return -1; 
    } 

    fseek(fp, 0L, SEEK_END); 

    // calculating the size of the file 
    int res = ftell(fp); 

    // closing the file 
    fclose(fp); 

    return res; 
} 

int main() {
    char *buffer = 0;   //contents of the file
    long int is = 0; // thing that will be returned
    long int ie = 0;    // thing that will be returned
    int index1; //index of "s"
    int index2; //index of "e"
    char *a;    //phrase after s
    char *b;    //phrase after e
    int bufferlength;   //length of txt file
    int negativeCount1 = 0;
    int negativeCount2 = 0;
    long length;
    char file_name[] = { "filename" }; 
    FILE *f = fopen(file_name, "rb");

    int res = findSize(file_name); 
    if (res != -1) 
        printf("Size of the file is %d bytes \n", res); 
    if (res == 0) {
        printf("empty file detected");
        return 0;
    }

    if (f) {
        {
            fseek(f, 0, SEEK_END);
            length = ftell(f);
            fseek(f, 0, SEEK_SET);
            buffer = malloc(length);
            if (buffer) {
               fread(buffer, 1, length, f);
            }
            fclose(f);
        }
        bufferlength = strlen(buffer);
        printf("%d \n", bufferlength);
    }
}

Upvotes: 2

Views: 2324

Answers (1)

KamilCuk
KamilCuk

Reputation: 141493

In C strings are zero terminated - the last element of a string has to be zero. In case all length characters you read from the file are non-zero, strlen() will read past the end of the buffer when searching for zero byte.

If you have it, you could use strnlen() to limit the maximum length of the buffer strlen() will try to measure:

 bufferlength = strnlen(buffer, length);
 // you can also limit printf
 printf(".*s\n", (int)length, buffer); // pendantics would check if length <= INT_MAX

Or typically allocate one byte more and make sure that the last byte is zero, so that strlen() doesn't read behind buffer.

  buffer = malloc(length + 1); // allocate one more
  // your error handling of failed allocation is strange
  if (buffer != NULL) { 
      abort();
  }
  buffer[length] = '\0'; // null terminate buffer
  fread (buffer, 1, length, f);
  fclose (f);
  bufferlength = strlen(buffer); // will read at max `length + 1` characters, 
                                 // because `buffer[length] = '\0'`
  printf("%s\n", buffer); // buffer is zero terminated - valid

Upvotes: 3

Related Questions