why 50% of allocated memory is not freed?

Discussion in 'C Programming' started by MN, Feb 24, 2009.

  1. MN

    MN Guest

    Hi there,
    I've done a small test for memory allocation/free and I can't
    understand why I'm getting only 50 % freed.
    In the next code the value of "max" and "s" can be changed during
    runtime.


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

    const int max = 15;
    const int s = 2;

    int* table(int max)
    {
    int i = 0;
    int* table = NULL;

    if (table != NULL)
    free(table);

    table = malloc (sizeof(int*)* max);

    for (i = 0; i < max; i++)
    {
    if (i == 0)
    table = 0;
    else
    {
    if (i == 1)
    table= 1;
    else
    table = table[i-1]*2;
    }
    }
    return (table);
    }

    char** test_foo(int max, int s)
    {
    int i = 0;
    char** test = NULL;
    test = (char**)malloc(sizeof(char**) * max);

    for (i = 0; i< max; ++i)
    test = (char*)malloc(sizeof(char*) * s+1);

    for (i = 0; i< max; ++i)
    strcpy((char*)&test[0], "22");

    return (test);
    }


    int main ()
    {
    int* table_tmp = NULL;
    char** test_tmp = NULL;

    int j = 0;

    if (table_tmp != NULL)
    free(table_tmp);

    table_tmp = malloc (sizeof (int*) * max);
    table_tmp = table (max);

    for (j = 0; j < max; ++j)
    {
    printf("table_tmp [%d] = %d\n", j, table_tmp[j]);
    }
    free (table_tmp);

    printf("\n");

    test_tmp = malloc (sizeof (char**) * max);
    for(j = 0; j < max; ++j)
    test_tmp[j] = malloc (sizeof (char*) * s+1);

    test_tmp = test_foo(max, s);

    for (j = 0; j < max; ++j)
    {
    printf("test_tmp [%d] = %s\n", j, (char*)&test_tmp[j][0]);
    }

    for (j = 0; j < max; ++j)
    {
    free (test_tmp[j]);
    }
    free (test_tmp);

    return 0;
    }

    I'm using valgrind tool to detect memory leak, like this:
    valgrind --tool=memcheck --leak-check=full --leak-resolution=high --
    num-callers=20 ./free_variable&> dump; gedit dump

    as result:


    ==28879== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 5 from
    1)
    ==28879== malloc/free: in use at exit: 495 bytes in 17 blocks.
    ==28879== malloc/free: 34 allocs, 17 frees, 990 bytes allocated.
    ==28879== For counts of detected errors, rerun with: -v
    ==28879== searching for pointers to 17 not-freed blocks.
    ==28879== checked 65,528 bytes.
    ==28879==
    ==28879== 120 bytes in 1 blocks are definitely lost in loss record 1
    of 3
    ==28879== at 0x4A05809: malloc (vg_replace_malloc.c:149)
    ==28879== by 0x40071F: main (test_free_variable.c:60)
    ==28879==
    ==28879==
    ==28879== 375 (120 direct, 255 indirect) bytes in 1 blocks are
    definitely lost in loss record 2 of 3
    ==28879== at 0x4A05809: malloc (vg_replace_malloc.c:149)
    ==28879== by 0x400793: main (test_free_variable.c:71)
    ==28879==
    ==28879== LEAK SUMMARY:
    ==28879== definitely lost: 240 bytes in 2 blocks.
    ==28879== indirectly lost: 255 bytes in 15 blocks.
    ==28879== possibly lost: 0 bytes in 0 blocks.
    ==28879== still reachable: 0 bytes in 0 blocks.
    ==28879== suppressed: 0 bytes in 0 blocks.
    ==28879== Reachable blocks (those to which a pointer was found) are
    not shown.
    ==28879== To see them, rerun with: --show-reachable=yes
     
    MN, Feb 24, 2009
    #1
    1. Advertising

  2. MN wrote:
    > Hi there,
    > I've done a small test for memory allocation/free and I can't
    > understand why I'm getting only 50 % freed.
    > In the next code the value of "max" and "s" can be changed during
    > runtime.
    >
    >
    > #include <stdio.h>
    > #include <stdlib.h>
    > #include <string.h>
    >
    > const int max = 15;
    > const int s = 2;
    >
    > int* table(int max)
    > {
    > int i = 0;
    > int* table = NULL;
    >
    > if (table != NULL)
    > free(table);


    Dead code. You just initialized it to NULL, so how should it have a differnt
    value?
    Unless you mant ti hav it as
    static int*table = NULL

    >
    > table = malloc (sizeof(int*)* max);


    Better:
    table = malloc (sizeof(table)* max);

    Also you'd better check for success/failure here to not dereference NULL a
    few lines down

    > for (i = 0; i < max; i++)
    > {
    > if (i == 0)
    > table = 0;
    > else
    > {
    > if (i == 1)
    > table= 1;
    > else
    > table = table[i-1]*2;
    > }
    > }
    > return (table);
    > }
    >
    > char** test_foo(int max, int s)
    > {
    > int i = 0;
    > char** test = NULL;
    > test = (char**)malloc(sizeof(char**) * max);
    >
    > for (i = 0; i< max; ++i)
    > test = (char*)malloc(sizeof(char*) * s+1);


    Why the casts?

    >
    > for (i = 0; i< max; ++i)
    > strcpy((char*)&test[0], "22");
    >
    > return (test);
    > }
    >
    >
    > int main ()
    > {
    > int* table_tmp = NULL;
    > char** test_tmp = NULL;
    >
    > int j = 0;
    >
    > if (table_tmp != NULL)
    > free(table_tmp);


    Dead code again

    >
    > table_tmp = malloc (sizeof (int*) * max);
    > table_tmp = table (max);


    here you leak memory allocated in main()

    > for (j = 0; j < max; ++j)
    > {
    > printf("table_tmp [%d] = %d\n", j, table_tmp[j]);
    > }
    > free (table_tmp);


    here you free mem from table(), but lost access to the mem from main()

    > printf("\n");
    >
    > test_tmp = malloc (sizeof (char**) * max);
    > for(j = 0; j < max; ++j)
    > test_tmp[j] = malloc (sizeof (char*) * s+1);
    >
    > test_tmp = test_foo(max, s);


    here you leak the merory allocated in main()

    > for (j = 0; j < max; ++j)
    > {
    > printf("test_tmp [%d] = %s\n", j, (char*)&test_tmp[j][0]);
    > }
    >
    > for (j = 0; j < max; ++j)
    > {
    > free (test_tmp[j]);
    > }
    > free (test_tmp);


    here you free memory allocated in test_foo(). But you already lost access to
    the one allocatted in main()

    > return 0;
    > }
    >
    > I'm using valgrind tool to detect memory leak, like this:
    > valgrind --tool=memcheck --leak-check=full --leak-resolution=high --
    > num-callers=20 ./free_variable&> dump; gedit dump
    >
    > as result:
    >
    >
    > ==28879== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 5 from
    > 1)
    > ==28879== malloc/free: in use at exit: 495 bytes in 17 blocks.
    > ==28879== malloc/free: 34 allocs, 17 frees, 990 bytes allocated.
    > ==28879== For counts of detected errors, rerun with: -v
    > ==28879== searching for pointers to 17 not-freed blocks.
    > ==28879== checked 65,528 bytes.
    > ==28879==
    > ==28879== 120 bytes in 1 blocks are definitely lost in loss record 1
    > of 3
    > ==28879== at 0x4A05809: malloc (vg_replace_malloc.c:149)
    > ==28879== by 0x40071F: main (test_free_variable.c:60)
    > ==28879==
    > ==28879==
    > ==28879== 375 (120 direct, 255 indirect) bytes in 1 blocks are
    > definitely lost in loss record 2 of 3
    > ==28879== at 0x4A05809: malloc (vg_replace_malloc.c:149)
    > ==28879== by 0x400793: main (test_free_variable.c:71)
    > ==28879==
    > ==28879== LEAK SUMMARY:
    > ==28879== definitely lost: 240 bytes in 2 blocks.
    > ==28879== indirectly lost: 255 bytes in 15 blocks.
    > ==28879== possibly lost: 0 bytes in 0 blocks.
    > ==28879== still reachable: 0 bytes in 0 blocks.
    > ==28879== suppressed: 0 bytes in 0 blocks.
    > ==28879== Reachable blocks (those to which a pointer was found) are
    > not shown.
    > ==28879== To see them, rerun with: --show-reachable=yes
     
    Joachim Schmitz, Feb 24, 2009
    #2
    1. Advertising

  3. MN

    sathya Guest

    On Feb 24, 4:40 pm, MN <> wrote:
    > Hi there,
    > I've done a small test for memory allocation/free and I can't
    > understand why I'm getting only 50 % freed.
    > In the next code the value of "max" and "s" can be changed during
    > runtime.
    >
    > #include <stdio.h>
    > #include <stdlib.h>
    > #include <string.h>
    >
    > const int max = 15;
    > const int s = 2;
    >
    > int* table(int max)
    > {
    >         int i = 0;
    >         int* table = NULL;
    >
    >         if (table != NULL)
    >                 free(table);
    >
    >         table = malloc (sizeof(int*)* max);
    >
    >         for (i = 0; i < max; i++)
    >         {
    >                 if (i == 0)
    >                         table = 0;
    >                 else
    >                 {
    >                         if (i == 1)
    >                                 table= 1;
    >                         else
    >                                 table = table[i-1]*2;
    >                 }
    >         }
    >         return (table);
    >
    > }
    >
    > char** test_foo(int max, int s)
    > {
    >         int i = 0;
    >         char** test = NULL;
    >         test = (char**)malloc(sizeof(char**) * max);
    >
    >         for (i = 0; i< max; ++i)
    >                 test = (char*)malloc(sizeof(char*) * s+1);
    >
    >         for (i = 0; i< max; ++i)
    >                 strcpy((char*)&test[0], "22");
    >
    >         return (test);
    >
    > }
    >
    > int main ()
    > {
    >         int* table_tmp = NULL;
    >         char** test_tmp = NULL;
    >
    >         int j = 0;
    >
    >         if (table_tmp != NULL)
    >                 free(table_tmp);
    >
    >         table_tmp = malloc (sizeof (int*) * max);
    >         table_tmp = table (max);
    >
    >         for (j = 0; j < max; ++j)
    >         {
    >                 printf("table_tmp [%d] = %d\n", j, table_tmp[j]);
    >         }
    >         free (table_tmp);
    >
    >         printf("\n");
    >
    >         test_tmp = malloc (sizeof (char**) * max);
    >         for(j = 0; j < max; ++j)
    >                 test_tmp[j] = malloc (sizeof (char*) * s+1);
    >
    >         test_tmp = test_foo(max, s);
    >
    >         for (j = 0; j < max; ++j)
    >         {
    >                 printf("test_tmp [%d] = %s\n", j, (char*)&test_tmp[j][0]);
    >         }
    >
    >         for (j = 0; j < max; ++j)
    >         {
    >                 free (test_tmp[j]);
    >         }
    >         free (test_tmp);
    >
    >         return 0;
    >
    > }
    >
    > I'm using valgrind tool to detect memory leak, like this:
    > valgrind --tool=memcheck --leak-check=full --leak-resolution=high --
    > num-callers=20 ./free_variable&> dump; gedit dump
    >
    > as result:
    >
    > ==28879== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 5 from
    > 1)
    > ==28879== malloc/free: in use at exit: 495 bytes in 17 blocks.
    > ==28879== malloc/free: 34 allocs, 17 frees, 990 bytes allocated.
    > ==28879== For counts of detected errors, rerun with: -v
    > ==28879== searching for pointers to 17 not-freed blocks.
    > ==28879== checked 65,528 bytes.
    > ==28879==
    > ==28879== 120 bytes in 1 blocks are definitely lost in loss record 1
    > of 3
    > ==28879==    at 0x4A05809: malloc (vg_replace_malloc.c:149)
    > ==28879==    by 0x40071F: main (test_free_variable.c:60)
    > ==28879==
    > ==28879==
    > ==28879== 375 (120 direct, 255 indirect) bytes in 1 blocks are
    > definitely lost in loss record 2 of 3
    > ==28879==    at 0x4A05809: malloc (vg_replace_malloc.c:149)
    > ==28879==    by 0x400793: main (test_free_variable.c:71)
    > ==28879==
    > ==28879== LEAK SUMMARY:
    > ==28879==    definitely lost: 240 bytes in 2 blocks.
    > ==28879==    indirectly lost: 255 bytes in 15 blocks.
    > ==28879==      possibly lost: 0 bytes in 0 blocks.
    > ==28879==    still reachable: 0 bytes in 0 blocks.
    > ==28879==         suppressed: 0 bytes in 0 blocks.
    > ==28879== Reachable blocks (those to which a pointer was found) are
    > not shown.
    > ==28879== To see them, rerun with: --show-reachable=yes





    Hi

    you are free'ing the memory which is allocated in the function
    1. table(int max)
    2. test_foo()
    but not free'ing the Memory allocated in the function main()


    Thanks
    Sathya
     
    sathya, Feb 24, 2009
    #3
  4. MN <> wrote:
    > I've done a small test for memory allocation/free and I can't
    > understand why I'm getting only 50 % freed.
    > In the next code the value of "max" and "s" can be changed during
    > runtime.


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


    > const int max = 15;
    > const int s = 2;


    > int* table(int max)


    Having an argument with the same name as a global variable
    is rather bad style (and it hides the global variable).

    > {
    > int i = 0;
    > int* table = NULL;


    Having a variable with the same name as a function is at least
    bad style.

    > if (table != NULL)
    > free(table);


    What's that for? One line uo you initialized 'table' to NULL.

    > table = malloc (sizeof(int*)* max);


    You allocate int pointers here instead of ints and use that
    memory afterwards as if it would be made for ints.

    > for (i = 0; i < max; i++)
    > {
    > if (i == 0)
    > table = 0;
    > else
    > {
    > if (i == 1)
    > table= 1;
    > else
    > table = table[i-1]*2;
    > }
    > }
    > return (table);


    Just a hint: 'return' is not a function, so the parentheses
    are unnecessary.

    > }


    > char** test_foo(int max, int s)
    > {
    > int i = 0;
    > char** test = NULL;
    > test = (char**)malloc(sizeof(char**) * max);


    Same problem as everywhere else, the pointer you assign the
    address returned by malloc() to doesn't fit the type of the
    memory you allocated. And drop the cast of the return value
    of malloc() - it won't buy you anything and might bite you
    if you forget to include <stdlib.h>.

    > for (i = 0; i< max; ++i)
    > test = (char*)malloc(sizeof(char*) * s+1);


    The same problem again. And another one is that the '*' operator
    binds stronger that '+' (as you learn in primary school;-) I
    guess you meant

    test = malloc(sizeof(char) * (s+1));

    > for (i = 0; i< max; ++i)
    > strcpy((char*)&test[0], "22");


    Wouldn't
    strcpy( test, "22" );

    be a bit easier to read? It does the same. And having to use
    casts is often a sign that there's a problem that hasn't been
    understood and instead the compiler has been gagged to keep
    it from pointing out the problem...

    > return (test);
    > }



    > int main ()


    Better:

    int main( void )

    > {
    > int* table_tmp = NULL;
    > char** test_tmp = NULL;


    > int j = 0;


    > if (table_tmp != NULL)
    > free(table_tmp);


    > table_tmp = malloc (sizeof (int*) * max);


    Same problem as above, 'table_tmp' is a pointer to int
    and then you allocate memory for a number of int pointers
    and assign that to 'table_tmp'. But for that to work
    correctly 'table_tmp' would have to have type pointer to
    pointer to int.

    > table_tmp = table (max);


    And here you throw away the memory you just allocated without
    freeing it since you overwrite the pointer you got from malloc().

    > for (j = 0; j < max; ++j)
    > {
    > printf("table_tmp [%d] = %d\n", j, table_tmp[j]);
    > }
    > free (table_tmp);


    Here you only free the memory that was allocated in
    function table().

    > printf("\n");


    > test_tmp = malloc (sizeof (char**) * max);


    Obviously you want to allocate an array of char pointers
    here, but you allocate memory for pointer to pointer to
    char. Avoid such mistakes by using

    test_tmp = malloc (max * sizeof *test_tmp);

    > for(j = 0; j < max; ++j)
    > test_tmp[j] = malloc (sizeof (char*) * s+1);


    You obviously want to allocate arrays of chars but you're
    actually allocate an arrray of char pointers.

    > test_tmp = test_foo(max, s);


    And here you forget what 'test_tmp' was pointing to (and
    what the pointers stored in 'test_tmp' were pointing to,
    too), that's another place where you can't free the memory
    you allocated anymore.

    > for (j = 0; j < max; ++j)
    > {
    > printf("test_tmp [%d] = %s\n", j, (char*)&test_tmp[j][0]);
    > }


    > for (j = 0; j < max; ++j)
    > {
    > free (test_tmp[j]);
    > }
    > free (test_tmp);


    Ad here you only free the memory allocated in test_foo().

    > return 0;
    > }


    > I'm using valgrind tool to detect memory leak, like this:
    > valgrind --tool=memcheck --leak-check=full --leak-resolution=high --
    > num-callers=20 ./free_variable&> dump; gedit dump


    You don't need to use valgrind to see that your program is
    leaking memory like a sinking ship;-) Rule: If you obtained
    memory from malloc() and friends never ever forget that
    value before you have free'ed the memory again.

    Regards, Jens
    --
    \ Jens Thoms Toerring ___
    \__________________________ http://toerring.de
     
    Jens Thoms Toerring, Feb 24, 2009
    #4
  5. MN <> writes:

    > I've done a small test for memory allocation/free and I can't
    > understand why I'm getting only 50 % freed.
    > In the next code the value of "max" and "s" can be changed during
    > runtime.
    >
    >
    > #include <stdio.h>
    > #include <stdlib.h>
    > #include <string.h>
    >
    > const int max = 15;
    > const int s = 2;
    >
    > int* table(int max)
    > {
    > int i = 0;
    > int* table = NULL;
    >
    > if (table != NULL)
    > free(table);
    >
    > table = malloc (sizeof(int*)* max);


    All you malloc calls have the same problem so this is as good an
    example as any to pick. You are allocating using the wrong size. An
    int * points to the first of many ints. I.e. you need malloc(max *
    sizeof(int)) rather than sizeof(int *). There is a better way to call
    malloc that avoids this problem:

    exp = malloc(size * sizeof *exp);

    This way you can be sure the objects are of the right size -- that of
    the kind of thing that 'exp' points to. It is sometimes neater to use
    exp[0] rather than *exp -- it depends on the form of exp.

    Of course, in this case, int * and int may have the same size, but
    that is unlikely to be true for you other allocations involving
    char/char * confusions! Now, there may be other problems with the
    code, but I would fix this first.

    <snip>
    > char** test = NULL;
    > test = (char**)malloc(sizeof(char**) * max);


    test = malloc(max * sizeof *test);

    > for (i = 0; i< max; ++i)
    > test = (char*)malloc(sizeof(char*) * s+1);


    test = malloc(s * sizeof test[0] + 1);

    but here, when the size is guaranteed to be 1, I don't use this form.
    I'd write simply:

    test = malloc(s + 1);

    > for (i = 0; i< max; ++i)
    > strcpy((char*)&test[0], "22");
    >
    > return (test);
    > }


    <snip>
    > int* table_tmp = NULL;
    > char** test_tmp = NULL;

    <snip>
    > table_tmp = malloc (sizeof (int*) * max);


    table_tmp = malloc(max * sizeof *table_tmp);

    and so on...


    --
    Ben.
     
    Ben Bacarisse, Feb 24, 2009
    #5
  6. Joachim Schmitz wrote:
    > MN wrote:
    >> Hi there,
    >> I've done a small test for memory allocation/free and I can't
    >> understand why I'm getting only 50 % freed.
    >> In the next code the value of "max" and "s" can be changed during
    >> runtime.
    >>
    >>
    >> #include <stdio.h>
    >> #include <stdlib.h>
    >> #include <string.h>
    >>
    >> const int max = 15;
    >> const int s = 2;
    >>
    >> int* table(int max)
    >> {
    >> int i = 0;
    >> int* table = NULL;
    >>
    >> if (table != NULL)
    >> free(table);

    >
    > Dead code. You just initialized it to NULL, so how should it have a
    > differnt value?
    > Unless you mant ti hav it as


    Oh boy... 'meant to have' of course, I really should read again what I write
    before hitting the send button...

    > static int*table = NULL
    >
    >>
    >> table = malloc (sizeof(int*)* max);

    >
    > Better:
    > table = malloc (sizeof(table)* max);


    Wrong, table = malloc (sizeof(*table)* max)

    >
    > Also you'd better check for success/failure here to not dereference
    > NULL a few lines down
    >
    >> for (i = 0; i < max; i++)
    >> {
    >> if (i == 0)
    >> table = 0;
    >> else
    >> {
    >> if (i == 1)
    >> table= 1;
    >> else
    >> table = table[i-1]*2;
    >> }
    >> }
    >> return (table);
    >> }
    >>
    >> char** test_foo(int max, int s)
    >> {
    >> int i = 0;
    >> char** test = NULL;
    >> test = (char**)malloc(sizeof(char**) * max);
    >>
    >> for (i = 0; i< max; ++i)
    >> test = (char*)malloc(sizeof(char*) * s+1);

    >
    > Why the casts?
    >
    >>
    >> for (i = 0; i< max; ++i)
    >> strcpy((char*)&test[0], "22");
    >>
    >> return (test);
    >> }
    >>
    >>
    >> int main ()
    >> {
    >> int* table_tmp = NULL;
    >> char** test_tmp = NULL;
    >>
    >> int j = 0;
    >>
    >> if (table_tmp != NULL)
    >> free(table_tmp);

    >
    > Dead code again
    >
    >>
    >> table_tmp = malloc (sizeof (int*) * max);
    >> table_tmp = table (max);

    >
    > here you leak memory allocated in main()
    >
    >> for (j = 0; j < max; ++j)
    >> {
    >> printf("table_tmp [%d] = %d\n", j, table_tmp[j]);
    >> }
    >> free (table_tmp);

    >
    > here you free mem from table(), but lost access to the mem from main()
    >
    >> printf("\n");
    >>
    >> test_tmp = malloc (sizeof (char**) * max);
    >> for(j = 0; j < max; ++j)
    >> test_tmp[j] = malloc (sizeof (char*) * s+1);
    >>
    >> test_tmp = test_foo(max, s);

    >
    > here you leak the merory allocated in main()
    >
    >> for (j = 0; j < max; ++j)
    >> {
    >> printf("test_tmp [%d] = %s\n", j, (char*)&test_tmp[j][0]);
    >> }
    >>
    >> for (j = 0; j < max; ++j)
    >> {
    >> free (test_tmp[j]);
    >> }
    >> free (test_tmp);

    >
    > here you free memory allocated in test_foo(). But you already lost
    > access to the one allocatted in main()
    >
    >> return 0;
    >> }
    >>
    >> I'm using valgrind tool to detect memory leak, like this:
    >> valgrind --tool=memcheck --leak-check=full --leak-resolution=high --
    >> num-callers=20 ./free_variable&> dump; gedit dump
    >>
    >> as result:
    >>
    >>
    >> ==28879== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 5 from
    >> 1)
    >> ==28879== malloc/free: in use at exit: 495 bytes in 17 blocks.
    >> ==28879== malloc/free: 34 allocs, 17 frees, 990 bytes allocated.
    >> ==28879== For counts of detected errors, rerun with: -v
    >> ==28879== searching for pointers to 17 not-freed blocks.
    >> ==28879== checked 65,528 bytes.
    >> ==28879==
    >> ==28879== 120 bytes in 1 blocks are definitely lost in loss record 1
    >> of 3
    >> ==28879== at 0x4A05809: malloc (vg_replace_malloc.c:149)
    >> ==28879== by 0x40071F: main (test_free_variable.c:60)
    >> ==28879==
    >> ==28879==
    >> ==28879== 375 (120 direct, 255 indirect) bytes in 1 blocks are
    >> definitely lost in loss record 2 of 3
    >> ==28879== at 0x4A05809: malloc (vg_replace_malloc.c:149)
    >> ==28879== by 0x400793: main (test_free_variable.c:71)
    >> ==28879==
    >> ==28879== LEAK SUMMARY:
    >> ==28879== definitely lost: 240 bytes in 2 blocks.
    >> ==28879== indirectly lost: 255 bytes in 15 blocks.
    >> ==28879== possibly lost: 0 bytes in 0 blocks.
    >> ==28879== still reachable: 0 bytes in 0 blocks.
    >> ==28879== suppressed: 0 bytes in 0 blocks.
    >> ==28879== Reachable blocks (those to which a pointer was found) are
    >> not shown.
    >> ==28879== To see them, rerun with: --show-reachable=yes
     
    Joachim Schmitz, Feb 24, 2009
    #6
  7. MN

    Kaz Kylheku Guest

    On 2009-02-24, MN <> wrote:
    > Hi there,
    > I've done a small test for memory allocation/free and I can't
    > understand why I'm getting only 50 % freed.
    > In the next code the value of "max" and "s" can be changed during
    > runtime.
    >
    >
    > #include <stdio.h>
    > #include <stdlib.h>
    > #include <string.h>
    >
    > const int max = 15;
    > const int s = 2;
    >
    > int* table(int max)
    > {
    > int i = 0;
    > int* table = NULL;
    >
    > if (table != NULL)
    > free(table);


    Pointless. The free function accepts null pointers. Note that in the above
    statement, table is null regardless of the execution path by which the
    statement is reached, so the statement is pointless.

    > table = malloc (sizeof(int*)* max);


    The apparent intent here is to allocate a vector of pointers. If that is
    the case, the pointer should be ``int **table''.

    If that is not the intent, the expression should be sizeof (int) * max.

    You could avoid such inconsistencies by using this idiom:

    /* size derived from pointer's referenced type */
    table = malloc(max * sizeof *table);

    > for (i = 0; i < max; i++)
    > {
    > if (i == 0)
    > table = 0;
    > else
    > {
    > if (i == 1)
    > table= 1;
    > else
    > table = table[i-1]*2;


    Here you will run into undefined behavior (signed integer overflow) unless
    the value of max is small.

    Anyway, this table is useless as a lookup table, since powers of two are easy
    to calculate.

    Note that a compound if statement like this, where an integer value is
    successively compared to some constant values, is better expressed as a switch:

    switch (i) {
    case 0:
    table = 0;
    break;
    case 1:
    table = 1;
    break;
    default:
    table = table[i-1] * 2;
    break;
    }

    Note how the redundancy in the two base cases now leaps out at you. When i is
    0, the value is 0, and when i is 1, the value is 1. The switch statement lets
    us condense this:

    switch (i) {
    case 0:
    case 1:
    table = i;
    break;
    default:
    table = table[i-1] * 2;
    break;
    }

    But of course you can replace the above switch with this:

    table = (i == 0) ? 0 : 1 << (i - 1);

    In either case, the behavior is undefined on overflow. If the multiplication by
    two calls for a value that exceeds INT_MAX, or if the 1 << (i - 1) shift
    tries to shift a 1 into the sign bit, the behavior is undefined.

    > }
    > }
    > return (table);
    > }
    >
    > char** test_foo(int max, int s)
    > {
    > int i = 0;
    > char** test = NULL;
    > test = (char**)malloc(sizeof(char**) * max);


    It's pointless to initialize a variable if the next statement overwrites that
    value. Such things only confuse people reading the code, because it is expected
    that a value stored into a variable is live -- i.e. has a use somewhere.

    Again, you have the wrong size calculation. It's a table of char * pointers.

    char **test = (char **) malloc(sizeof *test * max);

    > for (i = 0; i< max; ++i)
    > test = (char*)malloc(sizeof(char*) * s+1);


    Again, wrong calculation (more memory than needed). You're allocating
    for an array of char, so the size is (sizeof(char)* (s + 1)). But sizeof(char)
    is 1 by definition, so you just need to malloc(s + 1).

    It's still a good idea to write malloc(sizeof *test * s + 1), because
    it's easier to maintain if the code is changed to some other character type,
    like wchar_t for internationalization.

    Lastly, please take careful note that whitespace does not determine
    order of operations in C arithmetic!!!

    If you write

    a * b+c

    that does not cause the addition to be done first. It still means (a * b) + c.
    The expression sizeof (T) * n + 1 does not mean ``space for an array of T, with
    n + 1 elements'' it means ``space for an array of T of n elements, with 1 byte
    of slack.''

    Anyway, your code is too tiresome to keep commenting on further.

    Go through some tutorials, read books, the c.l.c FAQ, etc.
     
    Kaz Kylheku, Feb 24, 2009
    #7
  8. MN

    CBFalconer Guest

    Ben Bacarisse wrote:
    > <> writes:
    >

    .... snip ...
    >
    >> table = malloc (sizeof(int*)* max);

    >
    > All you malloc calls have the same problem so this is as good an
    > example as any to pick. You are allocating using the wrong size.
    > An int * points to the first of many ints. I.e. you need
    > malloc(max * sizeof(int)) rather than sizeof(int *). There is a
    > better way to call malloc that avoids this problem:


    Read the original line again, carefully.

    --
    [mail]: Chuck F (cbfalconer at maineline dot net)
    [page]: <http://cbfalconer.home.att.net>
    Try the download section.
     
    CBFalconer, Feb 24, 2009
    #8
  9. CBFalconer <> writes:
    > Ben Bacarisse wrote:
    >> <> writes:
    >>

    > ... snip ...
    >>
    >>> table = malloc (sizeof(int*)* max);

    >>
    >> All you malloc calls have the same problem so this is as good an
    >> example as any to pick. You are allocating using the wrong size.
    >> An int * points to the first of many ints. I.e. you need
    >> malloc(max * sizeof(int)) rather than sizeof(int *). There is a
    >> better way to call malloc that avoids this problem:

    >
    > Read the original line again, carefully.


    I think he did. So did I. table was declared as type int*. I see
    nothing wrong with what Ben wrote. If you do, please explain.

    --
    Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    Nokia
    "We must do something. This is something. Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"
     
    Keith Thompson, Feb 24, 2009
    #9
  10. MN

    CBFalconer Guest

    Keith Thompson wrote:
    > CBFalconer <> writes:
    >> Ben Bacarisse wrote:
    >>> <> writes:
    >>>

    >> ... snip ...
    >>>
    >>>> table = malloc (sizeof(int*)* max);
    >>>
    >>> All you malloc calls have the same problem so this is as good an
    >>> example as any to pick. You are allocating using the wrong size.
    >>> An int * points to the first of many ints. I.e. you need
    >>> malloc(max * sizeof(int)) rather than sizeof(int *). There is a
    >>> better way to call malloc that avoids this problem:

    >>
    >> Read the original line again, carefully.

    >
    > I think he did. So did I. table was declared as type int*. I see
    > nothing wrong with what Ben wrote. If you do, please explain.


    I see space for an array of int* pointers being assigned. So

    malloc(sizeof (int*) * max)

    is confusing, but identical to:

    malloc(max * sizeof (int*))

    of course all question is eliminated with

    table = malloc(max * sizeof *table);

    --
    [mail]: Chuck F (cbfalconer at maineline dot net)
    [page]: <http://cbfalconer.home.att.net>
    Try the download section.
     
    CBFalconer, Feb 25, 2009
    #10
  11. CBFalconer <> writes:
    > Keith Thompson wrote:
    >> CBFalconer <> writes:
    >>> Ben Bacarisse wrote:
    >>>> <> writes:
    >>>>
    >>> ... snip ...
    >>>>
    >>>>> table = malloc (sizeof(int*)* max);
    >>>>
    >>>> All you malloc calls have the same problem so this is as good an
    >>>> example as any to pick. You are allocating using the wrong size.
    >>>> An int * points to the first of many ints. I.e. you need
    >>>> malloc(max * sizeof(int)) rather than sizeof(int *). There is a
    >>>> better way to call malloc that avoids this problem:
    >>>
    >>> Read the original line again, carefully.

    >>
    >> I think he did. So did I. table was declared as type int*. I see
    >> nothing wrong with what Ben wrote. If you do, please explain.

    >
    > I see space for an array of int* pointers being assigned. So
    >
    > malloc(sizeof (int*) * max)
    >
    > is confusing, but identical to:
    >
    > malloc(max * sizeof (int*))


    Yes, but I ask you again: what was wrong with what Ben wrote?

    The original code used "sizeof(int*)", which was incorrect. Ben
    changed it to "sizeof(int)", which is correct (but not ideal; see
    below). Ben also swapped the operands of the multiplication, which
    IMHO makes the code a bit clearer.

    So how exactly was your advice that Ben should "Read the original line
    again, carefully" relevant or helpful?

    > of course all question is eliminated with
    >
    > table = malloc(max * sizeof *table);


    Of course. Just as Ben said in the part that you snipped, just after
    "There is a better way to call malloc that avoids this problem:".

    --
    Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    Nokia
    "We must do something. This is something. Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"
     
    Keith Thompson, Feb 25, 2009
    #11
  12. MN

    Chris Dollin Guest

    MN wrote:

    (I've no idea why the newsreplier has trashed your spaces)

    > table = malloc (sizeof(int*)* max);
    >
    > for (i = 0; i < max; i++)
    > {
    > if (i == 0)
    > table = 0;
    > else
    > {
    > if (i == 1)
    > table= 1;
    > else
    > table = table[i-1]*2;
    > }
    > }


    The initialisation code seems unnecessarily complicated.

    table[0] = 0, table[1] = 1;
    for (i = 2; i < max; i += 1) table = table[i-1] * 2;

    (The comma-expression and += are just hedgehog-stylistic.)

    --
    "Who do you serve, and who do you trust?" Galen, /Crusade/

    Hewlett-Packard Limited registered office: Cain Road, Bracknell,
    registered no: 690597 England Berks RG12 1HN
     
    Chris Dollin, Feb 25, 2009
    #12
  13. MN

    MN Guest

    Thanks to all of you for helping me to correct my code. Now it looks
    like this:

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

    const int max_length = 15;
    const int s_length = 2;


    int* table(int max)
    {
    int i = 0;
    int* table = NULL;

    table = malloc(max * sizeof *table);
    for (i = 0; i < max; i++)
    {
    switch (i)
    {
    case 0: table = 0;
    break;
    case 1:
    table = 1;
    break;
    default:
    table = table[i-1] * 2;
    break;
    }
    }

    return table;
    }

    char** test_foo(int max, int s)
    {
    int i = 0;
    char** test = NULL;

    test = malloc(max * sizeof(*test));

    for (i = 0; i< max; ++i)
    test = malloc((s+1) * sizeof(test));

    for (i = 0; i< max; ++i)
    strcpy(test, "22");

    return test;
    }


    int main (void)
    {
    int* table_tmp = NULL;
    char** test_tmp = NULL;
    int j = 0;

    table_tmp = malloc (max_length * sizeof (*table_tmp));
    table_tmp = table (max_length);

    for (j = 0; j < max_length; ++j)
    printf("table_tmp [%d] = %d\n", j, table_tmp[j]);

    printf("\n");

    test_tmp = malloc (max_length * sizeof *test_tmp);

    for(j = 0; j < max_length; ++j)
    test_tmp[j] = malloc ((s_length+1) * sizeof (*test_tmp[j]) );

    test_tmp = test_foo(max_length, s_length);

    for (j = 0; j < max_length; ++j)
    printf("test_tmp [%d] = %s\n", j, test_tmp[j]);

    // Now all allocated memory ----> here only 50 % is freed,
    why ?
    for (j = 0; j < max_length; ++j)
    free (test_tmp[j]);

    free (test_tmp);
    free (table_tmp);
    return 0;
    }

    But till now I don't know how to free memory allocated in the main
    function.
    I understand that writing code like this:

    for (j = 0; j < max_length; ++j)
    free (test_tmp[j]);

    free (test_tmp);
    free (table_tmp);

    Means that I'm only freeing memory that was allocated in functions
    "test_foo" and "table", but not the one in main function,
    i.e"test_tmp" and "table_tmp".
    This problem will reappear if I must functions "test_foo" and "table"
    in other functions.
     
    MN, Feb 25, 2009
    #13
  14. MN

    Ike Naar Guest

    In article <>,
    MN <> wrote:
    >Thanks to all of you for helping me to correct my code. Now it looks
    >like this:
    >
    >#include <stdio.h>
    >#include <stdlib.h>
    >#include <string.h>
    >
    >const int max_length = 15;
    >const int s_length = 2;
    >
    >
    >int* table(int max)
    >{
    > int i = 0;
    > int* table = NULL;
    >
    > table = malloc(max * sizeof *table);


    Instead of initialising table with NULL and then immediately assigning
    it a meaningful value, you might as well initialise it with the meaningful
    value:

    int *table = malloc(max * sizeof *table);

    In case of failure, malloc retuns a null pointer; it is better
    to check for this.

    > for (i = 0; i < max; i++)
    > {
    > switch (i)
    > {
    > case 0: table = 0;
    > break;
    > case 1:
    > table = 1;
    > break;
    > default:
    > table = table[i-1] * 2;
    > break;
    > }
    > }


    If max is guaranteed to be >=2, this can be simplified to:

    table[0] = 0;
    table[1] = 1;
    for (i = 2; i < max; i++)
    {
    table = table[i-1] * 2;
    }

    >
    > return table;
    >}
    >
    >char** test_foo(int max, int s)
    >{
    > int i = 0;
    > char** test = NULL;
    >
    > test = malloc(max * sizeof(*test));


    Again, you can initialise test with a meaningful value instead
    of initialising it with NULL and then assigning a meaningful value.
    And again, you should check if malloc returs a null pointer.

    >
    > for (i = 0; i< max; ++i)
    > test = malloc((s+1) * sizeof(test));


    This will allocate space for s+1 pointers to char.
    You want to allocate space for s+1 chars:

    test = malloc((s+1) * sizeof *test);

    >
    > for (i = 0; i< max; ++i)
    > strcpy(test, "22");


    This assumes s is at least 2.

    >
    > return test;
    >}
    >
    >
    >int main (void)
    >{
    > int* table_tmp = NULL;
    > char** test_tmp = NULL;
    > int j = 0;
    >
    > table_tmp = malloc (max_length * sizeof (*table_tmp));
    > table_tmp = table (max_length);


    Memory leak; the only pointer to the block returned by malloc()
    is overwritten by the result of the table() call.

    > for (j = 0; j < max_length; ++j)
    > printf("table_tmp [%d] = %d\n", j, table_tmp[j]);
    >
    > printf("\n");
    >
    > test_tmp = malloc (max_length * sizeof *test_tmp);
    >
    > for(j = 0; j < max_length; ++j)
    > test_tmp[j] = malloc ((s_length+1) * sizeof (*test_tmp[j]) );
    >
    > test_tmp = test_foo(max_length, s_length);


    Memory leak; the only pointer to the block returned by the previous
    malloc() is now overwritten by the result of test_foo().

    >
    > for (j = 0; j < max_length; ++j)
    > printf("test_tmp [%d] = %s\n", j, test_tmp[j]);
    >
    > // Now all allocated memory ----> here only 50 % is freed,
    >why ?


    The other 50% is leaked.

    > for (j = 0; j < max_length; ++j)
    > free (test_tmp[j]);
    >
    > free (test_tmp);
    > free (table_tmp);
    > return 0;
    >}
     
    Ike Naar, Feb 25, 2009
    #14
  15. MN <> wrote:
    > Thanks to all of you for helping me to correct my code. Now it looks


    Somewhat better, but still some problems...

    > like this:


    > char** test_foo(int max, int s)
    > {
    > int i = 0;
    > char** test = NULL;


    > test = malloc(max * sizeof(*test));


    > for (i = 0; i< max; ++i)
    > test = malloc((s+1) * sizeof(test));


    Here you forgot the '*' in front of 'test on the right hand side,
    you want the sizeof what 'table' is pointing to (a char), not the
    sizeof what 'table' is (a pointer to char). So make that

    test = malloc((s+1) * sizeof *test);

    > for (i = 0; i< max; ++i)
    > strcpy(test, "22");


    > return test;
    > }


    > int main (void)
    > {
    > int* table_tmp = NULL;
    > char** test_tmp = NULL;
    > int j = 0;


    > table_tmp = malloc (max_length * sizeof (*table_tmp));
    > table_tmp = table (max_length);


    Here's the problem you're mostly interested in, see below.

    > test_tmp = malloc (max_length * sizeof *test_tmp);


    > for(j = 0; j < max_length; ++j)
    > test_tmp[j] = malloc ((s_length+1) * sizeof (*test_tmp[j]) );


    > test_tmp = test_foo(max_length, s_length);


    And here again.

    > for (j = 0; j < max_length; ++j)
    > free (test_tmp[j]);


    > free (test_tmp);
    > free (table_tmp);
    > return 0;
    > }


    > But till now I don't know how to free memory allocated in the main
    > function.


    You can't the way you have written the program. In main() you have

    > table_tmp = malloc (max_length * sizeof (*table_tmp));
    > table_tmp = table (max_length);


    That way you first assign a value to 'table_tmp' that is the
    return value of malloc(), which points to allocated memory of
    'max_length' ints. But in the next line you assign a different
    value to 'table_tmp', irreversibly losing the first value.
    Thus you aren't able anymore to free the memory you obtained
    via your call of malloc() in main(). All you can still free
    is the memory you allocated in function table().

    It's like doing

    int x;
    x = 5;
    x = rand( );

    and afterwards wanting to get at the first value 'x' had (5).
    That simply isn't possible.

    I have no idea why you first allocate memory in main(), assign
    the value to 'table_tmp' and then do another allocation in
    table(), assigning the return value also to 'table_tmp'.
    Since the memory allocated first in main() is never used for
    anything I would guess that the simplest fix would be to
    simple delete the line

    > table_tmp = malloc (max_length * sizeof (*table_tmp));


    This doesn't change anything significant about your program
    and will avoid the memory leak. Or, if you insist on the
    rather useless allocation in main(), do

    > table_tmp = malloc (max_length * sizeof (*table_tmp));

    free( table_tmp );
    > table_tmp = table (max_length);


    to get rid of the allocated memory before you overwrite the
    only pointer you hold to it.

    The third possibiliy, of course, is to use different variables
    for storing the the address of the memory allocated in main()
    and the one allocated in table(). Then you can free both memory
    areas later on:

    table_tmp_1 = malloc (max_length * sizeof *table_tmp_1);
    table_tmp_2 = table (max_length);

    The same holds, of course, also for the second case where
    you allocate memory for 'test_tmp' and then the elements
    of 'test_tmp' and, in the next step, overwrite the value
    of 'test_tmp' with the return value of test_foo(), again
    making it impossible to free the memory you allocated in
    main() first.

    Since it's unclear to me what you intend with your program I
    can't say which of these alternatives is the best .

    > I understand that writing code like this:


    > for (j = 0; j < max_length; ++j)
    > free (test_tmp[j]);


    > free (test_tmp);
    > free (table_tmp);


    > Means that I'm only freeing memory that was allocated in functions
    > "test_foo" and "table", but not the one in main function,
    > i.e"test_tmp" and "table_tmp".


    Fine. And the only solution to avoid that is one of

    a) don't allocate memory in main()
    b) free the memory you allocated in main() before you over-
    write the only pointer you're having to that memory
    c) store the pointer to the memory allocated in main() in
    a different variable, so you can still free it later on.

    Regards, Jens
    --
    \ Jens Thoms Toerring ___
    \__________________________ http://toerring.de
     
    Jens Thoms Toerring, Feb 25, 2009
    #15
  16. MN

    MN Guest

    On Feb 25, 1:36 pm, (Jens Thoms Toerring) wrote:
    > MN <> wrote:
    > > Thanks to all of you for helping me to correct my code. Now it looks

    >
    > Somewhat better, but still some problems...
    >
    > > like this:
    > > char** test_foo(int max, int s)
    > > {
    > >         int i = 0;
    > >         char** test = NULL;
    > >         test = malloc(max * sizeof(*test));
    > >         for (i = 0; i< max; ++i)
    > >                 test = malloc((s+1) * sizeof(test));

    >
    > Here you forgot the '*' in front of 'test on the right hand side,
    > you want the sizeof what 'table' is pointing to (a char), not the
    > sizeof what 'table' is (a pointer to char). So make that
    >
    >                   test = malloc((s+1) * sizeof *test);
    >
    > >         for (i = 0; i< max; ++i)
    > >                 strcpy(test, "22");
    > >         return test;
    > > }
    > > int main (void)
    > > {
    > >         int* table_tmp = NULL;
    > >         char** test_tmp = NULL;
    > >         int j = 0;
    > >         table_tmp = malloc (max_length * sizeof (*table_tmp));
    > >         table_tmp = table (max_length);

    >
    > Here's the problem you're mostly interested in, see below.
    >
    > >         test_tmp = malloc (max_length * sizeof *test_tmp);
    > >         for(j = 0; j < max_length; ++j)
    > >                 test_tmp[j] = malloc ((s_length+1) * sizeof (*test_tmp[j]) );
    > >         test_tmp = test_foo(max_length, s_length);

    >
    > And here again.
    >
    > >         for (j = 0; j < max_length; ++j)
    > >                 free (test_tmp[j]);
    > >         free (test_tmp);
    > >         free (table_tmp);
    > >         return 0;
    > > }
    > > But till now I don't know how to free memory allocated in the main
    > > function.

    >
    > You can't the way you have written the program. In main() you have
    >
    > >         table_tmp = malloc (max_length * sizeof (*table_tmp));
    > >         table_tmp = table (max_length);

    >
    > That way you first assign a value to 'table_tmp' that is the
    > return value of malloc(), which points to allocated memory of
    > 'max_length' ints. But in the next line you assign a different
    > value to 'table_tmp', irreversibly losing the first value.
    > Thus you aren't able anymore to free the memory you obtained
    > via your call of malloc() in main(). All you can still free
    > is the memory you allocated in function table().
    >
    > It's like doing
    >
    > int x;
    > x = 5;
    > x = rand( );
    >
    > and afterwards wanting to get at the first value 'x' had (5).
    > That simply isn't possible.
    >
    > I have no idea why you first allocate memory in main(), assign
    > the value to 'table_tmp' and then do another allocation in
    > table(), assigning the return value also to 'table_tmp'.
    > Since the memory allocated first in main() is never used for
    > anything I would guess that the simplest fix would be to
    > simple delete the line
    >
    > >         table_tmp = malloc (max_length * sizeof (*table_tmp));

    >
    > This doesn't change anything significant about your program
    > and will avoid the memory leak. Or, if you insist on the
    > rather useless allocation in main(), do
    >
    > >         table_tmp = malloc (max_length * sizeof (*table_tmp));

    >
    >           free( table_tmp );
    >
    > >         table_tmp = table (max_length);

    >
    > to get rid of the allocated memory before you overwrite the
    > only pointer you hold to it.
    >
    > The third possibiliy, of course, is to use different variables
    > for storing the the address of the memory allocated in main()
    > and the one allocated in table(). Then you can free both memory
    > areas later on:
    >
    >          table_tmp_1 = malloc (max_length * sizeof *table_tmp_1);
    >          table_tmp_2 = table (max_length);
    >
    > The same holds, of course, also for the second case where
    > you allocate memory for 'test_tmp' and then the elements
    > of 'test_tmp' and, in the next step, overwrite the value
    > of 'test_tmp' with the return value of test_foo(), again
    > making it impossible to free the memory you allocated in
    > main() first.
    >
    > Since it's unclear to me what you intend with your program I
    > can't say which of these alternatives is the best .
    >
    > > I understand that writing code like this:
    > > for (j = 0; j < max_length; ++j)
    > >       free (test_tmp[j]);
    > > free (test_tmp);
    > > free (table_tmp);
    > > Means that I'm only freeing memory that was allocated in functions
    > > "test_foo" and "table", but not the one in main function,
    > > i.e"test_tmp" and "table_tmp".

    >
    > Fine. And the only solution to avoid that is one of
    >
    > a) don't allocate memory in main()
    > b) free the memory you allocated in main() before you over-
    >    write the only pointer you're having to that memory
    > c) store the pointer to the memory allocated in main() in
    >    a different variable, so you can still free it later on.
    >
    >                               Regards, Jens
    > --
    >   \   Jens Thoms Toerring  ___      
    >    \__________________________      http://toerring.de



    Hi again,
    Thanks for your explanation. I decided to use solution a).
    I reworte my code and even addanother function, which call to
    "test_foo" function.
    Now there is no memory leak in my program. Thanks again to of you.

    The new code is:
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>

    const int max_length = 15;
    const int s_length = 2;


    int* table(int max)
    {
    int i = 0;
    int* table = NULL;

    table = malloc(max * sizeof *table);
    if (!table)
    {
    fprintf(stderr, "table is Out of memory\n");
    exit (8);
    }

    for (i = 0; i < max; i++)
    {
    switch (i)
    {
    case 0: table = 0;
    break;
    case 1:
    table = 1;
    break;
    default:
    table = table[i-1] * 2;
    break;
    }
    }
    return table;
    }

    char** test_foo(int max, int s)
    {
    int i = 0;
    char** test = NULL;

    test = malloc(max * sizeof(*test));
    if (!test)
    {
    fprintf(stderr, "test is Out of memory\n");
    exit (8);
    }

    for (i = 0; i< max; ++i)
    {
    test = malloc((s+1) * sizeof(*test));
    if (!test)
    {
    fprintf(stderr, "test[%d] is Out of memory\n", i);
    exit (8);
    }
    }

    for (i = 0; i< max; ++i)
    sprintf(test, "%d", i);
    return test;
    }

    void test_bar(int max, int s)
    {
    char** bar = NULL;
    char** ptr = NULL;

    int k = 0;

    bar = malloc(max * sizeof(*bar));
    if (!bar)
    {
    fprintf(stderr, "bar is Out of memory\n");
    exit (8);
    }

    for (k = 0; k < max; ++k)
    {
    bar[k] = malloc((s+1) * sizeof(*bar[k]));
    if (!bar[k])
    {
    fprintf(stderr, "bar[%d] is Out of memory\n", k);
    exit (8);
    }
    }

    ptr = test_foo(max, s);

    for (k = 0; k < max; ++k)
    {
    strcpy(bar[k], ptr[k]);
    printf("bar[%d] = %s\n",k, bar[k]);
    }
    for (k = 0; k < max; ++k)
    {
    free(ptr[k]);
    free(bar[k]);
    }
    free(ptr);
    free(bar);
    return ;
    }

    int main (void)
    {
    int* table_tmp = NULL;
    char** test_tmp = NULL;
    int j = 0;

    table_tmp = table (max_length);

    for (j = 0; j < max_length; ++j)
    printf("table_tmp [%d] = %d\n", j, table_tmp[j]);
    printf("\n");

    test_tmp = test_foo(max_length, s_length);

    for (j = 0; j < max_length; ++j)
    printf("test_tmp [%d] = %s\n", j, test_tmp[j]);

    test_bar(max_length, s_length);

    // Now all allocated memory is freed,
    for (j = 0; j < max_length; ++j)
    free (test_tmp[j]);

    free (test_tmp);
    free (table_tmp);

    return 0;
    }
     
    MN, Feb 25, 2009
    #16
  17. MN <> writes:

    > for (i = 0; i < max; i++)
    > {
    > switch (i)
    > {
    > case 0: table = 0;
    > break;
    > case 1:
    > table = 1;
    > break;
    > default:
    > table = table[i-1] * 2;
    > break;
    > }
    > }


    I would write the switch as an if because there are really only two
    cases:

    if (i < 2)
    table = i;
    else table = table[i-1] * 2;

    and I would really want to write:

    table = i < 2 ? i : table[i-1] * 2;

    but coding style in force might prevent me. I like conditional
    expressions because they often seem to make more explicit what the
    pattern really is but they seem to be out of favour with bosses.

    --
    Ben.
     
    Ben Bacarisse, Feb 25, 2009
    #17
  18. Chris Dollin <> writes:

    > MN wrote:
    >
    > (I've no idea why the newsreplier has trashed your spaces)
    >
    >> table = malloc (sizeof(int*)* max);
    >>
    >> for (i = 0; i < max; i++)
    >> {
    >> if (i == 0)
    >> table = 0;
    >> else
    >> {
    >> if (i == 1)
    >> table= 1;
    >> else
    >> table = table[i-1]*2;
    >> }
    >> }

    >
    > The initialisation code seems unnecessarily complicated.
    >
    > table[0] = 0, table[1] = 1;


    The OP is presumably concerned about the case of max < 2.

    > for (i = 2; i < max; i += 1) table = table[i-1] * 2;


    --
    Ben.
     
    Ben Bacarisse, Feb 25, 2009
    #18
  19. MN

    Richard Guest

    Ben Bacarisse <> writes:

    > MN <> writes:
    >
    >> for (i = 0; i < max; i++)
    >> {
    >> switch (i)
    >> {
    >> case 0: table = 0;
    >> break;
    >> case 1:
    >> table = 1;
    >> break;
    >> default:
    >> table = table[i-1] * 2;
    >> break;
    >> }
    >> }

    >
    > I would write the switch as an if because there are really only two
    > cases:
    >
    > if (i < 2)
    > table = i;
    > else table = table[i-1] * 2;
    >
    > and I would really want to write:
    >
    > table = i < 2 ? i : table[i-1] * 2;
    >
    > but coding style in force might prevent me. I like conditional
    > expressions because they often seem to make more explicit what the
    > pattern really is but they seem to be out of favour with bosses.


    Which is strange because your use of "?:" is identical and if someone
    can not read that as C then, IMO, they have no place modifying the code.

    It reminds me of the time someone got told off for using *d++=*s++
    because someone thought it was a misuse of C. For me it *IS* C.
     
    Richard, Feb 25, 2009
    #19
  20. MN

    Chris Dollin Guest

    Ben Bacarisse wrote:

    > Chris Dollin <> writes:


    >> The initialisation code seems unnecessarily complicated.
    >>
    >> table[0] = 0, table[1] = 1;

    >
    > The OP is presumably concerned about the case of max < 2.


    Could be.

    if (max > 0) table[0] = 0;
    if (max > 1) table[1] = 1;

    Still better than entangling the initialisation of the first two elements
    into the loop.

    (I'd be tempted to always mallocate at least two elements to ensure things
    always worked, and risk the possibility that malloc would fail with those
    extra two.)

    --
    "- born in the lab under strict supervision -", /Genetesis/

    Hewlett-Packard Limited registered no:
    registered office: Cain Road, Bracknell, Berks RG12 1HN 690597 England
     
    Chris Dollin, Feb 25, 2009
    #20
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Replies:
    5
    Views:
    629
    Matt Wharton
    Dec 9, 2004
  2. jimjim
    Replies:
    28
    Views:
    884
    Michael Wojcik
    Apr 14, 2004
  3. Mr. SweatyFinger
    Replies:
    2
    Views:
    2,078
    Smokey Grindel
    Dec 2, 2006
  4. ravi
    Replies:
    72
    Views:
    1,519
    RCollins
    Sep 14, 2004
  5. Replies:
    5
    Views:
    561
    James Kuyper
    May 25, 2009
Loading...

Share This Page