|  | Seg-fault in recursion |  | |
| | | Newbie |  |
| Posted: Wed Jul 09, 2008 1:31 pm Post subject: Seg-fault in recursion |  |
| |  | |
Sorry if this appears twice.
Input : Hello I am a newbie Output: newbie a am I Hello
I am getting a segfault on running the progra.The debugger shows that it happens in the reverse routine Can anyone help me fix it ?
#include <stdio.h> #include<string.h> #include <stdlib.h>
void reverse(char **); void print_2_array(char** array1 ,int row,int col);
void reverse(char **words) { if(*(words + 1) != '\0') { reverse((words + 1)); } printf("%s",*(words + 0)); }
void print_2_array(char** array1 ,int row,int col) { int i, j;
for (i=0; i < row; i++) { for(j=0; j < col; j++) printf("%c", array1[i][j]); printf("\n"); } }
int main() { char line[200]; char **words; int i = 0,j = 0,k = 0; char* a =line;
words = (char**)malloc(10*sizeof(char*)); words[j] = (char*)malloc(15*sizeof(char));
memset(words[j],0,15*sizeof(char));
scanf("%[^\n]s",line);
for(; *a ;i++) { if(*a == ' ') { j++; words[j] = (char*)malloc(15*sizeof(char)); memset(words[j],0,15*sizeof(char)); *(*(words + j) + k) = '\0'; k = 0; } *(*(words + j) + k) = *a; k++; a++; } *(*(words + j ) +k) = '\0'; print_2_array(words,j+1,15); reverse(words); } |
| |
| | | Bartc |  |
| Posted: Wed Jul 09, 2008 1:53 pm Post subject: Re: Seg-fault in recursion |  |
| |  | |
"Newbie" <newbie@gmail.com> wrote in message news:g52ktd$jds$1@aioe.org...
| Quote: | Sorry if this appears twice.
Input : Hello I am a newbie Output: newbie a am I Hello
I am getting a segfault on running the progra.The debugger shows that it happens in the reverse routine Can anyone help me fix it ?
words = (char**)malloc(10*sizeof(char*));
|
Try inserting here:
memset(words,0,10*sizeof(char*));
However there lots of other issues with this program. For one thing mixing a[i] notation with *(a+i) which makes it hard to follow.
Try using proper diagnostic printing such as this, before any output routines; then you will know exactly what is in words:
for (i=0; i<10; ++i) { printf("Words[%d] = %x",i,words[i]); if (words[i]) printf(": \"%s\"",words[i]); printf("\n"); }
Also the constants 10 and 15 should be defined somewhere. The malloc values should be checked against NULL.
For rapid testing, forget the scanf and just use something like strcpy(line, "ABC DEF");
-- bartc |
| |
| | | Ben Bacarisse |  |
| Posted: Wed Jul 09, 2008 2:49 pm Post subject: Re: Seg-fault in recursion |  |
| |  | |
Newbie <newbie@gmail.com> writes:
| Quote: | Input : Hello I am a newbie Output: newbie a am I Hello
I am getting a segfault on running the progra.The debugger shows that it happens in the reverse routine Can anyone help me fix it ?
|
To remove the problem you are seeing, you need to make sure that the condition you test for in 'reverse' will eventually be met. In other words the array of pointers must end with a pointer that compares equal to '\0'[1]. That needs one line at the end of the for loop in main.
To make it better, you could:
- Remove the "magic numbers" (replace then with #defines). - Set and test pointers against NULL rather than '\0'. - Use p[i] syntax rather than *(p + i). - Stop long lines from overflowing your input buffer. - Stop lines with lost of words overflowing your pointer array. - Stop long words in a line overflowing your word arrays. - Do it without recursion. - Print the input without all the null characters. - Print the input using printf rather than with your own function. - Use a simpler data structure (maybe just an array of pointers?). - Use a simpler algorithm (reverse the words in place, maybe?).
[1] I mean that, but see my second improvement.
-- Ben. |
| |
| | | Newbie |  |
| Posted: Wed Jul 09, 2008 3:22 pm Post subject: Re: Seg-fault in recursion |  |
| |  | |
Ben Bacarisse wrote:
| Quote: | Newbie <newbie@gmail.com> writes:
Input : Hello I am a newbie Output: newbie a am I Hello
I am getting a segfault on running the progra.The debugger shows that it happens in the reverse routine Can anyone help me fix it ?
To remove the problem you are seeing, you need to make sure that the condition you test for in 'reverse' will eventually be met. In other words the array of pointers must end with a pointer that compares equal to '\0'[1]. That needs one line at the end of the for loop in main.
To make it better, you could:
- Remove the "magic numbers" (replace then with #defines). - Set and test pointers against NULL rather than '\0'. - Use p[i] syntax rather than *(p + i). - Stop long lines from overflowing your input buffer. - Stop lines with lost of words overflowing your pointer array. - Stop long words in a line overflowing your word arrays. - Do it without recursion. - Print the input without all the null characters. - Print the input using printf rather than with your own function. - Use a simpler data structure (maybe just an array of pointers?). - Use a simpler algorithm (reverse the words in place, maybe?).
[1] I mean that, but see my second improvement.
|
I did not write the code.It looked like a mess initially (far worse) .My friend asked me to debug it :(
This is my solution :
#include<stdio.h> #include<stdlib.h> #include<string.h> #define MAX 50
void reverse(char* begin, char* end) { char tmp; while(begin<end) { tmp = *begin; *begin = *end; *end = tmp; ++begin; --end; } return ; } char *reverse_words(char *str1) { unsigned int i; char *token; reverse(str1,str1+strlen(str1)-1);
token=strtok(str1," "); while(token != NULL) { reverse(token,token+strlen(token)-1); for(i=0;i<strlen(token)-1;token++); *++token=' '; token = strtok(NULL, " "); } return str1; }
int main(void) { char str1[MAX]; printf("Enter the string to be reversed word by word\n");
if(fgets(str1,MAX,stdin)!=NULL) { str1[strlen(str1)-1]='\0'; printf("Original String :%s\n",str1); printf("Reversed String :%s\n",reverse_words(str1)); } else printf("Null String entered"); return 0; } |
| |
| | | Tarique |  |
| Posted: Wed Jul 09, 2008 6:11 pm Post subject: Re: Seg-fault in recursion |  |
Ben Bacarisse wrote:
| Quote: | Newbie <newbie@gmail.com> writes:
Input : Hello I am a newbie Output: newbie a am I Hello
..snip..
To make it better, you could:
- Remove the "magic numbers" (replace then with #defines). - Set and test pointers against NULL rather than '\0'. - Use p[i] syntax rather than *(p + i). - Stop long lines from overflowing your input buffer. - Stop lines with lost of words overflowing your pointer array. - Stop long words in a line overflowing your word arrays. - Do it without recursion. - Print the input without all the null characters. - Print the input using printf rather than with your own function. - Use a simpler data structure (maybe just an array of pointers?). - Use a simpler algorithm (reverse the words in place, maybe?).
[1] I mean that, but see my second improvement.
|
Fixed it ! Thanks |
| |
| | | Barry Schwarz |  |
| Posted: Wed Jul 09, 2008 11:54 pm Post subject: Re: Seg-fault in recursion |  |
| |  | |
On Wed, 09 Jul 2008 22:40:20 +0530, Newbie <newbie@gmail.com> wrote:
snip
| Quote: | I did not write the code.It looked like a mess initially (far worse) .My friend asked me to debug it :(
This is my solution :
#include<stdio.h #include<stdlib.h #include<string.h #define MAX 50
void reverse(char* begin, char* end) { char tmp; while(begin<end) { tmp = *begin; *begin = *end; *end = tmp; ++begin; --end; } return ; } char *reverse_words(char *str1) { unsigned int i; char *token; reverse(str1,str1+strlen(str1)-1);
token=strtok(str1," "); while(token != NULL) { reverse(token,token+strlen(token)-1); for(i=0;i<strlen(token)-1;token++); *++token=' ';
|
These last two lines are an expensive way of saying *(token+strlen(token)) = ' '; or the equivalent token[strlen(token)] = ' '; which would call strlen exactly once instead of n times depending on the length of the current word.
| Quote: | token = strtok(NULL, " "); } return str1; }
int main(void) { char str1[MAX]; printf("Enter the string to be reversed word by word\n");
if(fgets(str1,MAX,stdin)!=NULL) { str1[strlen(str1)-1]='\0'; printf("Original String :%s\n",str1); printf("Reversed String :%s\n",reverse_words(str1)); } else printf("Null String entered");
|
This error message is incorrect. If the user presses ENTER in response to the prompt, fgets will store a '\n' followed by a '\0' in str1 and return the address of str1[0].
If fgets did return a NULL, it means either an error occurred obtaining the data or end of file was encountered before reading the (MAX-1)th character.
Remove del for email |
| |
|
|