Returning array of int in C

Question

I have this code:

 1 #include <stdio.h>                                                               
 2 #include <string.h>                                                              
 3 #define LENGTH(a) sizeof(a)/sizeof(a[0]);                                        
 4                                                                                  
 5 int *getNbits(int, int, char *);                                                 
 6                                                                                  
 7 int main() {                                                                     
 8    char ins[] = "00001111000011110000111100001111";                              
 9    int *x = getNbits(0, 32, ins);                                                
 10                                                                                  
 11    for (int i = 0; i < LENGTH(x) ; i++){                                                
 12                                                                                  
 13       printf("%d", *(x + i));                                                        
 14    }                                                                             
 15    return 0;                                                                     
 16 }                                                                                
 17                                                                                  
 18 int* getNbits(int start, int offset, char instr[33]) {                           
 19                                                                                  
 20    int result[offset - 1];                                                       
 21    for (int i = 0; i < offset; i++) {                                            
 22        result[i] = instr[i + start] == '1' ? 1 : 0;   //- '0';                   
 23    }                                                                             
 24    return result;                                                                
 25 }         

Basically, getNbits() gets an array of chars (which are 0's or 1's) and returns an array of ints, each element is a 0 or a 1.

If I try to print the array "result" as I am creating it in the for loop (using printf("%d",result[i]) ) it will return 000011110000.....

But I am having trouble with the return type, it gives me a warning: function returns address of local variable. And the for loop in main just prints garbage values.

Answer 1

The result variable is local to getNbits. This means it can be deallocated as soon as the function returns.

Don't return the address of a LOCAL variable as a pointer - it can and will be deallocated.

Instead, allocate the integers like this:

int* retVal =  malloc(sizeof(int) * 10);

Then return retVal from getNbits.

See this URL:

http://ww2.cs.mu.oz.au/~mgiuca/253/malloc.html

NOTE: If you use malloc above, you must also use free to release the allocated memory after it is no longer needed:

/* When the array is no longer needed */ 
free(x);

At the point of the free, the variable name is x because that was the lvalue assigned from the call to getNbits. Don't call free from getNbits.

Answer 2

int* getNbits(int start, int offset, char instr[33]) {                           
   int result[offset - 1];                                                       
   for (int i = 0; i < offset; i++) {                                            
      result[i] = instr[i + start] == '1' ? 1 : 0;   //- '0';                   
   }                                                                             
   return result;     
}

The int array result is declaired (by default) on the stack. This causes it to have a scope (lifespan) limited to inside the getNbits() function. Passing a reference to this array outside the function is not a good idea; as the stack space where the array formally resided will be used for another purpose.

The same operation might be better accomplished by allocating the array on the heap:

int* getNbits(int start, int offset, char instr[33]) {                           
   int *result = malloc(offset * sizeof(*result));  // As indicated by BLUEPIXY's comment
   if(NULL == result)
      {
      fprintf(stderr, "malloc() failed\n");
      goto CLEANUP;
      }

   for (int i = 0; i < offset; i++) {                                            
      result[i] = instr[i + start] == '1' ? 1 : 0;   //- '0';                   
   }    

CLEANUP:                                                                         
   return result;     
}

Of course, in the above case, the caller of getNbits() will have to remember to call free() on the returned array when it is no longer needed to return the allocated memory back to the heap.

Answer 3

It's simpler and less error prone, to allocate the array in main, where you call the results array.

int result[ sizeof ins];    /* Size known at compile time */

getNbits(0, 32, ins, result);  /* Pass arrays by reference */

void getNbits(int start, int offset, const char instr[], int result[]) {

That's using automatic storage on stack which is automatically recycled when a routine returns. As the code in questions stands, printf's variables allocated on the stack, will overwrite the values pointed to by result.

An advantage is generalising the getNbits subroutine, so it would work with longer length versions of "ins". Of course a real program, would have an error status returned by getNbits, to catch errors.

Not good practice to malloc, memory in a function, and have the caller responsible for freeing, it's likely to leak or be freed later multiple times, with more complicated programs and these are often hard to track down bugs.

Traditional code often defined structs or arrays, passed from functions like int result[] as static storage, so the address was not on the stack. That worked, but is not thread safe, and any values stored in data structures had to be copied anyway.

If you do write a library functions that dynamically constructs a data structure, then pair it with a routine to tidy up, rather than leak implementation details into the callers program!