Removing spaces from strings

Question

I tried to write a function that gets a string and creates a new string but without multiple spaces (leaving only 1 space between words).

So far I wrote this, but for some reason it crashs and the debugger shows nothing.

I also don't know where do I need to put the free function...

#define _CRT_SECURE_NO_WARNINGS
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char* upgradestring(char* oldtext);

int main()
{
    char str1[] =  "Chocolate     Can   Boost   Your Workout" ;

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

    printf("\n%s\n", upgradestring(str1));

    return 0;
}

char* upgradestring(char* oldtext)
{
    int i,j, count = 1;
    char *newstr;

    for (i = 0; oldtext[i] != '\0'; i++)
    {
        if (oldtext[i] != ' ')
            count++;
        else if (oldtext[i - 1] != ' ')
            count++;
    }
    newstr = (char*)malloc(count * sizeof(char));
    if (newstr == NULL)
        exit(1);

    for (i = 0, j = 0; (oldtext[i] != '\0')|| j<(count+1); i++)
    {
        if (oldtext[i] != ' ')
        {
            newstr[j] = oldtext[i];
            j++;
        }
        else if (oldtext[i - 1] != ' ')
        {
            newstr[j] = oldtext[i];
            j++;
        }
    }

    return newstr;
}

Answer 1

For starters neither declaration from the header <string.h> is used in your program. Thus this directive

#include <string.h>

may be removed from the program.

According to the C Standard the function main without parameters shall be declared like

int main( void )

The function with the strange name upgradestring:) does not change the argument. Hence it should be declared like

char* upgradestring( const char* oldtext);
                     ^^^^^

Take into account that the source string can start with blanks. In this case statements like this

    else if (oldtext[i - 1] != ' ')
        count++;

result in undefined behavior because there is an attempt to access memory beyond the string when i is equal to 0.

The condition

(oldtext[i] != '\0')|| j<(count+1); 

should be written at least like

(oldtext[i] != '\0') && j<(count+1); 
                     ^^^ 

though it is enough to check the index j because it can not be greater than the length of the source string.

You forgot to append the result string with the terminating zero '\0'.

Also it is not a good idea to exit the function with this statement

exit(1);

In this case you could just return a null pointer.

And the allocated memory should be freed before exiting the program.

As it has been mentioned before a source string can start with spaces and also have a redundant trailing space. I think it will be logically consistent to exclude them from the result string.

Usually the space character is considered in pair with the tab character. Moreover C has a special function isblank declared in the header <ctype.h> that checks whether a character is a space or a blank. (As far as I know the MS VS does not support this function)

Taking all this into account the function can be defined the following way as it is shown in the demonstrative program.

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

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

    const char *p = s;

    //  skip leading blanks
    while ( *p == ' ' || *p == '\t' ) ++p;

    _Bool last_blank = 0;

    for ( ; *p; ++p )
    {
        ++n;
        if ( ( last_blank = ( *p == ' ' || *p == '\t' ) ) )
        {
            while (  p[1] == ' ' || p[1] == '\t' ) ++p;
        }           
    }

    if ( last_blank ) --n;

    char *q = malloc( n + 1 );

    if ( q )
    {
        p = s;

        //  skip leading blanks
        while ( *p == ' ' || *p == '\t' ) ++p;

        size_t i = 0;
        for ( ; i < n; i++, ++p )
        {
            q[i] = *p == '\t' ? ' ' : *p;
            if ( q[i] == ' ' )
            {
                while (  p[1] == ' ' || p[1] == '\t' ) ++p;
            }
        }

        q[i] = '\0';
    }

    return q;
}

int main(void) 
{
    char s[] =  "\t\tChocolate  \t   Can \t  Boost   Your Workout   ";

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

    char *t = trim_blanks( s );

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

    free( t );

    return 0;
}

The program output is

"       Chocolate      Can    Boost   Your Workout   "
"Chocolate Can Boost Your Workout"

Answer 2

---

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

char *upgradestring(char *oldtext)
{
size_t len,src,dst,spc;
char *result;

        // First pass: count needed size
for (len=src=spc=0;oldtext[src]; src++){
        if (oldtext[src] != ' ') spc=0;       // non-space needs space
        else if(spc++) continue;              // skip non first space
        len++;
        }

result= malloc (1+len);

        // Second pass: copy(K&R style)
for (dst=src=spc=0; (result[dst] = oldtext[src]) ; src++){
        if (oldtext[src] != ' ') spc=0;      // non-space: rest counter
        else if(spc++) continue;             // skip non-first space
        dst++;
        }

return result;
}

---

Simplified version: dont calculate the size in a first pass, but start with the same size as the original, and resize after the second pass. (strdup() can be replaced by strlen+malloc+memcpy)

---

char * strdup(char *);

char *upgradestring2(char *oldtext)
{
size_t src,dst,spc;
char *result;

result= strdup (oldtext);

        // edit the copy, skipping all spaces except the first
for (dst=src=spc=0; result[src] ; src++){
        if (result[src] != ' ') spc=0;  // non-space:reset counter
        else if(spc++) continue;        // skip space,except the first

        result[dst++] = result[src]; // Copy
        }
result[dst] = 0;// terminate string;

// result=realloc(result, dst+1);

return result;
}

Answer 3

You're addressing [i-1] and it's not within the range of the original array if i==0.

Here's how you could do it:

Simply copy one by one and if the char is ' ', keep skipping while it is ' ', otherwise advance by one.

static size_t newlen(char const *o)
{
    size_t r=0;
    while(*o){
        r++;
        if(*o==' ') 
            while(*o==' ')
                o++;
        else
           o++;
    }
    return r;

}
char *upgradestring(char const *o)
{
    char *r, *p;
    size_t len = newlen(o);
    if( 0==(r = malloc(len+1)))
        return 0;
    r[len]=0;
    for(p=r;*o;){
        *p++=*o;
        if(*o==' ') 
           while(*o==' ') 
              o++;
        else
           o++;
    }
    return r;
}
int main()
{
    char str1[] =  "Chocolate     Can   Boost   Your Workout" ;
    char *new;
    printf("%s\n", str1);
    if(0 == (new = upgradestring(str1)))
        return 1;
    printf("%s\n", new);
    free(new);
}

Failures to allocate are best signalled by return codes (you wouldn't want a library function to abort your program if it fails).

In order to be able to free the returned string, you first must capture it in a variable.

Answer 4

Good attempt, but let's focus on when you need to free your memory. You allocate dynamically the memory inside the function, then call the function inside a printf, which will allow the string to print, but how will you deallocate it? Use a pointer to assign the return value of your function, print it, and then free it!

Moreover, you need to allocate space for as many characters the new string has, plus one for the null terminator, since C strings require that to work smoothly with functions coming from headers, such as printf().

Furthermore, we do not cast what malloc() returns in C, read more here.

Also this:

else if (oldtext[i - 1] != ' ')

should be written as:

else if (i != 0 && oldtext[i - 1] != ' ')

to avoid accessing oldtext[-1] which is out of bounds, when i is 0.

Lastly, the condition you used when populating the new string, would be better with a logical AND, instead of an OR, since we have to stop as soon as either condition is false (we do not want to read past the null terminator of the original string, or past the size of the new string).

Putting everything together, we:

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

char* upgradestring(char* oldtext)
{
    int i, j, count = 0;
    // compute 'count'
    for(i = 0; oldtext[i]; i++)
    {
        if (oldtext[i] != ' ')
            count++;
        else if (i != 0 && oldtext[i - 1] != ' ')
            count++;
    }
    char* newstr = malloc(count + 1); // PLUS ONE for the null terminator
    if(!newstr) // check if malloc failed
    {
        printf("Malloc failed\n");
        return 0;
    }
    // populate 'newstr'. We need to stop when either condition is false
    for (i = 0, j = 0; (oldtext[i]) && j<(count+1); i++)
    {
        // Same as your code
    }
    // Assign the null terminator!
    newstr[j] = '\0';
    return newstr;
}

int main(void) {
    char str1[] =  "Chocolate     Can   Boost   Your Workout" ;
    // store the result of your function into 'newstr'
    char* newstr = upgradestring(str1);
    // print it
    printf("%s\n", newstr);
    // free it, since you no longer need it!
    free(newstr);
    return 0;
}

Output:

Chocolate Can Boost Your Workout