14

I want a simple function that receives a string and returns an array of strings after some parsing. So, this is my function signature:

int parse(const char *foo, char **sep_foo, int *sep_foo_qty) {
    int i;
    char *token;
    ...
    strcpy(sep_foo[i], token); /* sf here */
    ...
}

Then I call it like this:

char sep_foo[MAX_QTY][MAX_STRING_LENGTH];
char foo[MAX_STRING_LENGTH];
int sep_foo_qty, error;

...

error = parse(foo, sep_foo, &sep_foo_qyt);

...

This way I get a warning during compilation:

warning: passing argument 2 of 'parse' from incompatible pointer type

And then a segmentation fault during execution in the line marked with /* sf here */

What is wrong in my C code?

Thanks in advance

Mark Ransom
  • 299,747
  • 42
  • 398
  • 622
mmutilva
  • 18,688
  • 22
  • 59
  • 82

4 Answers4

28

The warning is exactly right. Your function wants an array of pointers. You're giving it an array of arrays.

Expected:

 sep_foo:
 +------+       +-----+
 |char**|--> 0: |char*|-->"string1"
 +------+       +-----+
             1: |char*|-->"string2"
                +-----+
*sep_foo_qty-1: |...  |
                +-----+

What you provided:

           sep_foo:
           +--------------------------------+
        0: | char[MAX_STRING_LENGTH]        |
           +--------------------------------+
        1: | char[MAX_STRING_LENGTH]        |
           +--------------------------------+
MAX_QTY-1: | ...                            |
           +--------------------------------+

An array with elements of type X can "decay" into a pointer-to-X, or X*. But the value of X isn't allowed to change in that conversion. Only one decay operation is allowed. You'd need it to happen twice. In your case, X is array-of-MAX_STRING_LENGTH-chars. The function wants X to be pointer-to-char. Since those aren't the same, the compiler warns you. I'm a bit surprised it was just a warning since nothing good can come from what the compiler allowed to happen.

In your function, you could write this code:

char* y = NULL;
*sep_foo = y;

That's legal code since sep_foo is a char**, so *sep_foo is a char*, and so is y; you can assign them. But with what you tried to do, *sep_foo wouldn't really be a char*; it would be pointing to an array of char. Your code, in effect, would be attempting to do this:

char destination[MAX_STRING_LENGTH];
char* y = NULL;
destination = y;

You can't assign a pointer into an array, and so the compiler warns that the call is no good.

There are two ways to solve this:

  • Change the way you declare and allocate sep_foo on the calling side so it matches what the function expects to receive:

    char** sep_foo = calloc(MAX_QTY, sizeof(char*));
    for (int i = 0; i < MAX_QTY; ++i)
      sep_foo[i] = malloc(MAX_STRING_LENGTH);
    

    or, equivalently

    char* sep_foo[MAX_QTY];
    for (int i = 0; i < MAX_QTY; ++i)
      sep_foo[i] = malloc(MAX_STRING_LENGTH);
    
  • Change the prototype of the function to accept what you're really giving it:

    int parse(const char *foo, char sep_foo[MAX_QTY][MAX_STRING_LENGTH], int *sep_foo_qty);
    
Rob Kennedy
  • 161,384
  • 21
  • 275
  • 467
  • @che, An array can be converted to a pointer by taking the pointer of its first element (ptr = &array[0]). This can be done implicitly (ptr = array). The reverse is not true. – strager Jan 28 '09 at 01:19
  • Also, when you take the pointer of an array (ptr = &ar), ptr contains the address of the array as a whole. ptr would then be equal to &ar[0], in general, except the type is different. If you took the pointer of a pointer, you would get a number which, is not equal to that pointer (memory is ref'd). – strager Jan 28 '09 at 01:22
  • On your edit... You did a much better job at a diagram than I did. I'm the opposite of artistic. =] – strager Jan 28 '09 at 01:28
  • Rob, i like you answer very much. i also like how that answer nicely complements my recent pet peeve: http://stackoverflow.com/questions/423823/whats-your-favorite-programmer-ignorance-pet-peeve#484900 . I will put a link on that to your answer. have fun :) – Johannes Schaub - litb Jan 28 '09 at 02:51
  • Rob, I think you mistyped the variable name: sep_foo_qty is int*, sep_foo is char**. Please edit your answer because is really good but a bit confusing there. – mmutilva Jan 28 '09 at 16:42
  • Moreover, the explaination is very good, but you didn't provide the correct code to use as codelogic did. – mmutilva Jan 28 '09 at 16:43
  • Toto, he asked what's wrong, not how to fix it. I don't know what he really wanted, so I couldn't fix it. The function as shown doesn't "return an array of strings," but that's what he said it does. The prototype would return an array, but as called, it would *fill* an array. Which is right? – Rob Kennedy Jan 28 '09 at 16:57
  • Rob, I'm actually the one who asked the question in the first place. You are right, I did not asked 'how to fix it' i thought it was implicit ;). What I wanted was both things: a good explination as yours and the solution. I still don't understand why you talk about sep_foo_ty insetead of sep_foo – mmutilva Jan 29 '09 at 14:57
  • In my code sep_foo_qty is just an int * i use to return the large of the resulting array sep_foo (because the int returned by the function is an error code). Pleas clarify your answer with respect to that and if you also put the correct code this will become the accepted answer ;) – mmutilva Jan 29 '09 at 15:00
  • OK, @Alexander. I'm going to delete the comments I've made today and start over. Which part of my answer was unclear and prompted you to have to ask your question? – Rob Kennedy Sep 04 '14 at 18:29
  • The part that says "Your function wants an array of pointers", whereas it wants "char **sep_foo". – Incerteza Sep 04 '14 at 18:31
  • The `char**` argument in this function expects to receive an array of pointers, @Alexander. More precisely, it should be a *pointer to* an array of pointers, but there's no other way to pass such a thing, so the first level of pointer is implicit. Specifically, the pointers in the array are meant to point at arrays of `char`. The picture illustrates exactly that. – Rob Kennedy Sep 04 '14 at 18:38
  • Why an array of pointers? It expects to receive a pointer to a pointer of char, doesn't it? – Incerteza Sep 05 '14 at 03:31
  • Because the question says so, @Alexander. The function is declared to receive a pointer to a pointer to `char`, and its description is such that the first pointer to `char` may be followed by additional pointers to `char`, thus yielding an array. What did you expect instead? Perhaps you're not realizing that pointer-to-X and pointer-to-array-of-X are represented identically? – Rob Kennedy Sep 05 '14 at 03:42
15

Parameter 2 should be

char sep_foo[][MAX_STRING_LENGTH]

To clarify, you are passing a pointer to parse() and treating it as a pointer to a pointer. A multidimensional array in C is NOT an array of pointers. It is a single block of memory that is pointed to by the array variable. You cannot dereference it twice.

codelogic
  • 71,764
  • 9
  • 59
  • 54
  • 1
    I've been trying to understand strager's memory models, but you've nailed the difference in three clear and simple sentences. Thanks for the clarification! – che Jan 28 '09 at 01:47
4

sep_foo is defined as an array of arrays. In other words, when you use sep_foo, it points to the beginning of sequential memory. Here's a model:

(assume MAX_STRING_LENGTH = 16, MAX_QTY = 2)
sep_foo       = &&0000
sep_foo[0]    =  &0000
sep_foo[0][0] = *&0000 = 12
sep_foo[0][8] = *&0008 = 74
sep_foo[1]    =  &0010
sep_foo[1][0] = *&0010 = 12


0000  12 34 56 78  9A BC DE F0  74 10 25 89  63 AC DB FE
0010  12 34 56 78  9A BC DE F0  74 10 25 89  63 AC DB FE

However, your function expects an array of pointers (actually, a pointer to a pointer). This is modeled as such:

sep_foo_arg       =   &&0000
sep_foo_arg[0]    =  *&&0000 = &0010
sep_foo_arg[0][0] =  *&*&0000 = 12
sep_foo_arg[0][8] = *(&*&0000 + 8) = 74
sep_foo_arg[1]    =  *&&0002 = &0020
sep_foo_arg[1][0] = *&*&0000 = 12

0000  0010 0020  xxxx xxxx  xxxx xxxx  xxxx xxxx

0010  12 34 56 78  9A BC DE F0  74 10 25 89  63 AC DB FE
0020  12 34 56 78  9A BC DE F0  74 10 25 89  63 AC DB FE

Yeah ... Syntax may be a bit confusing for my explanations...

Anyway, you can solve this issue by telling your function how to treat the pointer pointed to. In particular, you would want to treat it as an array (a sequence of memory):

int parse(const char *foo, char (*sep_foo)[MAX_STRING_LENGTH], int *sep_foo_qty);
strager
  • 88,763
  • 26
  • 134
  • 176
  • strager, didn't you mean char (*sep_foo)[MAX_STRING_LENGTH] ? because as it is now, it's still a "char**". looks like a typo to me. – Johannes Schaub - litb Jan 28 '09 at 03:11
  • @litb, Ah, right. Normally I look this up, but I was in a hurry when I wrote it (couldn't you tell? =]). Thanks. – strager Jan 28 '09 at 03:34
  • @litb, Wait -- did YOU make a typo? I don't know which one is right any more. @_@ Some testing is in order later. – strager Jan 28 '09 at 03:35
  • i'm sorry i see i sounded confusing. well i mean you made teh typo :) look: T *f[N] is an array of T*. T (*f)[N] is a pointer to an array of T (what you wanted). the * binds more to the T than to the parameter name, so you have to paren it to the f, so that f becomes a pointer. – Johannes Schaub - litb Jan 28 '09 at 03:51
  • because if you say "T *f[N]", it is the same thing as "T **f" in the parameter list. i don't believe you meant that. – Johannes Schaub - litb Jan 28 '09 at 03:53
  • this is why the other guy could write "T f[][some_number]": the "[]" is just dropped, and the resulting parameter is a pointer to that thing: "T f[some_number]" => "T (*f)[some_number]". for one dimension might be clearer: "T f[]" = "T f" => "T *f". instead of [] u can write [X]: X is ignored. – Johannes Schaub - litb Jan 28 '09 at 03:58
  • it's like when you use function pointers: "int *f()" would mean a function that returns an int* (and in parameter lists means a function pointer to a function that returns an int*). but "int (*f)()" makes it a function pointer to a function returning int. – Johannes Schaub - litb Jan 28 '09 at 04:05
  • Not sure exactly why it is char (*f)[n] and not char *(f[n]). Wait ... now I see (while typing this)! Thanks for clearing that up, litb! – strager Jan 28 '09 at 18:58
  • Does `char (*sep_foo)[MAX_STRING_LENGTH]` mean "an array of the pointers to char"? Are the parenthesis needed there? – Incerteza Sep 04 '14 at 17:23
-2

If that is your exact code, then I'm guessing the segfault is because of the fact that you haven't allocated memory to the char* token inside your parse function, and then using that in your strcpy.

sykora
  • 96,888
  • 11
  • 64
  • 71
  • I don't need to allocate memroy for char *token as I'm using as the result of strtok_r function: token = strtok_r(copied_foo, "/", &save_ptr); ... token = strtok_r(NULL, "/", &save_ptr); Thanks – mmutilva Jan 28 '09 at 01:16