remove spaces from string

I have a little bug here. Can you let me know why I'm unable to print out or return the inputted string modified.

Code:

---

//ch11_9.c
//remove_spaces(char* given_string)

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

char* remove_spaces(char *given_string){

  size_t line  = strlen(given_string);
 
  //create a buffer to hold the string
  char* str_buf;
  if( (str_buf = malloc(line +1)) == NULL){
    perror("Unable to allocate storage for the string");
    return NULL;
  }

  //iterate through the original string and the buffer...allocating all non whitespace to the buffer
  while(*given_string && str_buf){

      if(isspace(*given_string))
        *str_buf = '-';
     
      else
        *str_buf = *given_string;
 
      //end of either conditional so increment both
      printf("%c", *str_buf);
      given_string++;
      str_buf++;
  }
  *str_buf='\0';
  return str_buf; 
}

int main(void){

  puts("enter your line of text...empty line to quit");
  char buffer[BUFSIZ];

  fgets(buffer, BUFSIZ, stdin);
  printf("\nnew string in main: %s\n", remove_spaces(buffer) );

  return 0;
}

---

At the time you return str_buf it points to the last character: '\0'. To free this memory you would need to capture that pointer from remove_space so you can free it main. Edit: that would not work either actually since str_buf has been altered since you malloced it. You could pass a pointer to pointer (**p) to the function as a remedy and alter the original string from within your remove_space function.

As was suggested, inside remove_spaces() function, save the beginning address of str_buf
save = str_buf; and return that.

Also I'm not sure why you did while(*given_string && str_buf). You only need to check while(*given_string).

Also the '\n' character is interpreted as 'isspace' for some reason, so you'll always get a trailing '-'.

Quote:

---

Originally Posted by Subsonics

At the time you return str_buf it points to the last character: '\0'. To free this memory you would need to capture that pointer from remove_space so you can free it main. Edit: that would not work either actually since str_buf has been altered since you malloced it. You could pass a pointer to pointer (**p) to the function as a remedy and alter the original string from within your remove_space function.

---

Thanks Subsonics. I altered the last part

Code:

---

20    //iterate through the original string and the buffer...allocating all non whitespace to the buffer
 21    int word_length = 0;
 22    while(*given_string && str_buf){
 23
 24      if(isspace(*given_string))
 25          *str_buf = '-';
 26
 27      else
 28          *str_buf = *given_string;
 29
 30      //end of either conditional so increment both
 31      printf("%c", *str_buf);
 32      given_string++;
 33      str_buf++;
 34      word_length+=1;
 35  }
 36  *str_buf='\0';
 37
 38  return (str_buf - word_length);
 39 }

---

that takes care of returning it to main correclty. Now I'd have to free the memory from main. Any suggetions? I've been looking at this for awhile so it may come to me later ;)

Just use free. Pass the pointer that points at the start of your string. ;)

Quote:

---

Originally Posted by nonoob

As was suggested, inside remove_spaces() function, save the beginning address of str_buf
save = str_buf; and return that.

---

sorry only saw this now. I coded an alternavtive. Do you see any issues with doing it that way?

Quote:

---

Also I'm not sure why you did [B]while(*given_string && str_buf). You only need to check while(*given_string).

---

Thanks. I removed it, when I was initially debugging I thought for some reason I was supposed to iterate over both addresses at the same time.
I've removed it now.

Quote:

---

Also the '\n' character is interpreted as 'isspace' for some reason, so you'll always get a trailing '-'.

---

yeah, a result of using fgets. I don't mind that for now. I think I can take it out in main before passing it to the funciton, thanks for pointing it out though.

Quote:

---

Originally Posted by hex_dump

sorry only saw this now. I coded an alternavtive. Do you see any issues with doing it that way?

---

Your way is fine.

Quote:

---

Originally Posted by std10093

Just use free. Pass the pointer that points at the start of your string. ;)

---

Code:

---

47    char* new_string = remove_spaces(buffer);
 48    printf("\nnew string in main: %s\n", new_string );
 49    free(new_string);

---

that work?

you tell me ;)

Quote:

---

Originally Posted by std10093

you tell me ;)

---

It compiles and runs fine. Its just because str_buf was declared in function scope i wasnt sure if freeing it like that would work. Thus im asking not if it works as in runs, but if its acceptable to free a pointer like that or is there some innocuous surprise that could come as a result im not aware of :)

You allocated space dynamically, thus this space is not being deallocated until you say so (with free). The pointer itself has local scope, thus dead after the termination of the function. However, you return a pointer and that is how you keep at all time a pointer that is set to the memory that you have allocated.

I liked very much that you fixed your code, almost by yourself ;) Bravo.

Excellent cause now I have another issue.

Code:

---

//ch11_9.c
//remove_spaces(char* given_string)

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

char* remove_spaces(char *given_string){

  size_t line  = strlen(given_string);
 
  //create a buffer to hold the string
  char* str_buf;
  if( (str_buf = malloc(line +1)) == NULL){
    perror("Unable to allocate storage for the string");
    return NULL;
  }

  //iterate through the original string...allocating all non whitespace to the buffer
  int word_len = 0;
  while(*given_string){

    if(*given_string !=' '){
        *str_buf = *given_string;
        printf("%c", *str_buf);
    }

      given_string++;
      str_buf++;
      word_len++;
  }
 
  //at the end of str_buffer so string...and return its start address
  *str_buf='\0';
  return (str_buf - word_len);
}

int main(void){

  puts("enter your line of text...empty line to quit");
  char buffer[BUFSIZ], *new_string;
  fgets(buffer, BUFSIZ, stdin);

  new_string = remove_spaces(buffer);
  printf("\nnew string in main: %s\n", new_string);
  free(new_string);

  return 0;
}

---

basically str_buf is getting all the characters as per the print statement...but when it returns it only prints up to the first whitespace..we're returning (str_buf - word len.)..which means we're returning up to the first whitespace....again. I'm sure it'll come to me tomorrow but any help would be appreciated.

Since you are trying, here is the problem. Input hello world...

I am going to increase the pointers - in your function- no matter what the current character is. So everything works as we want until we meet the first whitespace.
The if condition will be false, thus we won't copy anything, but we will increase the pointers. We then continue our job as intended, until we find the next whitespace or end. Here there is no whitespace.
So str_buf will be like this
|h|e|l|l|o|nothingAssignedByTheProgrammerHere|w|o| ....

So, why doesn't the printf outputs helloJUNKworld?
First idea I had was: Because it has been initialized to null terminators, thus the cell that has been untouched by you has a null terminator, thus printing ends there.!
Despite the fact that malloc does not guarantee something like that

Code:

---

The content of the newly allocated block of memory is not initialized, remaining with indeterminate values.

---

Moreover, local initialized variables do not have a standard value. But in my pc and yours you see what happens. They have the null terminator before getting a value by you.
Just after the malloc I did this

Code:

---

for(i=0;i<line+1;i++)
  if(str_buf[i]=='\0') printf("%d",i);

---

and here is my output

Code:

---

0123456789101112

---

which means that null terminators are all over the place.

Then in main, I did this

Code:

---

l = strlen(new_string);
  for(i=0;i<l+1;i++)
        printf("\nnew string in main: %c %d\n", new_string[i], l);

---

and this gave me output

Code:

---

new string in main: h 5

new string in main: e 5

new string in main: l 5

new string in main: l 5

new string in main: o 5

new string in main:  5  <-- HERE I HAVE THE NULL TERMINATOR

---

which actually gives base to somebody to say that all I said before is just not correct. Well, the truth is that strlen will stop at the null terminator (the cell that we left untouched...). And of course, that is what she should do, as strings in C are to be terminated with the null terminator! So, strlen supposes that when it reaches the first null terminator, the string is terminated.
So, if I do this

Code:

---

for(i=0;i<13;i++)
        printf("\nnew string in main: %c %d\n", new_string[i], l);

---

I will get as output this

Code:

---

new string in main: h 5

new string in main: e 5

new string in main: l 5

new string in main: l 5

new string in main: o 5

new string in main:  5      <--- NULL TERMINATOR

new string in main: w 5

new string in main: o 5

new string in main: r 5

new string in main: l 5

new string in main: d 5

new string in main:              <--- NEWLINE CHARACTER. Remember, we used fgets!
 5

new string in main:  5  <--- NULL TERMINATOR

---

Simple fix

Code:

---

if(*given_string !=' '){
        *str_buf = *given_string;
       
        printf("%c", *str_buf);
    }
    else{
        word_len--;
        str_buf--;
    }

---

But if I were you, I would just write

Code:

---

while(*given_string){
 
    if(*given_string !=' '){
        *str_buf = *given_string;
        str_buf++;
        word_len++;
        printf("%c", *str_buf);
    }
      given_string++;
  }

---

Do not do moves that do not need to be done. To your work with always the minimum steps and lines of code :) (as long as they remain readable of course :P )

Hope this helps :)