1

I have the following code:

#include <stdio.h>
#include <ctype.h>
#include <string.h>
#include <stdlib.h>
#include <sys/stat.h>

void print_usage()
{
  printf("%s\n", "usage");
}

int file_exist (char *filename)
{
  struct stat   buffer;   
  return (stat (filename, &buffer) == 0);
}

int parse_parameters(int argc, char *argv[], char** in)
{
  unsigned int i1 = 1; // 0 is the filename
  for (; i1 < argc; ++i1)
  {
    if( 0 == strcmp("-h", argv[i1]) )
    {
      print_usage();
      return 0;
    }
    else if( 0 == strcmp("-i", argv[i1]) )
    {
      *in = malloc( sizeof(char) * strlen(argv[++i1]) + 1 );
      strcpy(*in, argv[i1]);
      continue;
    }
    else
    {
      print_usage();
      return 1;
    }
  }
  return 0;
}

int main(int argc, char *argv[])
{
  if( argc != 3 )
  {
    print_usage();
    return 0;
  }

  char* in = NULL;

  int parse = parse_parameters(argc, argv, &in);
  if ( 0 != parse )
    return parse;

  printf("in: %s\n", in);

  FILE* finput = NULL ;
  if (file_exist(in))
    finput = fopen(in, "r");
  if (finput == NULL) {
      perror("fopen");
      exit(1);
  }

  free(in);
  fclose(finput);
  return 0;
}

After running it with valgrind with following parameters:

./main -i input

I get the following:

==30977== Memcheck, a memory error detector
==30977== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==30977== Using Valgrind-3.10.0.SVN and LibVEX; rerun with -h for copyright info
==30977== Command: ./main -i input
==30977== 
in: input
fopen: No such file or directory
==30977== 
==30977== HEAP SUMMARY:
==30977==     in use at exit: 6 bytes in 1 blocks
==30977==   total heap usage: 2 allocs, 1 frees, 574 bytes allocated
==30977== 
==30977== 6 bytes in 1 blocks are still reachable in loss record 1 of 1
==30977==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==30977==    by 0x400946: parse_parameters (main.c:31)
==30977==    by 0x4009E7: main (main.c:54)
==30977== 
==30977== LEAK SUMMARY:
==30977==    definitely lost: 0 bytes in 0 blocks
==30977==    indirectly lost: 0 bytes in 0 blocks
==30977==      possibly lost: 0 bytes in 0 blocks
==30977==    still reachable: 6 bytes in 1 blocks
==30977==         suppressed: 0 bytes in 0 blocks
==30977== 
==30977== For counts of detected and suppressed errors, rerun with: -v
==30977== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Why is that ? If I try to pass in as char* then it won't get changed after the parse_parameters function.

Patryk
  • 22,602
  • 44
  • 128
  • 244
  • 3
    File doesn't exist, so `finput == NULL` branch is taken, `exit(1)` is called, and program never has chance to `free(in)`. –  Jun 13 '15 at 12:20
  • regarding this line: '*in = malloc( sizeof(char) * strlen(argv[++i1]) + 1 );' 1) 'sizeof(char)' is always 1, has no effect, and just clutters the code. Suggest removing that expression. 2) always check (!=NULL) the returned value from malloc() (and family of functions) – user3629249 Jun 13 '15 at 12:22
  • in function: parse_parameters(), 1) the argv[] already contains the string and argv[2] already contains a pointer to that string. so rather than malloc, etc. just use *in = argv[2]. 2) following logic error: function returns 0 when parameter is '-h', which is telling the main function to continue, rather than exiting. – user3629249 Jun 13 '15 at 12:31
  • in function: parse_parameters() the loop is unneeded. because this line: 'for (; i1 < argc; ++i1)' will, on the second iteration of the loop see 'i1' as 2, which is less than argc (3) so the loop will execute again. Suggest removing loop – user3629249 Jun 13 '15 at 12:37
  • function: parse_parameters will never be executed if the only parameter is '-h' because in main, the check for argc==3 will have caused the program to already have exited – user3629249 Jun 13 '15 at 12:40
  • in main, the call to 'file_exist()' is not needed because the call to fopen() for read will discover the file does not exist and return NULL and not calling fopen() will mean that 'errno' is not properly set, so the call to perror() will fail to produce the correct syserror message – user3629249 Jun 13 '15 at 12:42

1 Answers1

5

Your program is exiting as a result of the exit (1) call, which occurs before you free (in). As a result you are seeing the valgrind message.

Andrew
  • 3,770
  • 15
  • 22
  • Actually, valgrind says the value of `in` is still a valid pointer at exit, so "reachable". If the `malloc` was done in another function using a local variable, it would be "unreachable" - *no* valid pointer to it will exist at exit. – Jongware Jun 13 '15 at 12:24
  • @Andrew That was indeed the problem. Thanks. I don't know how I've missed that – Patryk Jun 13 '15 at 12:39