2

I'm trying to get input from the user while allocating it dynamically and then "split" it using strtok.

Main Questions:

  1. Im getting an infinite loop of "a{\300_\377" and ",".
  2. Why do i get a warning of "Implicitly declaring library function "malloc"/"realoc" with type void"

Other less important questions:

3.i want to break, if the input includes "-1", how do i check it? As you can see it breaks now if its 1.

4.In the getsWordsArray() i want to return a pointer to an array of strings. Since i dont know how many strings there are do i also need to dynamically allocate it like in the getInput(). (I dont know how many chars are there in each string)

   int main(int argc, const char * argv[])
    {
        char input = getInput();
         getWordsArray(&input);  
    }
    char getInput()
    {   
        char *data,*temp;
        data=malloc(sizeof(char));
        char c; /* c is the current character */
        int i; /* i is the counter */
        printf ("\n Enter chars and to finish push new line:\n");
        for (i=0;;i++) {
            c=getchar(); /* put input character into c */
            if (c== '1')                // need to find a way to change it to -1
                break;
            data[i]=c; /* put the character into the data array */
            temp=realloc(data,(i+1)*sizeof(char)); /* give the pointer some memory */
            if ( temp != NULL ) {
                data=temp;
            } else {
                free(data);
                printf("Error allocating memory!\n");
                return 0 ;
            }
        }   
        printf("list is: %s\n",data); // for checking
        return *data;     
    }

    void getWordsArray(char *input)
    {
        char *token;
        char *search = " ,";
        token = strtok (input,search);
        while (token != NULL ) {
            printf("%s\n",token);
            token = strtok(NULL,search);
        }
    }

EDIT: i noticed i forgot to "strtok" command so i changed it to token = strtok(NULL,search);

I still get wierd output on the printf:

\327{\300_\377
Xephon
  • 393
  • 1
  • 12
Yevgeni
  • 1,533
  • 17
  • 32

3 Answers3

2

Change:

int main(int argc, const char * argv[])
{
    char input = getInput();
    getWordsArray(&input);  
}

to:

int main(int argc, const char * argv[])
{
    char *input = getInput();
    getWordsArray(input);  
}

with a similar to the return value of getInput():

char *getInput()
{   
    // ...
    return data;
}

In your code, you were only saving the first character of the input string, and then passing mostly garbage to getWordsArray().

For your malloc() question, man malloc starts with:

SYNOPSIS
   #include <stdlib.h>

For your getchar() question, perhaps see I'm trying to understand getchar() != EOF, etc.

Community
  • 1
  • 1
Joseph Quinsey
  • 9,553
  • 10
  • 54
  • 77
2

Joseph answered Q1.

Q2: malloc and realoc returns type void *. You need to explicitly convert that to char *. Try this:

data = (char *) malloc(sizeof(char));

Q3: 1 can be interpreted as one character. -1, while converting to characters, is equivalent to string "-1" which has character '-' and '1'. In order to check against -1, you need to use strcmp or strncmp to compare against the string "-1".

Q4: If you are going to return a different copy, yes, dynamically allocate memory is a good idea. Alternatively, you can put all pointers to each token into a data structure like a linked list for future reference. This way, you avoid making copies and just allow access to each token in the string.

Xephon
  • 393
  • 1
  • 12
  • 2
    R.e. `data = (char *) malloc(sizeof(char))`. The cast is necessary in C++, but unnecessary and indeed frowned upon in C. – Joseph Quinsey Jan 03 '14 at 19:29
  • @JosephQuinsey That's why I believe it is good practice to do such explicit type conversion even in C. Though in C++, this is also deprecated so one should avoid this unless absolutely necessary, ie, while reusing huge chunk of C code. – Xephon Jan 03 '14 at 19:31
1

Things that are wrong:

  1. Strings in C are null-terminated. The %s argument to printf means "just keep printing characters until you hit a '\0'". Since you don't null-terminate data before printing it, printf is running off the end of data and just printing your heap (which happens to not contain any null bytes to stop it).

  2. What headers did you #include? Missing <stdlib.h> is the most obvious reason for an implicit declaration of malloc.

  3. getInput returns the first char of data by value. This is not what you want. (getWordsArray will never work. Also see 1.)

Suggestions:

Here's one idea for breaking on -1: if ((c == '1') && (data[i-1] == '-'))

To get an array of the strings you would indeed need a dynamic array of char *. You could either malloc a new string to copy each token that strtok returns, or just save each token directly as a pointer into input.

Notlikethat
  • 20,095
  • 3
  • 40
  • 77
  • Thank you, but i didnt get the first thing about the printf - what do u suggest ? – Yevgeni Jan 03 '14 at 21:22
  • @YevgeniTarler Once you break out of the loop, you need to add one more element to the end of `data` and set that to `'\0'`. Then your `char` array becomes a valid null-terminated string. – Notlikethat Jan 03 '14 at 21:32