3
\$\begingroup\$

I am programming a parallel tree algorithm where I have to send data to other processors. The amount of data is not constant. Therefore I have to work with dynamic arrays using malloc() and realloc(). Since I am new to C I wondered if I am doing it right. I do not get any error. But I think that is not proof enough, that it works all the time. How could I really test this code? Do I have memory leaks? Do I occupy to much memory with this method?

Here I have an array containing 10 double values. Then I want add 3 new double values to that array. Finally I want to delete the whole array.

Here is my code:

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

int main(){
    int n = 10;
    double *p;

    p = malloc(sizeof(*p)*n); // similar to int array[n]
    if(p==NULL){
        printf("Error! Memory not allocated.");
        return 1;
    }
    for(int i=0; i<n; i++)
        printf("%lf\n", p[i]);
    printf("\n");

    int add = 3;
    double *temp;
    temp = realloc(p,(n+add)*sizeof(*temp));
    if(temp != NULL){
        p = temp;
    }else{
        free(p);
        printf("Error! Memory not reallocated\n");
        return 1;
    }
    for(int i=0; i<n+add; i++)
        printf("%lf\n", p[i]);

    return 0;
    free(p);
}

I tried to check the size of the arrays before and after reallocation. But it seems to me that this is not possible in C.

\$\endgroup\$
  • 6
    \$\begingroup\$ You shouldn't edit the question to incorporate parts of the answers given. It creates a confusing artefact. \$\endgroup\$ – cmh Jun 28 '17 at 17:52
  • \$\begingroup\$ I rolled back the last edit. Please see /help/someone-answers for what you can and cannot do after receiving answers. \$\endgroup\$ – Mathieu Guindon Jun 28 '17 at 19:37
5
\$\begingroup\$

If you swap the last two lines, so you actually free the memory before returning, then the code looks to be leak-free in the face of errors - well done! You do read uninitialized values from the allocated memory; don't do that even in a test program (Valgrind complains a lot).

Some notes:

This reads easier if you re-order the multiplication:

p = malloc(sizeof(*p)*n); // yours

p = malloc(n * sizeof *p); // mine

(BTW, it's good that you are using sizeof *p rather than sizeof (double), as this means no risk of the type becoming out of step with the size allocated. Definitely a good practice to be encouraged!).

Testing pointers against NULL is more idiomatic if you use the default conversion to boolean:

if(p==NULL){ // yours

if (!p) { // mine

Error messages should go to standard error, not standard output, and should end with a newline:

    printf("Error! Memory not allocated."); // yours

    fprintf(stderr, "Error! Memory not allocated.\n"); // mine

The reallocation can be simplified in a similar manner to the initial allocation; I'd declare and allocate in a single line like this:

double *temp; // yours
temp = realloc(p,(n+add)*sizeof(*temp)); // yours

double *temp = realloc(p, (n+add) * sizeof *temp); // mine

Complete program

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

int main() {
    int n = 10;

    double *p = malloc(n * sizeof *p);
    if (!p) {
        fprintf(stderr, "Error! Memory not allocated.\n");
        return 1;
    }

    /* initialize these values */
    for (int i = 0;  i < n;  ++i)
        p[i] = i;

    for (int i = 0;  i < n;  ++i)
        printf("%lf\n", p[i]);
    printf("\n");

    int add = 3;
    double *temp = realloc(p, (n+add) * sizeof *temp);
    if (temp) {
        p = temp;
    } else {
        free(p);
        fprintf(stderr, "Error! Memory not reallocated.\n");
        return 1;
    }

    /* initialize the new values */
    for (int i = n;  i < n+add;  ++i)
        p[i] = 100 + i;

    for (int i = 0;  i < n+add;  ++i)
        printf("%lf\n", p[i]);

    free(p);
    return 0;
}

If we build this with my usual Makefile and run it in Valgrind, we see no leaks:

gcc -std=c11 -fPIC -g -Wall -Wextra -Wwrite-strings -Wno-parentheses -Wpedantic -Warray-bounds 166874.c -o 166874
valgrind --leak-check=full ./166874
==30432== Memcheck, a memory error detector
==30432== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==30432== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info
==30432== Command: ./166874
==30432== 
0.000000
1.000000
2.000000
3.000000
4.000000
5.000000
6.000000
7.000000
8.000000
9.000000

0.000000
1.000000
2.000000
3.000000
4.000000
5.000000
6.000000
7.000000
8.000000
9.000000
110.000000
111.000000
112.000000
==30432== 
==30432== HEAP SUMMARY:
==30432==     in use at exit: 0 bytes in 0 blocks
==30432==   total heap usage: 3 allocs, 3 frees, 1,208 bytes allocated
==30432== 
==30432== All heap blocks were freed -- no leaks are possible
==30432== 
==30432== For counts of detected and suppressed errors, rerun with: -v
==30432== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
\$\endgroup\$
  • \$\begingroup\$ Great! How do I use the Memcheck? I am using Linux and gcc. \$\endgroup\$ – Samuel Jun 28 '17 at 17:48
  • 2
    \$\begingroup\$ Why did you remove the parens from the argument to sizeof? \$\endgroup\$ – Tavian Barnes Jun 28 '17 at 18:06
  • \$\begingroup\$ Technically if the "argument" to sizeof is an expression, then it is legal to omit the parentheses. The parentheses are only required when the "argument" is a type. So, sizeof *p and sizeof *temp are totally legal. But yeah, for readability purposes, always write the parens! @cat \$\endgroup\$ – Cody Gray Jun 28 '17 at 19:29
  • \$\begingroup\$ But you also said, "This does not compile". It does compile. \$\endgroup\$ – Cody Gray Jun 28 '17 at 19:33
  • 1
    \$\begingroup\$ @Samuel The tool is called valgrind, and it is one of the most useful tools for c programmers around. If you're using linux, chances are you already have it installed (and if not, it is very likely conveniently available from your package manager of choice). On how to use, you could begin by reading its quick start guide. \$\endgroup\$ – Ben Steffan Jun 28 '17 at 20:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.