1

so I've been trying to write this piece of code in c where the basic idea is that the user enters a positive integer and the program prints the digits in that number, and I've done it recursively but the thing is, when I compile it, I get no errors but the output is just some trash, on the other hand, I've tried debugging and so I've set some breakpoints and I keep getting fine results all the way and correct output while debugging, unlike when trying to compile the exact same piece of code If anyone has any idea why this is so, it would be really appreciated

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

char *numToWord (int number){
    char *newDigit = NULL;
    switch(number){
    case 0:
        newDigit = malloc (5*sizeof(char));
        sprintf(newDigit,"zero");
        break;
    case 1:
        newDigit = malloc (4*sizeof(char));
        sprintf(newDigit,"one");
        break;
    case 2:
        newDigit = malloc (4*sizeof(char));
        sprintf(newDigit,"two");
        break;
    case 3:
        newDigit = malloc (6*sizeof(char));
        sprintf(newDigit,"three");
        break;
    case 4:
        newDigit = malloc (5*sizeof(char));
        sprintf(newDigit,"four");
        break;
    case 5:
        newDigit = malloc (5*sizeof(char));
        sprintf(newDigit,"five");
        break;
    case 6:
        newDigit = malloc (4*sizeof(char));
        sprintf(newDigit,"six");
        break;
    case 7:
        newDigit = malloc (6*sizeof(char));
        sprintf(newDigit,"seven");
        break;
    case 8:
        newDigit = malloc (6*sizeof(char));
        sprintf(newDigit,"eight");
        break;
    case 9:
        newDigit = malloc (5*sizeof(char));
        sprintf(newDigit,"nine");
        break;
    }
    return newDigit;
}

char * writeAbsolute (int number) {
    char * word = NULL; // variable where the return value will be stored, must clean in main part
    int digit;
    digit = number % 10;
    if (number){
        number /= 10;
        word = writeAbsolute (number);
        char *newDigit = NULL;
        char *newWord = NULL;
        newDigit = numToWord(digit);
        if (word){
            int numOfLetters = (int)( strlen(newDigit)+strlen(word) );
            newWord = (char*) malloc(numOfLetters * sizeof(char));
            sprintf(newWord,"%s %s",word,newDigit);
            word = newWord;
        }
        else{
            word = newDigit;
        }
        free (newWord);
        free (newDigit);
    }
    return word;
}


int main() {
    int number;
    char * word;
    printf("Enter an integer:\n");
    scanf("%d",&number);
    word = writeAbsolute(number);
    printf("%s",word);
    return 0;
}
  • Firstly, strings are NULL-terminated (not the same as a trailing space) [check your sizes]. – polarysekt Sep 20 '14 at 21:30
  • Also, obligatory link to [Do I cast the result of malloc](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – polarysekt Sep 20 '14 at 21:34
  • thanks, I removed those unnecessary malloc castings and changed these "one ","two ","three "... to "one","two","three"... and fixed their sizes respectively (I still do have to allocate that additional sizeof(char) memory in addition to the length of the string, for the '\0' at the end of each string, am I right or not?), but as those trailing whitespaces were just for the sake of print formatting anyway, I got no change there, still getting some trash output –  Sep 20 '14 at 21:59
  • I'm chalking up an answer, but it has to do with the return of `strlen()`, which returns the number of characters in the string up to, but not including the NULL-terminator. You can still pad the strings with whitespace if you like, just make sure you make enough room for the NULL terminator. – polarysekt Sep 20 '14 at 22:02
  • NOTE: I scrapped my answer as I got distracted (hungry cat) and was slow to the draw. The accepted answer also takes into account that `sizeof(char)==1` and your call to `free()` on `word`. – polarysekt Sep 20 '14 at 22:13

1 Answers1

1

The first issue is that you are not allocating enough memory for all your strings. The string "1234" should be allocated with 5 characters due to the inherent trailing '\0' terminating byte. Simply add at least one to all your string allocation lengths:

newDigit = malloc(5*sizeof(char));
sprintf(newDigit,"six ");
...
int numOfLetters = (int) (strlen(newDigit) + strlen(word) + 1);

Also note that sizeof(char) is defined to be 1.

The second issue is that your are freeing your strings immediately after using them in this code:

if (word) {
    ...
    newWord = (char *) malloc(...);
    ...
    word = newWord;
}
else {
    word = newDigit;
}

free (newWord);
free (newDigit);

Assuming word is not NULL, when you free newWord at the end then word becomes invalid as it just points to the same string. Thus, the next time you try to use word you invoke undefined behaviour by trying to use previously freed memory.

What you probably want in this case is something like:

if (word) {
    int numOfLetters = (int)( strlen(newDigit)+strlen(word) + 1);
    newWord = (char*) malloc(numOfLetters);
    sprintf(newWord,"%s%s", word, newDigit);
    word = newWord;
    free(newDigit);
}
else {
    word = newDigit;
}
// Don't free anything else here
uesp
  • 6,194
  • 20
  • 15
  • thanks a lot, both of you guys! it was that freeing I shouldn't have done that caused all the trouble, gonna have to be more careful next time around about which memory I can free and which I still will be using. I voted your answer, thanks again –  Sep 20 '14 at 22:14