char *arr[n] Vs. char **arr

[yogeshmk]

I need to break a longer string (with strtok()) and store the smaller
strings in an array. since the number of small strings is not
fixed/known, I cannot use a declaration like char *format[n], so I
declared char** format = NULL;

Now the problem starts. Each time strtok() returns me a token (which is
char*), I call malloc() and I (want to) store the address returned in
the array format[], and copy the token to that address.

code snippet:
------------------
char **format = NULL;
char delim[] = ".-/:";
int i=0;

if ((tok = strtok(argv[1], delim)) != NULL)
{
if((format=(char*)malloc(strlen(tok))) != NULL)
strcpy(format+i, tok);
}

for (i=1; tok; i++)
{
if ((tok = strtok(NULL, delim)) != NULL)
if((format=(char*)malloc(strlen(tok))) != NULL)
strcpy(format+i, tok);
}

The malloc statement gives a segfault. In desparation, I have tried
various combinations of lvalue in malloc, but none of them were correct
(either I get a compiler warning like "invalid lvalue", or "assignment
makes an integer from pointer" etc.. or if compilation is successful,
run always fails with a segfault)

Q: Is above approach fundamentally incorrect? Can I declare something
as a pointer to pointer to char & treat like an array of strings? If
yes, what's wrong in above code? I have no hesitation in admitting that
my pointer fundamentals are not very clear yet -
~yogesh

 

[raxitsheth2000]

Yogesh,

if((format=(char*)malloc(strlen(tok))) != NULL)
strcpy(format+i, tok);

For sake of simplicity if we assume format is 2-d array, i.e. array of
String, where string itself is array of char.


you are allocating memory @ ith element of format array. (line 1,
above)
you are copying the string @ format+i ---> format[0]+i , i.e. on 1st
location.

I am not much more sure. but this may be the likely catch.

yes, what's wrong in above code? I have no hesitation in admitting that
my pointer fundamentals are not very clear yet -
~yogesh

Raxit

 

[Richard Bos]

I need to break a longer string (with strtok()) and store the smaller
strings in an array.

This in itself has its snares; for example, you do know that strtok()
demolishes the string it works on, don't you?

char **format = NULL;
char delim[] = ".-/:";
int i=0;

if ((tok = strtok(argv[1], delim)) != NULL)
{
if((format=(char*)malloc(strlen(tok))) != NULL)

Two problems in this line. First, there is no need to cast malloc(), and
in fact doing so can hide the error of forgetting to #include
<stdlib.h>, which causes undefined behaviour.
Second, you want to allocate memory for the string _including_ its
terminating null character.
So this line should be:

if ((format=malloc(strlen(tok)+1)) != NULL)

Ditto later on.


The major problem, however, is that you do allocate memory for the
individual strings, but not for the array (format) that contains all the
pointers themselves. So while would you copy the strings OK, everything
you write _to_ format itself goes to limbo.

This FAQ might help: <http://c-faq.com/aryptr/dynmuldimary.html>,
although in your case you don't know in advance how many strings you
have so you'd have to use realloc() on format as well.

Richard

 

[yogeshmk]

This in itself has its snares; for example, you do know that strtok()
demolishes the string it works on, don't you?

you caught me- (but I'm not going to use argv[1] again so it should
not be a problem)

char **format = NULL;
char delim[] = ".-/:";
int i=0;

if ((tok = strtok(argv[1], delim)) != NULL)
{
if((format=(char*)malloc(strlen(tok))) != NULL)

Two problems in this line. First, there is no need to cast malloc(), and
in fact doing so can hide the error of forgetting to #include
<stdlib.h>, which causes undefined behaviour.

I did not understand the above point.

Second, you want to allocate memory for the string _including_ its
terminating null character.
So this line should be:

if ((format=malloc(strlen(tok)+1)) != NULL)

Ditto later on.


The major problem, however, is that you do allocate memory for the
individual strings, but not for the array (format) that contains all the
pointers themselves. So while would you copy the strings OK, everything
you write _to_ format itself goes to limbo.

This FAQ might help: <http://c-faq.com/aryptr/dynmuldimary.html>,
although in your case you don't know in advance how many strings you
have so you'd have to use realloc() on format as well.

Richard

I'm pasting the changed code

if ((tok = strtok(argv[1], delim)) != NULL)
{
format = (char**)malloc(1*sizeof(char*));
if((format=(char*)malloc(strlen(tok)+1)) != NULL)
strcpy((char*)format+i, tok);
}
/*
* gdb shows the first token copied at 0th location of format .
*/
for (i=1; tok; i++)
{
if ((tok = strtok(NULL, delim)) != NULL)
if (format = (char**)realloc(format, (i+1)*sizeof(char*))
!= NULL)
if((format=(char*)malloc(strlen(tok)+1)) != NULL)
strcpy((char*)format+i, tok);
}

realloc() changes the address of format (something like 0x1) and
subsequent malloc() fails giving a segfault. Still scratching my head!
What's wrong in the code?
~yogesh

 

[deepak]

I need to break a longer string (with strtok()) and store the smaller
strings in an array. since the number of small strings is not
fixed/known, I cannot use a declaration like char *format[n], so I
declared char** format = NULL;

Now the problem starts. Each time strtok() returns me a token (which is
char*), I call malloc() and I (want to) store the address returned in
the array format[], and copy the token to that address.

code snippet:
------------------
char **format = NULL;
char delim[] = ".-/:";
int i=0;

if ((tok = strtok(argv[1], delim)) != NULL)
{
if((format=(char*)malloc(strlen(tok))) != NULL)
strcpy(format+i, tok);
}

for (i=1; tok; i++)
{
if ((tok = strtok(NULL, delim)) != NULL)
if((format=(char*)malloc(strlen(tok))) != NULL)
strcpy(format+i, tok);
}

The malloc statement gives a segfault. In desparation, I have tried
various combinations of lvalue in malloc, but none of them were correct
(either I get a compiler warning like "invalid lvalue", or "assignment
makes an integer from pointer" etc.. or if compilation is successful,
run always fails with a segfault)

Q: Is above approach fundamentally incorrect? Can I declare something
as a pointer to pointer to char & treat like an array of strings? If
yes, what's wrong in above code? I have no hesitation in admitting that
my pointer fundamentals are not very clear yet -

Yogesh.
You didn't allocated memory for double pointer.
You are mallocing only for the strings.

This will solve the problem.
test_string = (char **) malloc (sizeof(char *) * n);

But it will lead to the issue of the size of row.

 

[deepak]

I need to break a longer string (with strtok()) and store the smaller
strings in an array. since the number of small strings is not
fixed/known, I cannot use a declaration like char *format[n], so I
declared char** format = NULL;

Now the problem starts. Each time strtok() returns me a token (which is
char*), I call malloc() and I (want to) store the address returned in
the array format[], and copy the token to that address.

code snippet:
------------------
char **format = NULL;
char delim[] = ".-/:";
int i=0;

if ((tok = strtok(argv[1], delim)) != NULL)
{
if((format=(char*)malloc(strlen(tok))) != NULL)
strcpy(format+i, tok);
}

for (i=1; tok; i++)
{
if ((tok = strtok(NULL, delim)) != NULL)
if((format=(char*)malloc(strlen(tok))) != NULL)
strcpy(format+i, tok);
}

The malloc statement gives a segfault. In desparation, I have tried
various combinations of lvalue in malloc, but none of them were correct
(either I get a compiler warning like "invalid lvalue", or "assignment
makes an integer from pointer" etc.. or if compilation is successful,
run always fails with a segfault)

Q: Is above approach fundamentally incorrect? Can I declare something
as a pointer to pointer to char & treat like an array of strings? If
yes, what's wrong in above code? I have no hesitation in admitting that
my pointer fundamentals are not very clear yet -

Try this code.
It's just a casual one. I din't freed malloced variable etc.

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

main(int argc, char *argv[])
{
/* Copy the constant into the memory
* pinted to by 'test_string'
*/
char **test_string;

/* if 'test_string' is declared as below and the program will
give a
* 'Segmentation fault' This is because test_string' is
pointing
* to a constant i.e. somethin that cant be changed.

char *test_string="string to split up"; */

char *sub_string;
int i=0;
/* Extract first string */

test_string = (char **) malloc (sizeof(char *) * 10);

if ((sub_string = strtok(argv[1], "a")) != NULL){
*(test_string + i) = (char *) malloc (strlen(sub_string) + 1);
*(test_string + i) = sub_string;
printf ("%s\n", *test_string);
}
i++;
/* Extract remaining
* strings */
while ( (sub_string=strtok(NULL, "a")) != NULL)
{
*(test_string + i) = (char *) malloc (strlen(sub_string) + 1);
*(test_string + i) = sub_string;
printf("%s\t%s\n", *(test_string + i));
i++;
}
}

 

[Barry Schwarz]

I need to break a longer string (with strtok()) and store the smaller
strings in an array. since the number of small strings is not
fixed/known, I cannot use a declaration like char *format[n], so I
declared char** format = NULL;

Now the problem starts. Each time strtok() returns me a token (which is
char*), I call malloc() and I (want to) store the address returned in
the array format[], and copy the token to that address.

code snippet:
------------------
char **format = NULL;
char delim[] = ".-/:";
int i=0;

if ((tok = strtok(argv[1], delim)) != NULL)
{
if((format=(char*)malloc(strlen(tok))) != NULL)
strcpy(format+i, tok);
}

for (i=1; tok; i++)
{
if ((tok = strtok(NULL, delim)) != NULL)
if((format=(char*)malloc(strlen(tok))) != NULL)
strcpy(format+i, tok);
}

The malloc statement gives a segfault. In desparation, I have tried
various combinations of lvalue in malloc, but none of them were correct
(either I get a compiler warning like "invalid lvalue", or "assignment
makes an integer from pointer" etc.. or if compilation is successful,
run always fails with a segfault)

Q: Is above approach fundamentally incorrect? Can I declare something
as a pointer to pointer to char & treat like an array of strings? If
yes, what's wrong in above code? I have no hesitation in admitting that
my pointer fundamentals are not very clear yet -

Try this code.
It's just a casual one. I din't freed malloced variable etc.

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

main(int argc, char *argv[])
{
/* Copy the constant into the memory
* pinted to by 'test_string'
*/
char **test_string;

/* if 'test_string' is declared as below and the program will
give a
* 'Segmentation fault' This is because test_string' is
pointing
* to a constant i.e. somethin that cant be changed.

char *test_string="string to split up"; */

char *sub_string;
int i=0;
/* Extract first string */

test_string = (char **) malloc (sizeof(char *) * 10);

Don't cast the return from malloc. It never helps and can hide a
diagnostic regarding undefined behavior which you would really want to
see.

if ((sub_string = strtok(argv[1], "a")) != NULL){
*(test_string + i) = (char *) malloc (strlen(sub_string) + 1);

While *(test_string + i) is correct, the equivalent test_string is
the more common and preferred idiom. But i is always zero in this
block of code anyway.

*(test_string + i) = sub_string;

Definitely not. This causes a memory leak by destroying the only
pointer to the allocated memory. If you went to all the trouble to
allocate space for the string pointed to by sub_string, then you want
strcpy(test_string, sub_string);

printf ("%s\n", *test_string);
}
i++;
/* Extract remaining
* strings */
while ( (sub_string=strtok(NULL, "a")) != NULL)
{
*(test_string + i) = (char *) malloc (strlen(sub_string) + 1);
*(test_string + i) = sub_string;
printf("%s\t%s\n", *(test_string + i));
i++;
}
}

Remove del for email

 

[Barry Schwarz]

I need to break a longer string (with strtok()) and store the smaller
strings in an array. since the number of small strings is not
fixed/known, I cannot use a declaration like char *format[n], so I
declared char** format = NULL;

Now the problem starts. Each time strtok() returns me a token (which is
char*), I call malloc() and I (want to) store the address returned in
the array format[], and copy the token to that address.

As you note in a subsequent message, you don't use the original string
(in argv[1]) after strtok tokenizes it so the question is why copy
each token when you could use it in place.

code snippet:
------------------
char **format = NULL;
char delim[] = ".-/:";
int i=0;

if ((tok = strtok(argv[1], delim)) != NULL)
{
if((format=(char*)malloc(strlen(tok))) != NULL)

format requires format to be dereferenced. Where does format point
to?

strlen does not count the terminating '\0' but strcpy does copy it.
You are allocating one byte too few.

Don't cast the return from malloc. It never helps and can hide a
diagnostic regarding undefined behavior which you would really want to
see.

strcpy(format+i, tok);

The address of the allocated area is in format. format+i is a
completely different location in memory. If you meant *(format+i), I
recommend using the syntactically equivalent but much easier to read
format.

}

for (i=1; tok; i++)
{
if ((tok = strtok(NULL, delim)) != NULL)
if((format=(char*)malloc(strlen(tok))) != NULL)
strcpy(format+i, tok);
}

The malloc statement gives a segfault. In desparation, I have tried
various combinations of lvalue in malloc, but none of them were correct
(either I get a compiler warning like "invalid lvalue", or "assignment
makes an integer from pointer" etc.. or if compilation is successful,
run always fails with a segfault)

A desirable result since format was never initialized to point
somewhere. The best kind of undefined behavior is the kind that stops
your program when the behavior occurs rather than letting things
proceed till something else goes wrong.

Q: Is above approach fundamentally incorrect? Can I declare something

The approach is OK, you just missed some details.

as a pointer to pointer to char & treat like an array of strings? If

Yes, after you initialize it properly.

yes, what's wrong in above code? I have no hesitation in admitting that
my pointer fundamentals are not very clear yet -
~yogesh

You need to allocate space for format to point to N (some number) of
objects of type *format.

As you tokenize each string, you need to initialize the next available
object with the address of an allocated block large enough to hold the
token and its terminator. Then you copy the token into this memory.

When you are about to process the N+1th token, you need to reallocate
the memory format points to so that it points to more than N objects.
How much more is a design decision you get to make. Some increment by
1 or other small fixed quantity; some decide to double N. This
reallocation sets a new upper limit which you must continue to check
against as you continue copying tokens. Depending on the number of
tokens in your original string, you may need to perform this
reallocation multiple times.


Remove del for email

 

[Default User]

Richard Bos wrote:

I did not understand the above point.

http://c-faq.com/malloc/cast.html
http://c-faq.com/malloc/mallocnocast.html




Brian

 

[Simon Biber]

I'm pasting the changed code

if ((tok = strtok(argv[1], delim)) != NULL)
{
format = (char**)malloc(1*sizeof(char*));

Why not just write:
format = malloc(sizeof *format);

Here, format is a char **
*format is a char *
**format is a char

So, sizeof *format == sizeof (char*)

if((format=(char*)malloc(strlen(tok)+1)) != NULL)
strcpy((char*)format+i, tok);

This is wrong! The fact that you needed a cast should be a big red flag,
indicating to you that what you are typing is wrong!

You don't want to copy to format+i. That's the start of the format
array, which can only hold sizeof(char*), probably just 4 bytes.

You want to copy to format. That's the memory that you just allocated
with malloc.

}
/*
* gdb shows the first token copied at 0th location of format .
*/

As it should -- you told strcpy to copy there. Unfortunately you
probably also copied to some bytes past what was allocated to format,
which you had no right to write to.

for (i=1; tok; i++)
{
if ((tok = strtok(NULL, delim)) != NULL)
if (format = (char**)realloc(format, (i+1)*sizeof(char*))
!= NULL)

Please, for the love of God, get rid of the casts!

What you have written in the line above will work, but it's best
practise to send the result of realloc to a separate temporary variable,
so that if realloc fails, you don't lose your pointer to the original array.

I would write:
char **temp;
if(temp = realloc(format, (i+1) * sizeof *format)) != NULL)
{
format = temp;
...
}
else /* realloc failed */
{
return format;
}

if((format=(char*)malloc(strlen(tok)+1)) != NULL)
strcpy((char*)format+i, tok);

Again, this should be strcpy(format, tok);

}

realloc() changes the address of format (something like 0x1) and
subsequent malloc() fails giving a segfault. Still scratching my head!
What's wrong in the code?

realloc often changes the address. This is how it usually works: it
first allocates a new block of memory, then copies the contents of the
old block into the new block, then returns the address of the new block.

The subsequent malloc fails because you wrote too far into the memory
allocated for format, clobbering the special data structures that are
used by the memory allocation system to keep track of your memory
allocations.

If you fix the bug with the strcpy, it should work.

 

[deepak]

I need to break a longer string (with strtok()) and store the smaller
strings in an array. since the number of small strings is not
fixed/known, I cannot use a declaration like char *format[n], so I
declared char** format = NULL;

Now the problem starts. Each time strtok() returns me a token (which is
char*), I call malloc() and I (want to) store the address returned in
the array format[], and copy the token to that address.

code snippet:
------------------
char **format = NULL;
char delim[] = ".-/:";
int i=0;

if ((tok = strtok(argv[1], delim)) != NULL)
{
if((format=(char*)malloc(strlen(tok))) != NULL)
strcpy(format+i, tok);
}

for (i=1; tok; i++)
{
if ((tok = strtok(NULL, delim)) != NULL)
if((format=(char*)malloc(strlen(tok))) != NULL)
strcpy(format+i, tok);
}

The malloc statement gives a segfault. In desparation, I have tried
various combinations of lvalue in malloc, but none of them were correct
(either I get a compiler warning like "invalid lvalue", or "assignment
makes an integer from pointer" etc.. or if compilation is successful,
run always fails with a segfault)

Q: Is above approach fundamentally incorrect? Can I declare something
as a pointer to pointer to char & treat like an array of strings? If
yes, what's wrong in above code? I have no hesitation in admitting that
my pointer fundamentals are not very clear yet -

Try this code.
It's just a casual one. I din't freed malloced variable etc.

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

main(int argc, char *argv[])
{
/* Copy the constant into the memory
* pinted to by 'test_string'
*/
char **test_string;

/* if 'test_string' is declared as below and the program will
give a
* 'Segmentation fault' This is because test_string' is
pointing
* to a constant i.e. somethin that cant be changed.

char *test_string="string to split up"; */

char *sub_string;
int i=0;
/* Extract first string */

test_string = (char **) malloc (sizeof(char *) * 10);

Don't cast the return from malloc. It never helps and can hide a
diagnostic regarding undefined behavior which you would really want to
see.

if ((sub_string = strtok(argv[1], "a")) != NULL){
*(test_string + i) = (char *) malloc (strlen(sub_string) + 1);

While *(test_string + i) is correct, the equivalent test_string is
the more common and preferred idiom. But i is always zero in this
block of code anyway.

*(test_string + i) = sub_string;

Definitely not. This causes a memory leak by destroying the only
pointer to the allocated memory. If you went to all the trouble to
allocate space for the string pointed to by sub_string, then you want
strcpy(test_string, sub_string);

Hello Shwarzer,
Thanks for the update.
I never digged about such a problem and from now onwards i'll follow
it.