Damian
Damian

Reputation: 3050

using pointer to array of chars

I want to pass a pointer to pointer to a function, allocate memory in function, fill it with strings and get it back, but all seems not to be working. Program prints nothing outside the function. There is most important pieces of code:

struct record ** getRegEx( int *counter, char** keys )
{
    *counter = 0;
   //get some records, its number is *counter, max lenght of each string is 64

   //COUNTER IS NOT 0! ITS VALUE DEPENDS ON OTHER OPERATIONS I HAVENT WRTTEN HERE
   //...        
    keys =(char ** ) malloc((*counter)*(sizeof(char *)));
    for (j = 0; j < *counter; j++)
    {
      keys[j] = (char* )malloc(64*sizeof(char));            
    }

    strcpy(keys[j],key.dptr);

    printf("size %d : \n", sizeof(**keys));//1
    printf("size %d : \n", sizeof(*keys));//4
    printf("size %d : \n", sizeof(keys[0]));//4
    printf("size %d : \n", sizeof(keys));//4    
   //...
}


/*Out of the function, inside the function OK*/
char**   keys;
int count;  
results = getRegEx(&count, &keys); //&keys or keys - makes no difference
for(int k=0 ; k< *count;k++) //test
{
    printf("keys in db %s: s\n", keys[k]); //nothing!?
}

I have made it works by replacing function header with something like struct record ** getRegEx( int *counter, char*** keys ) (and using *keys and *keys[i] instead of key and keys[i] inside the function). Thanks for All!

Upvotes: 0

Views: 258

Answers (4)

John Bode
John Bode

Reputation: 123598

Major issues:

  1. Your types don't match up. In the call to getRegEx(&count, &keys), the type of the expression &keys is char ***, not char **. Note that if you want to modify the value of keys, you will have to pass a pointer to it. This leads to the next problem...

  2. Because you're modifying the value of the parameter keys, not what it points to, any changes you make in the function are not reflected in the caller.

Here's a modified version of your code, illustrating what I think you're trying to do:

struct record **getRegEx(int *counter, char ***keys)
{
   ...
   // type of keys = char ***
   // type of *keys = char **
   // type of **keys = char *

   *keys = malloc(*counter * sizeof **keys); // note no cast, operand of sizeof
   if (*keys)
   {
     int j;
     for (j = 0; j < *counter; j++)
     {
       // type of (*keys)[j] == char *
       // type of *(*keys)[j] = char

       (*keys)[j] = malloc(64 * sizeof *(*keys)[j]); 
     }
   }
   ...
}

Note that I don't cast the result of malloc. As of the 1989 version of the C standard, you don't have to, so it saves from visual clutter. It also protects you from a potential bug; if you forget to include stdlib.h or otherwise don't have to prototype for malloc in scope, the compiler will assume the function returns int. Without the cast, you'll get diagnostic on the order of "incompatible types for assignment." Adding the cast will supress the diagnostic, and you may have subtle (or not-so-subtle) runtime errors as a result.

Also, note that I'm using sizeof on objects, not types. Again, this helps reduce visual clutter, and it also protects you in case you decide to change the base type of keys; you don't have to update every malloc call as well.

Why (*keys)[j] instead of *keys[j]? The expression keys is not the location of the beginning of our array, but rather points to that location. We have to dereference keys to get the address of the array, which we then subscript.

Upvotes: 1

Chris Lutz
Chris Lutz

Reputation: 75469

  1. There's a serious problem here:

    results = getRegEx(&count, &keys); //&keys or keys - makes no difference
    

    Your comment is wrong - it does make a difference. keys is of type char ** (which getRegEx expects), &keys is of type char ***.

  2. Your function has a return type, but returns nothing.

  3. You allocate dynamic memory for your keys variable in the function, but the function (as it's written) cannot pass that memory out of the function. Your function should take a char ***, and you should pass it as &keys (which, as previously stated, is of type char ***.)

  4. Your size is always going to be zero, since you set *count = 0 at the beginning of your function (when you shouldn't be setting it at all in your function, and should be passing count by value instead of by pointer). The exact effects of malloc(0) are implementation defined.

  5. You cast the return value of malloc. This isn't wrong, but it's unnecessary in C (and if you're really using C++, you should say so) and can make things harder down the road.

  6. You never check the return values of malloc for failure.

  7. You use the counter pointer outside the function where it is declared. Outside the function call, you should be using the count variable you passed as a parameter. Function parameters don't continue to exist outside the functions they're used in.

Upvotes: 2

abelenky
abelenky

Reputation: 64730

You have a function getRegEx, that is declared to return a struct record **.

You know what's missing from that function?
anything that looks like a return statement!

Upvotes: 0

Doug Currie
Doug Currie

Reputation: 41220

You are passing zero to malloc so you have nothing to return in keys.

Your for loop never runs.

Both are because (*counter) is zero.

Upvotes: 3

Related Questions