HiveBrain v1.2.0
Get Started
← Back to all entries
patterncModerate

Sample printf implementation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
implementationsampleprintf

Problem

Here is my code for printf() implementation in C. Please share your thoughts for improvements.

#define PRINTCHAR(c) putchar(c)
#define MAXLEN 256

// Write a printf function which simulates printf() in C.
int formatAndPrintString(char *pcFmt, va_list lList)
{
    //Holds the number of characters printed, 0 on error
    int nChars = 0;

while(*pcFmt != '\0')
{
    if(*pcFmt == '%')
    {
        pcFmt++;

        int iTemp = 0;
        char acTemp[MAXLEN] = ""; 
        char *pcTemp = NULL, cTemp = 0;

        switch (*pcFmt)
        {
        case 'd':
            iTemp = va_arg(lList, int);
            itoa(iTemp, acTemp, 10); 

            for(int i = 0; i < strlen(acTemp); i++)
            {
                PRINTCHAR(acTemp[i]); nChars++;
            }
            break;
        case 's':
            pcTemp = va_arg(lList, char*);

            while(*pcTemp != '\0')
            {
                PRINTCHAR(*pcTemp); nChars++;
                pcTemp++;
            }
            break;
        case 'c':
            cTemp = va_arg(lList, int);  
            PRINTCHAR(cTemp);
            nChars++;
            break;
       default:
          break;
       }
       pcFmt++;
    }
    else 
    // print the char to console
    {
       PRINTCHAR(*pcFmt);
       pcFmt++; nChars++;
    }
}
return nChars;
}

// Print function identical to printf() in stdio.h
int printfunc(char *pcFormat, ...)
{
    if(!pcFormat)
    {
        return 0;
    }

    va_list lList;
    va_start(lList, pcFormat);

    return(formatAndPrintString(pcFormat, lList));
}

Solution

Bugs

There is no call to va_end()

The va_start() macro must be called first, and it initializes ap, which can be passed to va_arg() for each argument to be processed.  Calling
 va_end() signals that there are no further arguments, and causes ap to be invalidated.  Note **that each call to va_start() must be matched by a
 call to va_end(), from within the same function**.


Char and Int do not have the same size.

This advice was wrong:

case 'c':
        cTemp = va_arg(lList, int);


I am surprised this passes your unit tests. It will read a value off the parameter list but also advance it sizeof(int) places. While a character is only of length sizeof(char).

Note  sizeof(int) > sizeof(char)
      sizeof(char) == 1


I am going to leave the above in (because it is a learning experience for me).

From: @PeteBecker C99 Section: 6.5.2.2/7


If the expression that denotes the called function has a type that does include a prototype, the arguments are implicitly converted, as if by assignment, to the types of the corresponding parameters, taking the type of each parameter to be the unqualified version of its declared type. The ellipsis notation in a function prototype declarator causes argument type conversion to stop after the last declared parameter. The default argument promotions are performed on trailing arguments.

Thus the char argument is promoted to int when effectively pushed to the stack. So you need to use int when reading it from the var args list. Thus the code is correct.

Unchecked increment:

default:
          break;
       }
       pcFmt++;


Pretty sure this would break if I used the string "%"

Notes:

Whenever you call PRINTCHAR() you also execute nChars++; so why not put both into the same functioin.

Extensions you should add in the long run.

You cover the bare minimum

You have format d/s/c but in printf there are a lot more with a lot more options: nore Types, type Extension (long/short), width, more formatting, etc.

No Locale Coverage

Output printing is affected a lot by current locale.

Code Snippets

The va_start() macro must be called first, and it initializes ap, which can be passed to va_arg() for each argument to be processed.  Calling
 va_end() signals that there are no further arguments, and causes ap to be invalidated.  Note **that each call to va_start() must be matched by a
 call to va_end(), from within the same function**.
case 'c':
        cTemp = va_arg(lList, int);
Note  sizeof(int) > sizeof(char)
      sizeof(char) == 1
default:
          break;
       }
       pcFmt++;

Context

StackExchange Code Review Q#96354, answer score: 10

Revisions (0)

No revisions yet.