srsxyz
srsxyz

Reputation: 231

Reversing String in C for loop error

I have an array of strings and am trying to reverse each string in the array to see if that string is a palindrome. I am using a for loop to increment an int i (the index). However after the I call the reverse function, the value of i becomes some really large number and I cant figure out why this is happening.

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

void revString(char *dest, const char *source);

int main() {    
    const char *strs[] = {
        "racecar",
        "radar",
        "hello",
        "world"
    };

    int i;
    char res[] = "";
    for (i = 0; i < strlen(*strs); i++) {
        printf("i is %d\n", i);
        revString(&res[0], strs[i]); //reversing string
        printf("i is now %d\n", i); 

        //comparing string and reversed string  
        if (strcmp(res, strs[i]) == 0) {
            printf("Is a palindrome");
        } else {
            printf("Not a palindrome");
        }
    }
    return 0;
}

void revString(char *dest, const char *source) {
    printf("%s\n", source);
    int len = strlen(source);
    printf("%d\n", len);
    const char *p;
    char s;
    for (p = (source + (len - 1)); p >= source; p--) {
        s = *p;
        *(dest) = s; 
        dest += 1;
    }
    *dest = '\0';
}

This is the output showing the value of i before and after the revString function is called.

i is 0
i is now 1667588961
Illegal instruction: 4

Upvotes: 1

Views: 112

Answers (2)

chqrlie
chqrlie

Reputation: 144695

There are multiple problems in your code:

  • You pass a destination array char res[] = ""; that is much too small for the strings you want to reverse. It's size is 1. This causes buffer overflow, resulting in undefined behavior.

    Use char res[20]; instead.

  • You enumerate the array of string with an incorrect upper bound. Use this instead:

    for (i = 0; i < sizeof(strs) / sizeof(*strs); i++)
    
  • The termination test for the loop in revString() is incorrect too: decrementing p when is equal to source has undefined behavior, although it is unlikely to have an consequences. You can simplify this function this way:

    void revString(char *dest, const char *source) {
        size_t len = strlen(source);
        for (size_t i = 0; i < len; i++) {
            dest[i] = source[len - i - 1];
        }
        dest[len] = '\0';
    }
    

Here is the resulting code:

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

void revString(char *dest, const char *source) {
    size_t len = strlen(source);
    for (size_t i = 0; i < len; i++) {
        dest[i] = source[len - i - 1];
    }
    dest[len] = '\0';
}

int main(void) {
    const char *strs[] = { "racecar", "radar", "hello", "world" };
    char res[20];

    for (size_t i = 0; i < sizeof(strs) / sizeof(*strs); i++) {
        revString(res, strs[i]);
        //comparing string and reversed string  
        if (strcmp(res, strs[i]) == 0) {
            printf("Is a palindrome\n");
        } else {
            printf("Not a palindrome\n");
        }
    }
    return 0;
}

Upvotes: 1

Dhruv Khatri
Dhruv Khatri

Reputation: 813

Here is Final Code with some change

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

void revString(char* dest, const char* source);
int main(){
    const char* strs[] = {
        "racecar",
        "radar",
        "hello",
        "world"
    };

    static int i;
    char res[] = "";
    int length = (int) sizeof(strs)/sizeof(char*);
    for(i = 0; i < length; i++)
    {
        printf("i is %d\n", i);
        revString(&res[0], strs[i]); //reversing string
        printf("i is now %d\n", i);

        //comparing string and reversed string
        if(strcmp(res, strs[i]) == 0){
            printf("Is a palindrome");
        }else{
            printf("Not a palindrome");
        }
    }
    return 0;
}
void revString(char* dest, const char* source){
    printf("%s\n", source);
    int len = (int) strlen(source);
    printf("%d\n", len);
    const char* p;
    char s;
    for(p = (source + (len - 1)); p >= source; p--){
        s = *p;
        *(dest) = s; 
        dest += 1;
    }
    *dest = '\0';

}

Change 1 :-

int i; to static int i; (Reason:- i is local variable you are calling function so when function call the value of i will remove and after that it will assign garbage value.)

change 2 :-

strlen(*strs) to length of array (because strlen(*strs) will give the length of first string)

Upvotes: 0

Related Questions