--- In [email protected], "Hermann" <[EMAIL PROTECTED]> wrote:
>
> My Code below won't work,
> I would like to split strings delimited by commas and put them into
> arrays;
> Anyone can fix my code?
> Thank you.
> 
> 
> #include <stdio.h>
> #include <string.h>
> 
> int main()
> {
> char *mywords;
> unsigned int i,a;
> char *myarray;
> 
> myarray = "\0";
> mywords = "Hallo,Hi,Hai,Hey";
> 
>       for(a=0;a<=3;a++){
>               for(i=0;i<=strlen(mywords)-1;i++){
>                       if(mywords[i]!=','){
>                               myarray[a] =
>                                   strcat(myarray[a],mywords[i]);      
>                       }
>               }
>       }
> printf("%s\n",myarray[0]);
> printf("%s\n",myarray[1]);
> printf("%s\n",myarray[2]);
> printf("%s\n",myarray[3]);
> }

There are several mistakes in the code.

The first mistake is that strcat() expects that the string which you
want to copy (its second parameter) is terminated by a NUL character.
Which is not the case in your setup except for the last string. You
have to make sure that every string is copied up to (but not
including) the separating comma or up to the NUL character which
terminates "mywords".
The function you will want to use is named strncat().

The second mistake is that "myarray" should not be a "char *" but a
"char **". You want to have "myarray" set up such that every of its
elements is a single "char *" (at least the printf() statements show
that you want to do this). In other words, you want to have "myarray"
being an array of "char *".
So you either have to declare it as a "char **" or as a "char * []".
Personally I prefer "char **" in such cases, but there's another
gotcha which I'll describe below.

The third mistake (directly related to the second one) is that you
have to set up every single "char *" in the array "myarray" points to
dynamically. Just from a declaration of a pointer this pointer will
not point to any useful or useable contents; you first have to set up
some memory to point to, then you can initialise this memory and use
it. So the inner loop might look like this:

LastPosition = -1;
/* ^^ because we start the search for ',' at position 0 */

for (String_Index = 0; String_Index <= 3; String_Index++)
{ for (Index = 0; Index <= strlen( mywords) - 1; Index++)
  { /* We've found another string boundary either if we */
    /* have encountered a ',' or the terminal NUL: */
    if (mywords [Index] == ',' || ! mywords [Index])
    { /* The actual count of characters in this string: */
      Length = Index - LastPosition - 1;

      /* Allocate dynamic memory for this string; beware that */
      /* we have to allocate one extra byte for the NUL byte: */
      if ( ! (myarray [a] = malloc( Length + 1)))
      { fprintf( stderr, "ERROR: out of memory!\n");
        exit( EXIT_FAILURE);
      } /* Out Of Memory!!! */
      else
      { /* Copy all characters of this string: */
        strncat( myarray [String_Index],
              mywords [LastPosition + 1], Length);
        /* Set up the terminal NUL byte: */
        myarray [String_Index] [Length] = '\0';
      } /* if memory had been allocated */

    } /* if end of one part string found */

  } /* for every character in "mywords[]" */

} /* for all four expected substrings */


Of course this is still not perfect. But for this exercise this should
suffice.

EXCEPT for one point I've left out above:
If you declare "myarray" as a "char **", then you will first have to
set up this array using "malloc()" (or better "calloc()") before
entering the loop above.

Why?

Because declaring "myarray" as a "char **" does NOT allocate any
memory for this array; it just tells the compiler that "myarray" is
supposed to point to an array of "char *" pointers, but it also tells
the compiler that YOU and no one else (in particular not the runtime
system) assumes responsibility for maintaining the memory that
"myarray" and all pointers it points to point to!

Meaning: if you deal with pointers, you WILL HAVE to allocate and
deallocate the memory they point to on your own. The runtime system
doesn't give a damn about them, that's completely up to you.


Regards,
Nico

Reply via email to