Segmentation fault when converting string to upper case

Question

Segmentation fault: could someone help me to understand my mistake?

Target: make a new string with only CAPS LETTERS.

Also, I am trying to identify letters I do not want by referring to the ASCII table, hopefully, that is the right approach.

CS50 IDE, going through CS50 course by Harvard

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

// Points assigned to each letter of the alphabet
int POINTS[] = {1, 3, 3, 2, 1, 4, 2, 4, 1, 8, 5, 1, 3, 1, 1, 3, 10, 1, 1, 1, 1, 4, 4, 8, 4, 10};

// int compute_score(string word);
int put_down_caps_only(string word);

int main(void)
{

    string word1 = get_string("Player 1: ");
    string word1CapsOnly="";
    char chr;
    
    for(int i=0; i<strlen(word1);i++)
    {
        if(word1[i]<123&&word1[i]>96)
        {
            // use to upper function
            // is lower would work here nicely
            chr = toupper(word1[i]);
            strncat(word1CapsOnly, &chr, 1);
            
        }
        else if(word1[i]<65 || word1[i]>122)
        {
            //ignore
        }
        else
        {
            //just add, upper alredy
            strncat(word1CapsOnly, &word1[i], 1);
        }
        
    }
    printf("%s", word1CapsOnly);
//     int score1 = compute_score(word1);
    // TODO: Print the winner
}

/* int compute_score(string word)
{
    // TODO: Compute and return score for string
    
} */

So you added the tags c#, c++ and c. These are three different environments. Please clarifiy, in what language you write the code — TheTanic, Jul 06 '21 at 11:46
AFAIK there is no `string` in C ... is it a `typedef` to `char *` ? — Selvin, Jul 06 '21 at 11:50
Please show a [mre]. If that's your first post you should have taken our [tour] an read [ask]. And maybe take a look at the [reference for `strncat`](https://en.cppreference.com/w/cpp/string/byte/strncat), I don't think you understood what it does. — Lukas-T, Jul 06 '21 at 11:51
Please post your whole code including all functions/headers. — kiner_shah, Jul 06 '21 at 11:56
@user3121023 , I will try to use it later once I understand why It is not working — Igor Shevelev, Jul 06 '21 at 11:56
I am not sure why you need `word1CapsOnly`, you can easily modify `word1` and replace lower case characters with upper case. BTW, instead of using integers to represent ASCII, you can use characters in single quote e.g. `'A'` — kiner_shah, Jul 06 '21 at 12:02
@user3121023, I might be wrong, but I am trying to use this function: strncat, which is part of string.h library and it would add chars to the end of the string. — Igor Shevelev, Jul 06 '21 at 12:03
You probably don't need that function. If you want the string `word1CapsOnly` for some reason, just copy `word1` using `strncpy()` and then change characters to upper case in the copy. — kiner_shah, Jul 06 '21 at 12:04
@kiner_shah, yep, thank you, I will remove it later, I do not need to keep original word indeed — Igor Shevelev, Jul 06 '21 at 12:05
@selvin Welcome to CS50, where to avoid confusion they pretend that C *does* have a `string` type. (Needless to say this fiction does not, in fact, avoid confusion.) — Steve Summit, Jul 06 '21 at 12:23
Please, do not edit this question to ask a new question. If you have a new question, [ask a new question](https://stackoverflow.com/questions/ask). — Dada, Jul 06 '21 at 18:30

score 0 · Answer 1 · answered Jul 06 '21 at 17:42

The do while loop in certain circumstances increments int n until after wrapping around it reaches some target value to no avail other than taking minutes; it not even computes the correct wordScore. You better drop the whole body of the for loop and, since you obviously only cater for ASCII, replace it by the simple statement:

      if (word[i]>='A' && word[i]<='Z') wordScore += POINTS[word[i]-'A'];

score 0 · Answer 2 · answered Jul 06 '21 at 18:40

Your code does not allocate any space for word1CapsOnly - it just initializes it to point to a character array which consists of a single byte containing the NUL (zero) character. The easiest thing to do is to allocate sufficient space to begin with:

string word1CapsOnly = malloc(strlen(word1)+1);
word1CapsOnly[0] = '\0';

In cs50.h, string is just a synonym type for char * - it's not like it's going to do any memory management for you. As with most things string-ish in C, that's all on you.

Segmentation fault when converting string to upper case

2 Answers2