Hi, I am a C newbie and I have the following question:

        I have a an array of arrays of chars declares as char ** and I would like
to dynamically add more strings. here is what I did:

        char **lines;
        char **temp;
        if (new_string) {
                temp = lines;
                while (temp)
                        temp++;        
                lines = (char **)realloc(lines,sizeof(char *));
                *temp = new_string;
        }

        This does not work as the call to realloc segfaults. Does anyone have a
working (and, therefore, BETTER) way of doing this? Or could anyone tell
me why the realloc call segfaults?

        Thanx

                Mitko
--

I'll hold off giving a solution;  working it out for yourself should
be educational.  But I will give some pointers (no pun intended)....

The reason realloc "segfaults" is probably that you haven't initialised
the variable lines.  realloc assumes it's first argument has been
assigned using malloc/realloc or (IIRC) a NULL pointer.  Anything
else is undefined.  As an aside, if realloc changes the size of the
array, any new elements added are NOT initialised.

Apart from that, you have a few other problems

1)  temp points at some position relative to lines.  Then you realloc
      lines, which means that temp is pointing at some undefined
      location.  So the  *temp = new_string; line will give undefined
      results.

2)  I'd guess your while statement should be  while(*temp) ....
      Of course, that assumes you initialise the lines array correctly.

3)  Your realloc statement will always set lines to be an array of
       one pointer to char *.  The second argument is an absolute
       size, not how much the array should grow....
--

Here's what I would do:

        char**  array = 0;
        int     count = 0;

        void    add(char *newstr)
        {
                char**  tmp = calloc(count + 1, sizeof(char*));
                int     i;

                for (i = 0; i < count; i++)
                        tmp[i] = array[i];      /* copy pointers to new array */
                tmp[i] = malloc(strlen(newstr) + 1);
                strcpy(tmp[i], newstr);         /* add new string */
                if (array)
                        free(array);            /* free old pointer array */
                array = tmp;                    /* setup new pointer array */
                count++;
        }

HTH,
        AriL
--
Pain and disappointment are inevitable. Misery is optional.
Homepaged at http://www.angelfire.com/or/lukumies
* Do not reply via email *
--

UB. lines has not been initialised.

UB. lines has not been initialised. Moreover, you do not need to (and
therefore should not) cast realloc(), and you pass it the total new size
of the desired block, not, as I gather from this snippet you're doing,
the size increase.

Richard
--

realloc is seg faulting because (as someone pointed out
yesterday) realloc will only work if passed (a) a NULL pointer or
(b) a variable which has already had space malloc()ed to it.
lines is an automatic variable, which means that it can have any
value when you get to the realloc the first time, which is a bit
of an issue, and causes the segfault.

As to allocating a 2-D array, you should have a look at
http://www.eskimo.com/~scs/C-faq/q6.16.html

Cheers,
Dave.

--
David Neary,               E-Mail dave.dot.neary.at.palamon.dot.ie
Palamon Technologies Ltd.  Phone +353-1-634-5059      
--

That's not an array of arrays, it's a pointer to a pointer to char.
It only becomes an array of arrays after you initialize it like that.

This 'while' already is highly suspicious, IMHO. You almost certainly wanted
to write
                while (*temp)
                        temp++;

This would advance temp to the end of the array.

Now you reallocate a 1-element array of pointers to char. This is most
probably not what you wanted. The cast is superfluous and potentially
dangerous, too.  Something like

                size_t length = (temp - lines);

                temp = realloc(lines, (length + 1) * sizeof(char *));
                if (temp == NULL) {
                        error_handling();
                } else {
                        lines = temp;
                        lines[length] = new_string;
                }

would be closer to what you need.              

What you really have to do is to keep track of the size of your
dynamically allocated array of pointers to char (i.e. you may want to
keep 'length' visible along with 'lines'). You currently use a 'NULL'
pointer at the end of the array for that, but I don't see you
initialize the new last element to NULL anywhere, to guarantee proper
termination.
--

Even if all the snow were burnt, ashes would remain.
--

I take it new_string was defined as "char *newstring;"?

This effectively does nothing as lines has not had anything allocated
to it; the contents are undefined.

This is almost certainly not doing what you want.  Short version: The
while loop runs until temp is "zero", which, since temp is a pointer,
is likely to be a NULL pointer.

A "simple" method of doing what it looks like you want would be to
check against the contents of temp, or to put it another way, check
against what temp points to.  This does require that the end of your
array has been initialized to a NULL pointer (or whatever value you're
checking it against).

In any case, you'll need to have alloc()'d something to lines in the
first place before this will work at all.

This is dangerous, as realloc() returns a NULL if (for some reason) it
can't fulfill your request.  If that happens, you'll have just sent the
block lines used to point to off into hyperspace.  At the very least,
have another temporary variable to store the pointer returned from
realloc() until you check it to make sure it's not NULL.

The why is that you can't realloc() until you've done a malloc() (or,
at the very least, have initialized lines to NULL to tell realloc() to
act as malloc()).  The realloc() function changes the size of a
previously allocated block of memory, which, in this case, lines must
point to.  When lines is declared, it's not being initialized, and so
points to a undefined location of memory.

You're welcome.

There are a few other points in there that likely won't do what you
expect, but they should be visible with the allocation issues cleared
up.

(BTW, watch the long lines; they don't easily lend themselves to
quoting.)
--

PGP key fingerprint = 48 94 7A 54 59 B6 C0 77  1F F6 94 55 0C 55 51 C4
+Accept: text/plain; charset=ISO-8859-*,UTF-*
--