3

I've created a function to convert a number into a roman numeral. I know the logic of the conversion itself is correct, however, each time strncpy is called, it overwrites the previous value of "rom". I even tried calling it back to back and it only returned the latter.

Here's a snippet from the code:

   char* rom = (char*) calloc (10,sizeof(char));

    while(intval>=1000){
        intval -= 1000;
    strncpy(rom,"M",2);
    }

Maybe using calloc is part of the issue, but I tried using malloc and it gave me the same result.

stan
  • 4,885
  • 5
  • 49
  • 72
  • You can post code in your question, and it's preferred so that if an external site goes down, links don't break and questions don't become unhelpful. – Seth Carnegie Sep 10 '11 at 00:13
  • Two nit-picks. First - `sizeof(char)` is `1`, why bother writing all that out? Second - don't cast the return value from `calloc()` - it can hide `#include` bugs that will come back to bite you as linker errors. – Carl Norum Sep 10 '11 at 00:15
  • 2
    strncpy is doing what strncpy does. And in your case, since all your source values are literals and you're specifying the length of the literal including null terminator, it does exactly what strcpy does. – Hot Licks Sep 10 '11 at 00:16
  • Note that you could finesse the problem by incrementing `rom` after every assignment to it. Eg, `strcpy(rom,"M"); rom += strlen("M");` Then `rom` is always pointing to the next place to store, and you can do the cheap strcpy vs the (slightly) more expensive strcat. (Of course, you need to keep a pointer to the original start of `rom` to be able to access the final result.) – Hot Licks Sep 10 '11 at 00:26

2 Answers2

11

you want to append, but strcpy just copies to the address (and overwrites). use strcat or strncat

Foo Bah
  • 25,660
  • 5
  • 55
  • 79
3

I believe you want strcat() and not strcpy()

Hogan
  • 69,564
  • 10
  • 76
  • 117