It is my first day writing programs in C and am confused as to why this code does not work. I have programmed before in C#.
Essentially, I am trying to initialise an empty string called onesString and run the C equivalent of what onesString += "I" is in, say, C#. It is not letting me access the memory and overwrite the string. This is within the context of trying to convert an integer into a roman numeral.
Here is the code, which I have tried to piece together from tutorials online:
#include <stdio.h>
char * ToRomanNumeral(int integer) {
if (integer > 10) {
return "Not implemented";
}
int ones = integer % 10;
char * onesString = malloc(1000);
//makes the ones string
if (ones == 9) {
strcpy(onesString, "IX");
}
else if (ones >= 5) {
strcpy(onesString, "V");
}
else if (ones == 4) {
strcpy(onesString, "IV");
}
for (int i = 0; i < ones % 5; i++)
{
strcpy(onesString, strcat(onesString, "I"));
}
return onesString;
}
int main() {
// printf() displays the string inside quotation
printf("Hello, World!");
printf(ToRomanNumeral(1));
return 0;
}
The issue occurs on the line strcpy(onesString, strcat(onesString, "I"));. It gives me the error Access violation reading location.
I do not understand what is wrong with this, as I have allocated a lot of space with the malloc(1000) line of code.
I have also tried instead initialising onesString as char onesString[] = "";, char * onesString = ""; but nothing seems to work.
What can I do to get the desired result?
7
// MAKE SURE TO INCLUDE THE RIGHT HEADERS
#include <stdlib.h> // malloc
#include <string.h> // string manip. functions
char* onesString = malloc(1000);
// like John Bollinger mentioned, the memory needs to be zero initialized
// (theoretically only the first character, but better to be safe than sorry)
memset(onesString, 0, 1000);
strcat(onesString, "I");
C will tolerate not having the headers included directly, but it will give you a warning. And doing it propperly is better for the compiler and the LSP. You never kwow when an obsolete feature will get deprecated.
The faster version for appending a single character in a loop would look something like that:
int end = strlen(onesString);
for (int i = 0; i < ones % 5; i++)
{
onesString[end++] = 'I';
}
No need to seek the null terminator every time.
Just one more little thing, the loop falsely adds I to strings that actually intended for subtracting. A little change to your logic can fix that:
char * ToRomanNumeral(int integer) {
int ones = integer % 10;
char * onesString = malloc(16); // maximal length reachable
if (integer > 10) {
// always return an allocation, so you can always free it
// mentioned by Explorer09
strcpy(onesString, "not implemented");
return onesString;
}
// makes the ones string
if (ones == 9) {
strcpy(onesString, "IX");
}
else if (ones == 4) {
strcpy(onesString, "IV");
}
else
{
if (ones >= 5) {
strcpy(onesString, "V");
}
for (int i = 0; i < ones % 5; i++)
{
strcat(onesString, "I");
}
}
return onesString;
}
3