CODEWITHSUNDEEP
c
Lyesmith
Lyesmith

Reputation: 35

Why is this code giving segmentation fault?

I'm trying to write a program that takes any number of one-word text string arguments, each less than 128 characters long. The program copies text from stdin to stdout, except that any of the words seen in the input are replaced with the word "CENSORED".

Example: I have this file called poem.txt:

Said Hamlet to Ophelia,
I'll draw a sketch of thee,
What kind of pencil shall I use?
2B or not 2B?

The program should do this:

./censor Ophelia < poem.txt
Said Hamlet to CENSORED,
I'll draw a sketch of thee,
What kind of pencil shall I use?
2B or not 2B?

Here's my code:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
int main(int argc, char *argv[])
{
    char lines[200][200];
    int numLines=0,i,j;
    int nbytes = 128;
    int bytes_read=0;
    char *my_string;
    char * pch;
    //reading from stdin
    while(stdin)
    {
    my_string=(char *) malloc (nbytes + 1);
    bytes_read = getline (&my_string, &nbytes, stdin);  
    strcpy(lines[numLines++],my_string);
    }

    //scanning and replacing specified words by "CENSORED"
    for(i=0;i<argc;i++)
    {
        for(j=0;j<numLines;j++)
        {   
             pch = strstr (lines[j],argv[i]);
             strncpy (pch,"CENSORED",8);
        }
    }
    //display the result in output screen
    for(j=0;j<numLines;j++)
        {
            printf("\n%s",lines[i]);
        }

}

The problem is that this is giving segmentation fault, but I can't identify the mistake.

Upvotes: -1

Views: 648

Answers (1)

cdlane
cdlane

Reputation: 41925

You're not properly overwritting a hit with the replacement which might be longer or shorter -- you're just stuffing it in regardless (potentially overwriting the terminal \0, possibly leading to your segmentation fault). Also, it looks like you miss double hits as you only check each command line word once against each line. Finally, you've made this more complicated by storing all the lines -- no line affects any other so why store them rather than process and print each line in turn?

Here's a overall simplified approach with more detailed replacement code:

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

#define REPLACEMENT "CENSORED"
#define BUFFER_SIZE (1024)

int main(int argc, char *argv[])
{
    ssize_t bytes_read;
    char *s, *line = malloc(BUFFER_SIZE);
    size_t nbytes = BUFFER_SIZE, replacement_length = strlen(REPLACEMENT);

    // read from stdin
    while ((bytes_read = getline(&line, &nbytes, stdin)) != -1)
    {  
        // scanning and replacing specified words
        for (int i = 1; i < argc; i++)
        {
            while ((s = strstr(line, argv[i])) != NULL)
            {
                size_t search_length = strlen(argv[i]);
                size_t tail_length = strlen(s + search_length);

                (void) memmove(s + replacement_length, s + search_length, tail_length + 1);
                (void) memcpy(s, REPLACEMENT, replacement_length);
            }
        }

        // display the result in output screen
        (void) fputs(line, stdout);
    }

    free(line);
}

Oh yeah, and you forgot to free what you malloc'd. And you're searching for the name of the program as one of your targets...

EXAMPLE

> ./a.out pencil 2B < poem.txt
Said Hamlet to Ophelia,
I'll draw a sketch of thee,
What kind of CENSORED shall I use?
CENSORED or not CENSORED?

Upvotes: 2

Related Questions