Pointer issue in C - what am I doing wrong here?

Question

For a bit of background, I'm writing a meter reading application in C for a small 16-bit handheld computer that runs a proprietary version of DOS.

I have a screen that displays meter information and prompts the user to type in a reading. When the user presses the enter key on the unit, the following code will execute:

/* ...
 * beginning of switch block to check for keystrokes
 * ...
 */
case KEY_ENTER: {
    /* show what has been entered */
    if(needNew == 0) {
        /* calculate usage for new reading */
        double usg = 0;
        int ret = CalculateNewUsage(vlr, buf, &usg);
        VerifyReadScreen(vlr, ret, buf, &usg);
        needRedraw = TRUE;
    }
    break;
}
/* .... end switch statement */

vlr is a pointer to a struct that holds all account/meter information, buf is of type char[21] used to store numerical keystrokes for the reading which is handled above this block. My variables all contain valid data when I check them both before and after calling CalculateNewUsage.

However when I check variable data again after entering VerifyReadScreen, newread is pointing somewhere random in memory and returns what looks like a copyright notice. The interesting thing is no matter what account or what reading I enter - the same invalid data for newread in VerifyReadScreen is printed on the screen. I am passing the address to VerifyReadScreen in the same manner as CalculateNewUsage, but somehow I've ended up with something different.

Here is VerifyReadScreen:

BYTE VerifyReadScreen(const VLRREC * vlr,
                        const int status,
                        const char * newread,
                        const double * usage) {

    /* snip a whole bunch of irrelevant formatting code */

    printf("%s", (*newread)); /* prints funky copyright text */

    /* snip more irrelevant formatting code */
    return TRUE;
}   

Thank you to Jefromi for pointing out that the code where I am actually printing newread in VerifyReadScreen should really read:

printf("%s", newread); /* yay! */

because I didn't need to dereference newread because printf does this for me. I was essentially passing a pointer to a pointer which was some arbitrary place in memory.

Answer 1

I think I'm confident enough to post this as an answer:

BYTE VerifyReadScreen(const VLRREC * vlr, const int status, const char * newread, const double * usage) {
...
    LCD_set_cursor_pos(19 - strlen(newread), 3);
    printf("%s", (*newread)); /* prints funky copyright text */
...
}

You've got a string (char*) newread, but in that printf, you're dereferencing it, which gives you the first character of the string. You then use it as the argument to a %s for printf, so it tries to go to the memory address given by that character and print what it finds there.

P.S. You got unlucky - generally, doing something like this is likely to give you a segfault, so you can track it down to that line and realize there's a pointer error right there.

Answer 2

I don't known if that's the problem you are facing, but the buffer used in VerifyReadScreen, which is 21 characters long, will most likely overflow:

if(strlen((*vlr).ServAdd) >= 20) {
    sprintf(buffer, "%20s", (*vlr).ServAdd);
}

The %20s format specifier doesn't prevent sprintf to write more than 20 characters. It just pads the string with spaces if it's shorter than 20 characters (or did you want <= 20 in the if-condition?).

else {
    memset(buffer, 0x20, (int)(strlen((*vlr).ServAdd) / 2) + 1);
    strcat(buffer, (*vlr).ServAdd);
}

Here some padding is done depending on the strings length, but I don't see how it would make sure that the result isn't longer than 20 characters.