3

Beginner here again! I am having some trouble in a bit of code I am testing in order to try to write a array into a file, and then, hopefully, understand how to multiply each element in each row and column (write each element a certain number of times by row and column). Valgrind reports my leak is at like 34, which is the second fopen() and I don't understand why that is and how to fix it:

    #include <stdio.h>
    #include <stdlib.h>
    #define SIZE 2 

    int main(int argc, char * argv[])
    {   
       //read commandline args.
    if (argc != 3)
    {
        printf("Usage: %s <input file> <output file>\n", argv[0]);
        return 1;
    }

    //filenames
    char * infile = argv[1];
    char * outfile = argv[2];

    //create files.
    FILE * infileptr = fopen(infile, "w");
    if(infileptr == NULL)
    {
        printf("Could not create file %s.\n", argv[1]);
        free(infileptr);
        return 1;
    }   
    FILE * outfileptr = fopen(outfile, "w");
    if(outfileptr == NULL)
    {
        printf("Could not create file %s.\n", argv[2]);
        free(outfileptr);
        return 1;
    }

    //fill the infile with an array contents.
    int inArray[SIZE][SIZE];
    int count = 0, row, column;
    //intialize the array.
    for (row = 0; row < SIZE * SIZE; row++)
    {
        for(column = 0; column < SIZE * SIZE; column++)
        {
            inArray[row][column] = count++;
        }
    }
    //write to the infile.
    for (row = 0; row < SIZE * SIZE; row++)
    {
        for(column = 0; column < SIZE * SIZE; column++)
        {
            fprintf(infileptr, "%i", inArray[row][column]);
        }
    } 
    fclose(infileptr);

When I try to run it, I get the following:

    ./multiplyarray infile.txt outfile.txt
    *** stack smashing detected ***: ./multiplyarray terminated
    Aborted (core dumped)

Valgrind reports:

==12385== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==12385== Command: ./multiplyarray infile.txt outfile.txt
==12385== 
*** stack smashing detected ***: ./multiplyarray terminated
==12385== 
==12385== Process terminating with default action of signal 6 (SIGABRT)
==12385==    at 0x4E6D267: raise (raise.c:55)
==12385==    by 0x4E6EEC9: abort (abort.c:89)
==12385==    by 0x4EB0C52: __libc_message (libc_fatal.c:175)
==12385==    by 0x4F50E8B: __fortify_fail (fortify_fail.c:38)
==12385==    by 0x4F50E2F: __stack_chk_fail (stack_chk_fail.c:28)
==12385==    by 0x400884: main (multiplyarray.c:62)
==12385== 
==12385== HEAP SUMMARY:
==12385==     in use at exit: 552 bytes in 1 blocks
==12385==   total heap usage: 2 allocs, 1 frees, 1,104 bytes allocated
==12385== 
==12385== 552 bytes in 1 blocks are still reachable in loss record 1 of 1
==12385==    at 0x4C2BBCF: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==12385==    by 0x4EA711C: __fopen_internal (iofopen.c:69)
==12385==    by 0x400784: main (multiplyarray.c:34)
==12385== 
==12385== LEAK SUMMARY:
==12385==    definitely lost: 0 bytes in 0 blocks
==12385==    indirectly lost: 0 bytes in 0 blocks
==12385==      possibly lost: 0 bytes in 0 blocks
==12385==    still reachable: 552 bytes in 1 blocks
==12385==         suppressed: 0 bytes in 0 blocks
==12385== 
==12385== For counts of detected and suppressed errors, rerun with: -v
==12385== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Aborted (core dumped)
Unpossible
  • 603
  • 6
  • 23

1 Answers1

4

The problem is in your loop:

for (row = 0; row < SIZE * SIZE; row++)
    {
        for(column = 0; column < SIZE * SIZE; column++)
        {
            inArray[row][column] = count++;
        }
    }

Since you declared inArray as int inArray[SIZE][SIZE];, you're trying to write beyond the declared size. Note that you use SIZE * SIZE as delimiter for your loops, where you should have used only row < SIZE and column < SIZE

I guess you got confused with another common construct for initializing this kind of matrix. Something like this would also work for you:

for (i = 0; i < SIZE * SIZE; i++)
    {
        *(inArray + i) = count++;
    }

As a side note, there's no need to free() your FILE pointer in

if(infileptr == NULL)
{
    printf("Could not create file %s.\n", argv[1]);
    free(infileptr);
    return 1;
}   

because it was not allocated in case of error (you're essentialy doing free(NULL))

Vitor
  • 2,734
  • 29
  • 40
  • Actually, then is not only "no need" (which implies `free`ing is optional), but he **must not**. – too honest for this site Feb 02 '16 at 13:38
  • @Olaf as far as I known, free(NULL) is a NOP. This is also stated in free() manpage: "The free() function frees the memory space pointed to by ptr, which must have been returned by a previous call to malloc(), calloc(), or realloc(). Otherwise, or if free(ptr) has already been called before, undefined behavior occurs. **If ptr is NULL, no operation is performed.**" – Vitor Feb 02 '16 at 13:41
  • While that is correct, the presence of that code implies some deeper missconceptions by OP; `free`ing an obvious _null pointer_ -but acknowledged, I should have used the weaker "should". Nit-pick: he does not `free` the "FILE pointer", but tries to `free` the object it points to. (which just happens to be caught by `free` - btw: it is in general better to reference a first-level reference, i.e. the C standard here). – too honest for this site Feb 02 '16 at 13:49
  • Yeah, you're right. That's why I've put that in the answer, even though it was not the problem. And thanks for the advise about using the C standard. Will try to remember it the next time. – Vitor Feb 02 '16 at 13:54