I have the following code

int eval(char* string) {

char* token;

outputString[0] = NULL;

token = strtok(string, " ");

while(token != NULL){

   if(strstr((char *) token, "@") != NULL) {
      strcat(outputString, "@");
      token = calc(token);
   strcat(outputString, token);
   strcat(outputString, " ");
   token = strtok(NULL, " "); 

 printf("%s\n", outputString);


It is called on a string that looks like this

The water has @TREE and @SKY in it.

This code works completely fine. However I want to make it more efficient.

The code is supposed to do the following --

Cut the string up by spaces, however, if an @ is found, it should be sent away to be evaluated by calc();. If none are found, we continue splitting the string by spaces until one is actually found. While we are running through, we concatenate it onto outputString and continue forward.

  • \$\begingroup\$ How aaa@bbb and similar tokens should be processed? \$\endgroup\$
    – vnp
    Apr 4, 2017 at 23:03

1 Answer 1

  • outputString doesn't have enough room to accomodate the results. It can hold just one byte. It is just unlucky coincidence that the code does not segfault. In any case, undefined behaviour.

    If the intention is just to print the result out, you don't need outputString at all. Just printf the tokens as you compute them.

  • strcat must scan the entire outputString to figure out where it ends. The overall time complexity of the repeated strcats becomes roughly quadratic. I recommend to maintain a pointer to the end of outputString, and strcpy the token.


Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.