I'm trying to call a function inside of a token loop in C. I keep segfaulting?

77 Views Asked by At
#include <stdio.h>
#include <string.h>

void pigLatinConsonant(char input[]) {
    char end[] = "ay";
    int length = strlen(input);
    int first_vowel = -1;
   
    for (int i = 0; i < length; i++) {
        if (strchr("aeiou", input[i])) {
            first_vowel = i;
            break;
        }
    }

    if (first_vowel != -1) {
        char consonant_cluster[length + 1];
        strncpy(consonant_cluster, input, first_vowel);
        consonant_cluster[first_vowel] = '\0';

        strcpy(input, input + first_vowel);
        strcat(input, consonant_cluster);
    }

    strcat(input, end);
    printf("Translated word: %s\n", input);
}

int main() {
    char sentence[100];
    printf("Enter a sentence: ");

    fgets(sentence, sizeof(sentence), stdin);

    char sentence_copy[sizeof(sentence)];
    strcpy(sentence_copy, sentence);

    char *token = strtok(sentence_copy, " ");

    while (token != NULL) {
        pigLatinConsonant(token);
        token = strtok(NULL, " ");
    }

    return 0;
}

Here I tried to call this method from the loop but I think it has something to do with how my function is built. The user enters a sentence, it should move the first two characters, and put them at the end of the word and add ay to it.

3

There are 3 best solutions below

5
Vlad from Moscow On

For starters your code has nothing common with the assignment

The user enters a sentence, it should move the first two characters, and put them at the end of the word and add 'ay' to it.

This statement

strcpy(input, input + first_vowel);

already invokes undefined behavior.

You may not use strcpy when ranges are overlapping.

Instead you should use memmove.

Also this statement

strcat(input, end);

can overwrite memory outside the original array.

You should allocate a new character array within the function that is gretaer by 3 than the length of the original string and build in it a new string.

if to use a variable length array to store the converted string then the function can look the following way as shown in the demonstration program below

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

void pigLatinConsonant( const char *s ) 
{
    const char end[] = "ay";

    size_t length = strlen( s );

    char pig_latin[length + sizeof( end )];

    size_t i = 0;

    while (i != length && strchr( "aeiou", s[i] ) == NULL ) ++i;

    if (i == length)
    {
        strcpy( pig_latin, s );
    }
    else
    {
        strcpy( pig_latin, s + i );
        strncat( pig_latin, s, i );
        pig_latin[length] = '\0';
    }

    strcat( pig_latin, end );

    printf( "Translated word: %s\n", pig_latin );
}

int main( void )
{
    pigLatinConsonant( "pig" );
}

The program output is

Translated word: igpay
0
B. Pantalone On

The problem is that token is a pointer into your original input string, and that string is modified with every call to pigLatinConsonant(token). The function strtok() expects this string not to change.

Instead of copying the sentence into sentence_copy, copy token into a temporary word string and pass that into pigLatinConsonant(word).

0
Fe2O3 On

From the man page for strcpy():
"Caveat**: The strings src and dst may not overlap."
So, using strcpy() to shift left the right-portion of the string causes UB.
(In some implementations, strcpy() begins copying with the '\0' as this can execute faster.)
It pays to make oneself very acquainted with the man pages...


Below shows how to search for the first vowel using a proven Standard Library function, and how to output regions of strings ( more information gleaned by careful study of the man page for printf().) This version includes a hyphen to highlight the expected syllabification.

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

void piggish( char *cp ) {
    size_t vowel1 = strcspn( cp, "aeiou" ); // lowercase only
    if( vowel1 == 0 )
        printf( "%s-ay ", cp );
    else
        printf( "%s-%.*say ", cp + vowel1, vowel1, cp );
}

int main( void ) {
    char str[] = "when in rome do as the romans do\n";

    /* Notice the '\n' that fgets() leaves in the returned buffer
     * Notice the delimiters for strtok() eats that '\n'
     */

    puts( str );
    for( char *cp = str; ( cp = strtok( cp, " \n" ) ) != NULL; cp = NULL )
        piggish( cp );
    putchar( '\n' );

    return 0;
}

Result:

when in rome do as the romans do
en-whay in-ay ome-ray o-day as-ay e-thay omans-ray o-day

Since segments of the received word are output, there's no need for buffers additional to those provided by printf().

Comprehensive testing and increasing robustness is left as an exercise for the OP. For instance, dealing with words separated by a comma.


** "Caveat" is a Latin word. Perhaps expand this code to use a lexicon of commonly used Latin words to avoid pig-ifying those.