garbage output after whitespace removing

Question

i've this code:

     int i =0;
  char * str = "ar   bitrary             whitespace";

  int whitespace=0,index;
  for(index = 0;index < strlen(str);index++)
    {
      if(isspace(str[index]) != 0)
    {
      whitespace++;
    }
    }


  char * tmp = (char *)calloc(strlen(str)-whitespace +1,sizeof(char));

  memset(tmp,'\0',strlen(tmp)+1);

  while(i < strlen(str))
    {
      if(isspace(str[i]) != 0)
    {
      i++;
      continue;
    }else if(isspace(str[i]) == 0)
    {

      strcat(tmp,&str[i]);
      i++;
    }

    }


  printf("\nnew string is: %s \n",tmp);

the problem is that the output is a string without the whitespace removed + some garbage character. I've used memset to null terminate tmp,is there the problem?

Garbage after a string sounds like you missing a string terminator. — Some programmer dude
– Some programmer dude, Commented Feb 21, 2017 at 17:44

Vlad from Moscow · Accepted Answer · 2017-02-21 21:14:31Z

1

The length of the source string could be calculated before this loop

for(index = 0;index < strlen(str);index++)

Otherwise if the code will not be optimized the function strlen will be called for each iteration of the loop. In fact using of the function is redundant for such a task.

This statement

memset(tmp,'\0',strlen(tmp)+1);

does not make sense because the call of calloc already initialized the memory with zeroes.

This statement

strcat(tmp,&str[i]);

also copies blanks from the source string after the position i. So it can write beyond the memory allocated for the array pointed to by the pointer tmp.

You can write a separate function that can look as it is shown in this demonstrative program

#include <stdlib.h>
#include <stdio.h>
#include <ctype.h>

char * remove_blanks( const char *s )
{
    size_t n = 0;

    const char *p = s;

    do
    {
        if ( !isspace( ( unsigned char )*p ) ) ++n;
    } while ( *p++ );

    char *t = malloc( n );

    if ( t )
    {
        char *q = t;
        p = s;

        do
        {
            if ( !isspace( ( unsigned char )*p ) ) *q++ = *p;
        } while ( *p++ );
    }

    return t;
}

int main(void) 
{
    char * str = "ar   bitrary             whitespace";

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

    char *t = remove_blanks( str );

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

    free( t );
}

The program output is

"ar   bitrary             whitespace"
"arbitrarywhitespace"

edited Feb 21, 2017 at 21:14

answered Feb 21, 2017 at 18:54

Vlad from Moscow

313k27 gold badges204 silver badges358 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

SirLeoncavallo Over a year ago

Thanks,I'm trying to understand a couple of things: when you in the beginning say that the "strlen" in the for loop was redundant,could that be solved like "int ln = strlen(str)" and than using ln as upper limit for the loop?(avoiding like this the continuous call to strlen). Second things: the cast to unsigned character is really necessary? Third things: in the do-while loop as the argument of the while you use "*p++" and then in the isspace inside the loop you write "*p" as the argument, but *p don't indicate only to the first element of the string? Sorry for the questions,but I'm learning.

Vlad from Moscow Over a year ago

@Collapsed You can calculate the length either before the loop or in the part of the first expression of the for statement. If some characters can have negative values then it is necessary to cast to unsigned char. In the do while loop there is access to each character of the original string using the pointer p. So it must be increased to point each character sequentially and this is done in the condition of the loop.

Iharob Al Asimi · Accepted Answer · 2017-02-21 17:55:13Z

1

this is your problem

 memset(tmp,'\0',strlen(tmp)+1);

strlen(tmp) works by looking for '\0' in tmp, you have a chicken and egg situation here.

You should not be doing a memset any way, just tack on a '\0' when you fnish copying

And dont use strcat, instead maintain a pointer to tmp and just do *p = str[i] then increment p

edited Feb 21, 2017 at 17:55

Iharob Al Asimi

53.1k6 gold badges66 silver badges100 bronze badges

answered Feb 21, 2017 at 17:54

pm100

50.6k25 gold badges95 silver badges158 bronze badges

1 Comment

Iharob Al Asimi Over a year ago

I don't understand, are you implying that there is no 0 terminator or that memset() only sets the first byte to 0?

Iharob Al Asimi · Accepted Answer · 2017-02-21 17:59:14Z

1

I will not read your question, you overwrite the '\0' terminator for sure.

Now that I read your question, it looks like you need to understand strings and arrays better,

Don't ever write while (i < strlen(str))
Don't use strcat() for adding a single character, you apparently did overwrite the '\0' there. Furthermore, don't ever use strcat() for concatenating more than to pieces of a string.

Also notable,

You memset() after calloc() which already initialized to 0. That means that you are enforcing something that is not necessary, and trying it twice as if it failed the first time which I can guarantee it didn't.

In fact, since you have used calloc() and all bytes pointed to by tmp are 0 then strlen(tmp) will return 0, thus your memset() is equivalent to
```
tmp[0] = '\0';
```
and you REALLY don't need initialize tmp except when you finally copy the actual bytes from str.

I always advice against calloc() for strings, because

You don't really need to initialize something twice.
You should be sure your code does take the terminating '\0' into account and not simply assume that it's there because you calloc()ed. That is a bug that you just hide with calloc() but it shows up at some point.

Try this and see if you can understand the reasons for my changes

#include <stdio.h>
#include <ctype.h>
#include <stdlib.h>

int main(void)
{
    int whitespace;
    int length;
    char *str = "ar   bitrary             whitespace";
    char *tmp;

    whitespace = 0;
    for (length = 0; str[length] != '\0'; ++length) {
        if (isspace(str[length]) != 0) {
            whitespace++;
        }
    }
    tmp = malloc(length - whitespace + 1);
    if (tmp == NULL)
        return -1;
    for (int i = 0, j = 0; str[i] != '\0'; ++i) {
        if (isspace(str[i]) != 0)
            continue;
        tmp[j++] = str[i];
    }
    tmp[length - whitespace] = '\0';
    printf("new string is: %s\n",tmp);

    free(tmp);
    return 0;
}

edited Feb 21, 2017 at 17:59

answered Feb 21, 2017 at 17:42

Iharob Al Asimi

53.1k6 gold badges66 silver badges100 bronze badges

3 Comments

SirLeoncavallo Over a year ago

Thanks for having answered. What is the reason why i don't have to write "while(i < strlen(str))" ? After everything I've to enforce the '\0' termination as you did(talking about the general case of working with strings)?

Roy Avidan Over a year ago

the strlen function will take time and spac. when you write while(strlen(str)) the loop will go to the function every time until the loop will end. when you write int l=strlen(str) before and use l you will call strlen only once.

Iharob Al Asimi Over a year ago

@Collapsed Unlike in PHP where it's common to see strlen() in a loop control, in c it's not a good idea because the length is not stored anywhere. So every time you call it, it counts characters one by one. The same goes for strcat(). Knowing the length of a string whose length will not change during the execution is a nice advantage in c that you can use for improving performance.

Collectives™ on Stack Overflow

garbage output after whitespace removing

3 Answers 3

2 Comments

1 Comment

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

1 Comment

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related