-1
string key = "YTNSHKVEFXRBAUQZCLWDMIPGJO";
    for (int i = 0; i < 25; i++)
    {
        key[i] = toupper(key[i]);
    }
    printf("%s", key);

I am using the cs50 header file for the string datatype. I wrote some code related to encrypting as a problem set in the cs50 course. The code compiled but didn't completely execute. The exit status came as False.

I also tried printing some stuff in the later half of the code I had written but it didn't print. I step by step commented the rest of the code to check what was creating the problem, and it came out be the above lines. These lines of code compile but and even the executable runs properly, but there is no output. For ex.

string key = "YTNSHKVEFXRBAUQZCLWDMIPGJO";
    for (int i = 0; i < 25; i++)
    {
        key[i] = toupper(key[i]);
    }
    printf("%s", key);
    
    printf("Hello");

If I ran this, even the bottom printf hello wouldn't print.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
TheThinker0x4D2
  • 11
  • 2
  • You cannot modify a string literal. `char key[] = "YTNSHKVEFXRBAUQZCLWDMIPGJO";` would work. – Retired Ninja Feb 22 '23 at 06:38
  • You should also use "%c" to print a character – Emanuel P Feb 22 '23 at 06:45
  • 1
    When you get the "string literal" part working, be advised that your string is 26 letters long (plus a null terminator)... `i < 25` is not going to reach the end... – Fe2O3 Feb 22 '23 at 06:49
  • TL;DR: the root of your problem is the dysfunctional CS-50 class. Consider switching to some better form of learning material which doesn't mystify C strings or try to hide basic C functionality underneath the carpet. – Lundin Feb 22 '23 at 14:11

1 Answers1

2

string key = "YTNSHKVEFXRBAUQZCLWDMIPGJO"; is a misleading definition. The type string is a typedef for char *, so it defines a pointer key initialized with the address of a 27 byte string literal (26 letters and a null terminator).

String literals must not be modified in C, they really should have a type const char[], but for historical reasons, the C Standard allows them to be used as initializers for char * pointers, but it is the responsibility of the programmer to not attempt to modify them.

As you modify the contents of this string, the code has undefined behavior. The fact that the modification is moot (the contents are already upper case) does not matter, storing a value to key[i] has undefined behavior. Undefined behavior means anything can happen: the program can stop and exit with or without an error, it can seem to work but have other side effects, or not...

You should instead define key as an array of char:

#include <stdio.h>

int main() {
    char key[] = "YTNSHKVEFXRBAUQZCLWDMIPGJO";

    printf("Hello, initial key is %s\n", key);
    for (int i = 0; key[i] != '\0'; i++) {
        key[i] = toupper((unsigned char)key[i]);
    }
    printf("new key: %s\n", key);
    return 0;
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • Anyway, the original example string doesn't produce any change, as they're already all uppercase - this is to prevent the next "Hey, but I don't see any difference!" – LuC Feb 22 '23 at 09:21
  • @LuC: yes indeed, and I did mention *The fact that the modification is moot (the contents are already upper case) does not matter*. I guess a smart compiler could identify this and not produce the offending code, but it would be more productive if the compiler identifies the attempted modification of a string literal and report this problem. Incidentally, compiling with proper flags `-Wall -Werror` would stop at `string key = "YTNSHKVEFXRBAUQZCLWDMIPGJO";` – chqrlie Feb 22 '23 at 09:39