0

I was actually trying some C programming code and I came up with this program, which for some reason is working only when an integer is declared and is given some value.

The code below works perfectly

#include<stdio.h>

int main()
{


    FILE *file = fopen("/proc/cpuinfo","r");
    char *line;
    int count = 0;
    if(file!=NULL)
    {
        while(fscanf(file," %[^\n]",line)!=-1)
        {
            printf("\n%s",line);
        }
        printf("\n\n\t* * * * * * * * * * * * * * * * * * * * * \n\n");
    }
    else
    {
        printf("\nFile Does not Exist\n");
    }
    return 0;
}

but this does not work , (I mean, when I run this I get an infinite loop of (null) values.

#include<stdio.h>

int main()
{
    FILE *file = fopen("/proc/cpuinfo","r");
    char *line;

    if(file!=NULL)
    {
        while(fscanf(file," %[^\n]",line)!=-1)
        {
            printf("\n%s",line);
        }
        printf("\n\n\t* * * * * * * * * * * * * * * * * * * * * \n\n");
    }
    else
    {
        printf("\nFile Does not Exist\n");
    }
    return 0;
}

I am using gcc compiler

gcc -v

Using built-in specs. COLLECT_GCC=gcc COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.6/lto-wrapper Target: x86_64-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Ubuntu/Linaro 4.6.3-1ubuntu5' --with-bugurl=file:///usr/share/doc/gcc-4.6/README.Bugs --enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr --program-suffix=-4.6 --enable-shared --enable-linker-build-id --with-system-zlib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.6 --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --enable-gnu-unique-object --enable-plugin --enable-objc-gc --disable-werror --with-arch-32=i686 --with-tune=generic --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu Thread model: posix gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5)

Can anyone explain what is happening here, I would like to know why this is happening.

user3234648
  • 70
  • 1
  • 8
cyberpirate92
  • 3,076
  • 4
  • 28
  • 46

4 Answers4

3

You are not allocating memory for the line buffer, which in you case is only an invalid pointer.

In the first case, you were (un)lucky, since the pointer was pointing to a writable area (specifically, you were overwriting the memory area in which count was located), but in the second case, a segmentation error is raised.

Replace:

char *line; // Pointer to... somewhere?

with:

char line[4096];

or any other suitable buffer size. If you need a buffer that won't fit in the stack, then declare it as global variable (strongly discouraged, unless your application is really small or you know what you are doing) or allocate it on the heap, with:

char *line = malloc(4096 * 1024);

and remember to call free(line) when the buffer is not needed anymore (e.g. before terminating the program).

Stefano Sanfilippo
  • 32,265
  • 7
  • 79
  • 80
2
char *line;

should be something like

char line[1024];

And add a fclose.

edit because of comment:
If you want the first definition and malloc/free: 

char *line;
line = malloc(1024);
...
free(line);
deviantfan
  • 11,268
  • 3
  • 32
  • 49
  • thanks, Using an array works , but is it possible to acheive the same using pointers ? If yes, I would like to know how. – cyberpirate92 Jan 26 '14 at 11:28
  • 1
    The name of an array variable is actually a pointer to the first element. – Stefano Sanfilippo Jan 26 '14 at 11:29
  • 1
    Basically, you are passing a pointer to fscanf. If you want more "pointerness", use malloc/free to allocate the 1024 bytes by yourself – deviantfan Jan 26 '14 at 11:29
  • @StefanoSanfilippo : Ok, I mean, if we use an array with a fixed size, what if the size of the line is much higher than the size we provided, if we use big array size it won't be efficient isn't it. – cyberpirate92 Jan 26 '14 at 11:31
  • As you don´t know the line size before reading, you would have to get a bigger array once you know the current is not sufficient. Somewhat more complex, as you have to do the reading yourself (instead of fscanf) and handle the whole memory stuff also. – deviantfan Jan 26 '14 at 11:33
  • In those situations you either read in chunks if possible (so that you keep the buffer and memory footprint small) or you allocate on the heap with `malloc`/global. – Stefano Sanfilippo Jan 26 '14 at 11:38
  • @cyberPheonix here's some discussion of the buffer overflow problem: http://stackoverflow.com/questions/1621394/how-to-prevent-scanf-causing-a-buffer-overflow-in-c – Douglas Jan 27 '14 at 14:38
1

fscanf writes to a buffer pointed to by char *line;, but line is never set to point anywhere useful. Presumably adding the extra int causes memory layout to be a little different, so it breaks in a different way.

Douglas
  • 36,802
  • 9
  • 76
  • 89
0

Better yet, on Linux at least, you can use getline(3)

int main() {
  FILE *file = fopen("/proc/cpuinfo","r");
  char *line=NULL;
  size_t linesiz= 0;
  ssize_t linelen= -1;
  if (file == NULL) { perror("/proc/cpuinfo"); exit (EXIT_FAILURE); };
  while ((linelen=getline(&line,&linesiz))>=0) 
    fputs(line,stdout);
  free(line), line=NULL;
  fclose(file);
}

However, in practice, we all know that  /proc/cpuinfo contain a few long lines (for flags notably), but probably the longest line is less than e.g. 512 bytes (on my i3770K with a Linux 3.13 kernel, it has 483 bytes). Knowing that (which is not exactly guaranteed, but is true today, see proc(5)) you could simply code with fgets(3)

char linebuf[512];
do {
  if (fgets(linebuf,sizeof(linebuf),file)==0) break;
  fputs(linebuf, stdout);
} while(!feof(linebuf));

Beware that feof(3) should be used after (not before) some <stdio.h> input operation (here fgets).

PS. Don't forget to compile with all warnings and debug info (e.g. gcc -Wall -g) and to use the debugger (gdb).

Basile Starynkevitch
  • 223,805
  • 18
  • 296
  • 547