Alex Smith
Alex Smith

Reputation: 113

Getting 'Segmentation Fault' when running simple string manipulation program

I'm trying to learn some C and am having a little bit of trouble with manipulating strings. In trying to learn them, I've decided to make a simple spanish verb conjugator, but I'm getting stuck. Right now I'm just trying to drop the last 2 non '\0' of the string and then add a 'o' to it. (For example, for an input of "hablar" I want it to output "hablo"). Here's my code. I've tried to be overly detailed in my comments to hopefully aid in figuring out what I'm missing conceptually.

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

/* Reimplemented the length function of a string for practice */
int len(char *);
void conjugatePresentAr(char *, char *);

int len(char *arr){
    int l = 0;
    while (*arr++ != '\0'){
        l++;
    }
    return l;
}

void conjugatePresentAr(char *verb, char *output){
    output = verb;
    int i = len(verb);
    while (output < (verb + i -2)){
        *output = *verb;
        output++;
        verb++;
    }
    *output = 'o';
    output++;
    *output = '\0';
}

int main(){
    char input[20];
    scanf("%s", input);
    printf("%s\n",input);
    char conjugated[20];
    conjugatePresentAr(input, conjugated);
    printf("%s\n", conjugated);
    return 0;
}

For any input I get Segmentation Fault: 11. I've spent a decent amount of time looking around here and reading through books on pointers but can't quite seem to figure out what I'm messing up. I appreciate your help!

Upvotes: 1

Views: 106

Answers (4)

user3131905
user3131905

Reputation:

In function conjugatePresentAr() you have alterered the argument *output

output = verb;

Is an address affectation, not value. Should reread pointer definition

Upvotes: 1

Weather Vane
Weather Vane

Reputation: 34575

In conjugatePresentAr() you have changed the argument *output, possibly because you thought that copies the string.

output = verb;

so the function doesn't write anything to the string you supplied. Then when you print it, it's still an uninitialised variable.

Upvotes: 3

NadavL
NadavL

Reputation: 400

You can't copy strings (char *) by assignment, like you did here:

output = verb;

What you do here is just change output to point at the input string, so any changes made to one of the strings will also apply to the other one - since they both point to the same memory.

you need to explicitly a function for copying the memory - such as strcpy (make sure to supply a null terminated string) or memcpy.

And, regarding your logic, since you don't really check the string for 'ar' in the end, and just assume there is, why not use something a little simpler like this:

void conjugatePresentAr(char *verb, char *output) 
{
    strcpy(output,verb);
    int len = strlen(verb);
    output[len - 2] = 'o';
    output[len - 1] = '\0';
}

Upvotes: 1

Useless
Useless

Reputation: 67772

int i = len(verb);
while (output < (verb + i -2)){
    *output = *verb;
    output++;
    verb++;
}

will keep going forever: you're chasing (verb + i - 2) as it recedes into the distance (you increment verb inside the loop).

Try something like:

char *end = verb + strlen(verb) - 2;
while (output < end) {
    ...
    verb++; /* this doesn't change end */
}

(and also fix the bug Weather Vane spotted which I entirely missed).


Note: in general, string processing is hard to do well in C, because the built-in facilities are so low-level. It's actually much easier to use C++ with its string and stringstream facilities.

If you're sticking to C, explicitly tracking length and allocated capacity alongside the char pointer (as the C++ string does for you) is good practice. Oh, and there's no obvious benefit to re-writing strlen.

Upvotes: 3

Related Questions