Thread: String function problem

Hi, I have made a function that takes a pointer to a string, and the strings length and cleans out all the rubbish and adds a terminating null so that I can use the string without possible problems.

The function works fine appart from the code to copy the new string back over-writing the old one at the end. I think I have gone about it in the wrong way but strings confuse me a bit still.

Here is the function, but I am sure its the stuff under the "Now put the cleaned up string back again" comment thats the problem.

Code:

void fix_string(int length, char *old_string)
{
    int i, j;
    char new_string[MAX_NAME_LENGTH];
    
    /* Loose the extra off the end of a long name */
    if (length >= MAX_NAME_LENGTH)
        length = MAX_NAME_LENGTH;
    
    /* Put only the letters and numbers into the new name */    
    for (i = 0, j = 0; i < length; i++)
        if( isalnum(old_string[i]) )
            new_string[j++] = old_string[i];
        else
            printf("Woopdy blody do\n");
    
    /* Add the terminating null character */
    new_string[j] = '\0';
    
    printf("test: %s\n", new_string);
    
    /* Now put the cleaned up string back again */
    while (new_string[i] != '\0')
    {
        old_string[i] = new_string[i];
        i++;
    }    
    
    old_string[i] = '\0';
        
}

Thanks

you could also use strcpy to copy the new string into the old buffer.

Code:

char *strcpy(char *str1, const char *str2);

edit: argh.. too slow

I'l use that if I can't get it working but I was wondering is there something wrong with the way I have done it?

you have to reset i to 0, before running into theloop

Originally Posted by IceBall

you have to reset i to 0, before running into theloop

Oh no! what a stupid mistake! sorry about that everyone!

A common mistake. For loops like that which use an variable to index all the elements of a string, it's probably easier to use a for loop insead of a while. In case you don't know much about for loops, here's a syntax reference. If you already know about for loops, ignore the rest of this post.

Code:

for(initial action; loop condition; looping action)
 {
  loop contents;
 }

The initial action is performed only once, at the beginning of the loop. The loop condition is a logic statement that will determine if the loop continues. The looping action will be performed at the beginning of each pass through the loop, including the first. Since you seem to understand the while loop, let me rewrite this as a while loop.

Code:

initial action;
while(loop condition)
 {
  looping action;
  loop contents;
 }

If you were to rewrite that peice your code as a for loop, it look like this.

Code:

  for(i = 0; new_string[i] != '\0'; ++i)
     old_string[i] = new_string[i];

For me, for loops simply make the code easier to understand, but it's really only a matter of personal preferrence.

Note that I didn't put {} tokens around the contents of the loop, simply because there is only one statement there and they are not necessary.

Yea I realised that a for loop would be better already Thanks everyone