string reversal in c

Question

#include<stdio.h>
#include<malloc.h>
#include<string.h>
#define SUCCESS 0
#define FAILURE -1
int str_rev(char **s, char **d){
  int count = 0;
  if(s == NULL || d == NULL){
   printf("\n Invalid address received! \n");
   return FAILURE;
  }
  else{
   while(**s != '\0'){
    **s++;count++;
   }
   while(count > 0){
    **d++ = **s--;count--;
   }
   **d = '\0';
   return SUCCESS;
  }
}
int main(){
 int ret_val = SUCCESS;
 char *a = "angus";
 char *b;
 b = malloc((strlen(a) * sizeof(*a)) + 1);
 ret_val = str_rev(&a,&b);
 if(ret_val == FAILURE){
   printf("\n String is not reversed! going to quit! \n");
   free(b);
   return FAILURE;
 }
 printf("\n b:%s \n",b);
 free(b);
 return SUCCESS;
}

I am writing a simple program without the use of predefined function for string reversal. But this throws me a segmentation fault. I beleive i'm accessing the correct memory address.

EDITED:

#include<stdio.h>
#include<malloc.h>
#include<string.h>
#define SUCCESS 0
#define FAILURE -1
int str_rev(char *s, char **d){
  int count = 0;
  if(s == NULL || d == NULL){
   printf("\n Invalid address received! \n");
   return FAILURE;
  }
  else{
   while(*s != '\0'){
    s++;count++;
   }
   s--;
   while(count > 0){
    printf("\n *s:%c \n",*s);   // prints the values correctly in the reverse order
    *(*d)++ = *s--;count--;
    printf("\n **d:%c \n",*((*d)-1)); // doesnt print the values, after the assignement
   }
   **d = '\0';
   printf("\n s:%s *d:%s \n",s,*d); // both s and d doesnt print the values copied
   return SUCCESS;
  }
}
int main(){
 int ret_val = SUCCESS;
 char *a = "angus";
 char *b,*x;
 b = malloc((strlen(a) * sizeof(*a)) + 1);
 x = b;
 if(b == NULL){
 }
 ret_val = str_rev(a,&b);
 if(ret_val == FAILURE){
   printf("\n String is not reversed! going to quit! \n");
   free(b);
   return FAILURE;
 }
 printf("\n b:%s \n",b);
 free(b);
 return SUCCESS;
}

I changed the code as above, as 'a' contains the string. hence a single pointer is enough to point to that location as no changes needs to be done. But even after this above change The contents in 's' are not getting copied to 'd'. And i'm getting a seg fault after printing "printf("\n b:%s \n",b);" .

Answer 1

In addition to memory allocation problem also second problem in your code:

First
after your copy loop:

   while(count > 0){
    **d++ = **s--;count--;
   }

You do not terminate d string ny null

add

**d= '\0';

Second: after your first loop

   while(**s != '\0'){
    **s++;count++;
   }

You copy from Null do destination first char become '\0' then how you can print using %s

you should decrements s to point back to last char instead of null char. by --s.

Third

memory allocation do like:

 char *a = "angus";
 char *b;
 b = malloc(strlen(a)*sizeof(*a) + 1);

don't forget to free() memory for b

Four
Next is you forgot to return return SUCCESS; from str_rev()

Firth

you are passing pointer to pointer that change the b and 's' value it self in calling function. When you call with &s and modifies s then no string points to "angus" string.

Do like below I coded you logic using single pointer instead.

int str_rev(char *s, char *d){
  int count = 0;
  if(s == NULL || d == NULL){
   printf("\n Invalid address received! \n");
   return FAILURE;
  }
  else{
   while(*s != '\0'){
    s++;
    count++;
   }
   count;
   --s;
   while(count > 0){
    *d = *s;
   // printf("\n %c %c", *d, *s);
    d++ ;
    s--;
    count--;
   } 
   *d = '\0';
  }
  return SUCCESS;
}

in main just call as:

ret_val = str_rev(a, b);

EDIT: Second Code

I notice you are not happy with my suggestion to use single pointer for both!

Well in your second (EDIT) there are some repeating errors:

(1): From function str_rev() your again forgot to return SUCCESS.
(2): Your syntax for str_rcv function is int str_rev(char *s, char **d), first argument is char* but in main() you call it like ret_val = str_rev(&a,&b); that is wrong incompatibly pointer assignment. you should call like:

ret_val = str_rev(a, &b); 

(3): IMPORTANT FOR YOU: In second argument you are passing &b Where as in str_rev() function you are updating d pointer hence updating b to which you allocated memory through malloc(), You can't do that!
This will cause an error also: memory clobbered before allocated block

You should rectify your code to call like this: (read comments please)

 b = malloc((strlen(a) * sizeof(*a)) + 1);
 if(b == NULL){
   return FAILURE; // added this line too 
 }
 char* x = b;  // first assign b to x, notice x and b are of 
               // same type char*
 ret_val = str_rev(a,&x);  // know pass &x instead of &b 

(4): although my previous code also working get new version too:

#define SUCCESS 0
#define FAILURE -1
int str_rev(char *s, char **d){
  int count = 0;
  if(s == NULL || d == NULL){
   printf("\n Invalid address received! \n");
   return FAILURE;
  }
  else{
   while(*s != '\0'){
    s++;count++;
   }
   s--;
   while(count > 0){   
    *(*d)++ = *s--;
    printf("\n *s:%c And **d: %c\n",*(s+1), *((*d)-1));  // for bug finding 
                             // because s decremented and d incremented  
     count--;

   }
   **d = '\0';

   return 0;
  }
}

the main function():

int main(){
 int ret_val = SUCCESS;
 char *a = "angus";
 char *b;
 b = malloc((strlen(a) * sizeof(*a)) + 1);
 if(b == NULL){
   return -1;
 }
 char* x = b;
 ret_val = str_rev(a,&x);
 if(ret_val == FAILURE){
   printf("\n String is not reversed! going to quit! \n");
   free(b);
   return FAILURE;
 }
 printf("\n b:%s \n",b);
 free(b);
 return SUCCESS;
}

Its working Output is:

 *s:s And **d: s

 *s:u And **d: u

 *s:g And **d: g

 *s:n And **d: n

 *s:a And **d: a

 b:sugna 

Here your running code at Codpad

Answer 2

Here:

**s++

you are incrementing the char ** s. This alters it to point to the next char * which is not meaningful in your program.

Because of operator precedence, **s++ is the same as (*(*(s++)). Which is to say, it returns the value of the char pointed to by the char * pointed to by s, and as a side-effect increments s to point to the 'next' char * (which isn't well-defined because you do not have an array of char *s).

A typical idiom in C string manipulation is *p++, which is the same as (*(p++)). This returns the value of the char pointed to by p and as a side effect sets p to point to the next char, which would be the next character in the string. To do the same with a char ** one must write *(*p)++, or more explicitly (*((*p)++)).

Also, it is not necessary to use char **s to reverse a string; it can be done with only char *s.

Answer 3

In this line

b = malloc(sizeof(b));

sizeof(b) is just the size of a pointer and that is not ehough to fit a whole string.

Either pass the size you want to malloc

b = malloc(42);
b = malloc(strlen(a) + 1);

or change b into an array instead of a pointer

char b[42];

---

Other than that, I would highly recommend learing to use tools like gdb or valgrind to debug these segmentation faults. At the least they will tell you what line is segfaulting and just that will help a lot.

Answer 4

Change ret = str_rev(&a,&b); to ret_val = str_rev(&a,&b);

Answer 5

Please find a reworked function as below

int str_rev(char **sinp, char **dout){
    int count = 0;
    char *s = *sinp; // Dereference the starting address of source
    char *d = *dout; // Dereference the starting address of destination

    if(s == NULL || d == NULL){
        printf("\n Invalid address received! \n");
        return FAILURE;
    }
    else{
        while(*s != '\0'){
            *s++;count++;
        }
        s--; // Requires a decrement as it is pointing to NULL

        while(count > 0){
             *d++ = *s--;
             count--;
        }
    }
    *d = '\0'; // Need a NULL terminator to convert it to string
    return SUCCESS; // Requires a return as all branches of function should return values
}

Answer 6

char str[] = "hello";
char *foo = foo;

*foo++ is the equivalent to *(foo++), not (*foo)++. foo++ causes foo to point to the next char.

char str[] = "hello";
char *foo = str;
char **bar = &foo;

**bar++ is the equivalent to **(bar++);, not (**bar)++. bar++ causes bar to point to the next char *... Do you see a problem?

(*bar)++ causes foo to point to the next char. consider what you'd be doing to your caller's a and b variables in ret_val = str_rev(&a,&b);, (*s)++; and (*d)++.