Bare-Bones implementation of printf

Question

I made a bare-bones implementation of printf based on an exercise in Chapter 7, problem 5 of "Pointers on C" by Kenneth A. Reek. What criticism do you have of my writing style?

For instance, do you find I added far too many header files in code. Should I add #ifndef guards for the header files? Do you find any of the functions throughout the implementation too difficult too read?

For instance, the myprintf function is the most important function, as it takes advantage of all other functions throughout the program. Is the way myprintf is implemented with switch statements too messy?

#include <stdio.h>
#include <stdarg.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
#include <math.h>
#include <limits.h>
#include <float.h>
#include <assert.h>

#define MAX_LENGTH 1000

void reverse (char *s) {
    char temp;  

    for ( int i = 0, j = strlen(s)-1; i < j; i++, j--)
    {
        temp = s[i];
        s[i] = s[j];
        s[j] = temp;
    }
}


char * printd(long long int n)
{

  static char d[1000];
  static char abs[1000];
  static char * abs_p = abs;
  static int sign;
  if ( n < 0 )
  {
    *d = ('-');
    n = -n;
  }

  while ( lldiv(n,10).quot != 0)
  {
    *abs_p++ = (lldiv(n,10).rem+'0');

    n = (lldiv(n,10).quot);

  }

  reverse(abs);
  strcat(d,abs);

  return d;
}

char * lltoa(long long int n)
{
    static char s[MAX_LENGTH];

    long long int i, sign;

    if ((sign = n) < 0) /* record sign */
        n = -n;     /* make n positive by n =-n */
    i = 0;
    do      /* generate digits in reverse order */
    {
        s[i++] = lldiv(n,10).rem + '0'; /* get next digit with n % 10 + '0'; */

    } while (n = lldiv(n,10).quot, n > 0);       /* delete it with while ( (n /= 10) > 0 )*/

    if (sign < 0)
        s[i++] = '-';
    s[i] = '\0';
    reverse(s);

    return s;

}

char * lltoa2(long long int n)
{

  char d[MAX_LENGTH];
  char abs[MAX_LENGTH];
  char * abs_p = abs;
  int sign;
  if ( n < 0 )
  {
    *d = ('-');
    n = -n;
  }

  while (lldiv(n,10).quot != 0)
  {
    *abs_p++ = (lldiv(n,10).rem+'0');

    n = (lldiv(n,10).quot);

  } 

    *abs_p++ = (lldiv(n,10).rem+'0'); /* first digit after pos or neg sign must be included */

  reverse(abs); /* contents of abs are originally in reverse order of input */
  d[0] = '\0';
  strcat(d,abs);

  return &d[0];
}

double nround(double input, double power)
{

    double marge = pow(10,power);

    double up = input * marge;

    double result = ((double)(llround(up)))/marge;

    return result;

}

// ftoa rounds accurately up to 15 digits, guranteed

// ftoa can only deal with integral values with an absolute value less than or equal to LLONG_MAX

/* ftoa only works until 9.9999999999999999e15 rounded to

zero decimal places

*/
char * ftoa(const double input, const double power) // pow is the '*' in "%.*f"
{

  const double in = nround(input,power); 

  static char a[MAX_LENGTH] = "\0";

  int j = 0;

  while (j < strlen(a) ) a[j++] = '\0';

  long long int f_to_i = (long long int)(in); 

  strcat(a,lltoa(f_to_i)); strcat(a,".");

  double integral = 0;

  double fraction = 0;

 (in >= 0) ? (fraction = modf(in,&integral)): (fraction = modf(-in,&integral)); /* stores fractional part of input */

  char non_zero_mantissa[1000] = "\0"; /* stores non-zero digits in mantissa after leading zeros following decimal point */

  strcat(non_zero_mantissa,lltoa(llround(fraction*pow(10,power))));


  int i = strlen(non_zero_mantissa);

  while ( i++ < power )
  {
    strcat(a,"0");

  }

  strcat(a,non_zero_mantissa);

    return a;

#if 0
    int c = 0;

    while ( a[c] != '\0') putchar(a[c++]);
#endif
}

//#if 0
int myprintf(char const * s,...)
{

  va_list var_arg;

  char * s_p = (char *)(s-1);

  va_start(var_arg,s);

  int ROUND_TO = 0;

  char rounding[1000];

  while (*++s_p != '\0')
  {
    switch(*s_p)
    {
        case '%':
            {
            switch(*++s_p)
            {

                case 'f':
                    {
                    char const * f_p = ftoa(va_arg(var_arg,double),6);
                    while ( *f_p != '\0') putchar(*f_p++);
                    break;
                    }
                case 'd':
                    {
                    char const * d_p = (char *)lltoa(va_arg(var_arg,long long int));
                    while ( *d_p != '\0') putchar(*d_p++);
                    break;
                    }
                case 's':
                    {
                    char const * string_p = va_arg(var_arg,char *);
                    while ( *string_p != '\0') putchar(*string_p++);
                    break;
                    }
                case 'c':
                    {
                    putchar((char)va_arg(var_arg,int));
                    break;
                    }
                case '.':
                    {
                        if (isdigit(*(s_p+1))) //peek after '.' and see if there is a digit
                        {
                            ROUND_TO = 1;

                            int i = 0;

                            char * r_p = rounding;

                            while (isdigit(*++s_p)) { *r_p++ = *s_p; }

                            *r_p = '\0';

                            if ( *s_p == 'f' ) 
                            {
                                myprintf("%s",ftoa(va_arg(var_arg,double),atoi(rounding)));
                                ++s_p;
                            }
                            else
                            {

                                putchar('%');

                                putchar(*s_p);

                                myprintf("%s",atoi(rounding));

                            }
                        }
                        else
                        {
                            putchar('%');

                            putchar(*s_p++);
                        }



                    }
                default:
                    {
                    putchar(*s_p);
                    break;
                    }
            }
            break;
        }
        default:
            {
            putchar(*s_p);
            break;
            }


  }

}


  va_end(var_arg);
  return 1;
}

//#endif

//#if 0
int main(void) 
{

#if 0
    ftoa(-3.9999,3);
    putchar('\n');
    ftoa(-1.5555,2);
    putchar('\n');
    ftoa(-3.39823929,5);
    putchar('\n');
    ftoa(-3.0000000000000099,15);
    putchar('\n');
    ftoa(-3.6666666666666666,15);
    putchar('\n');
    ftoa(-3.4545,3);
    putchar('\n');
    ftoa(-3.454599,5);
    putchar('\n');
    ftoa(-1.0000000000000009,15);
    putchar('\n');

    ftoa(3.9999,3);
    putchar('\n');
    ftoa(1.5555,2);
    putchar('\n');
    ftoa(3.39823929,5);
    putchar('\n');
    ftoa(3.0000000000000099,15);
    putchar('\n');
    ftoa(3.6666666666666666,15);
    putchar('\n');
    ftoa(3.4545,3);
    putchar('\n');
    ftoa(3.454599,5);
    putchar('\n');
    ftoa(1.0000000000000009,15);
    putchar('\n');

    ftoa(9.9999999999999999e15,14); //FAILS!!!
    putchar('\n');

myprintf("Thatcher Swag\n%s\n%f\n%c\n%d\n","Swiss Cheese",3.45,'T',3535232523);

double a = 3.14, b = 325.3235, c = 790.866;
myprintf("My bank account amount: %f\n",c);

double light_speed = 3.0e8; 

double planck_mass = 2.17647051*pow(10,-8); 

myprintf("Speed of light: %f\n",light_speed*planck_mass);

printf("Speed of light: %f\n",light_speed*planck_mass); //TESTS PASS

myprintf("%f\n",planck_mass);

printf("%f\n",planck_mass); //TESTS_PASS
#endif
myprintf("%.5f\n",43.235278);
myprintf("%.5f\n",3.14159265);
myprintf("%.3f\n",4.9999);
myprintf("%.10f\n",1558.2392038592972022352698);
myprintf("%.15f\n",1.2392038592972022352698); //myprintf can only round a floating-point that has 15 digits, including before mantissa or less
myprintf("%f\n",323.3435);
myprintf("%s %c %.3f %d\n","Swiss",'A',123.3257,1325352);

myprintf("%s\n",lltoa2(32235235));
myprintf("%s\n",lltoa2(232362351));
myprintf("%s\n",lltoa2(LLONG_MAX));
myprintf("My parents gave me %d hot dogs for July 4\n",359);

}
//#endif

Answer 1

There is so much going on here that needs improvement that the correctness of converting double to a string has not been assessed yet. I am doubtful that its correctly rounds edge cases. Example

myprintf("%f\n",0.00000049999999999999993); --> 0.000001
// expect 0.000000

---

... I added far too many header files in code(?)

This is a minor concern. Adding some extra standard C header files is rarely a significant mistake.

Should I add #ifndef guards for the header files?

Code should have 3 portions: myprintf.c for int myprintf(char const * s,...), myprintf.h for its declaration and other important constants, defines, non-static functions and a main.c/test.c to show how to use and test myprintf().

myprintf.h should use guards.

Do you find any of the functions throughout the implementation too difficult too read?

Yes.

Opinion: too much vertical white-space.

Strange use of magic numbers. Why 1000 when MAX_LENGTH or another mcaro-constant could be used. Avoid naked magic numbers like 1000

#define MAX_LENGTH 1000
....
static char d[1000];

Mixed use of static. In both cases, better code would pass in the buffer and eschew static buffers.

static char s[MAX_LENGTH]; // in lltoa()
char d[MAX_LENGTH]; // in lltoa2()

Re-use of very common functions name for a non-function with char abs[MAX_LENGTH];. abs() is a very common standard function and use here as a non-function is very distracting.

Unexplained #if 0 blocks of code. If not needed, then delete them before review. Similar issue of //#if 0.

Inconsistent indent depth increment. Sometimes it is 2, sometimes 4. Hmmmm.

Inconsistent vertical white-space. Tip: use an auto formatter. Life is too short to do this/maintain this by hand.

Is the way myprintf is implemented with switch statements too messy?

case '.': is certainly messy.

---

Errors

Failure with various values

myprintf("%d\n",LLONG_MIN); --> "-(\n".
myprintf("%f\n",DBL_MAX); --> "-9223372036854.775391\n"
myprintf("%f\n",-0.0); "0.000000\n", expected "-0.000000\n".
Use signbit() rather than if (sign < 0).

Invalid test

myprintf("%s %c %.3f %d\n","Swiss",'A',123.3257,1325352); passes 1325352 which is an int or long, yet long long expected. Result: UB.

Strange use of llround()

Unclear why code uses limits computation

// double result = ((double)(llround(up)))/marge;
double result = (round(up))/marge;

Failure with 0

lltoa2(0) return an empty string. Use a do{} while loop instead.

Code lacks a test driver

All the myprintf() oblige a in person review of output rather than an automatic one. Test code should compare against the regular printf(). Idea: at the top of myprintf(), add code with vprintf(), then a personal review only need to sight compare pairs of output to see if they math.

Better: re-write code to mysprintf() and then compare string results with sprintf().

The key point is that to write a good myprintf(), you need a mini-tester jig (harness) so automated testing of millions of combination are possible.

Easy stuff

Enable all compiler warnings

Here are some of what came up for me - without even reading the code.
Save time, enable all warnings.

Why sign?

// warning: unused variable 'sign' [-Wunused-variable]
static int sign;

Undefined behavior to return address of local object. This is the biggest error. Until I corrected this, code would not run - and it was so easy to find.

//  warning: function returns address of local variable [-Wreturn-local-addr]
char d[MAX_LENGTH];
return &d[0];

Why ROUND_TO?

// warning: variable 'ROUND_TO' set but not used [-Wunused-but-set-variable]
int ROUND_TO = 0;

Small stuff

Undefined behavior

n = -n; is undefined behavior when n == LLONG_MAX in lltoa2().

Replicated use of lldiv()

Instead of 2 calls lldiv(), use one. Or better just code n/10 and n%10. Good compilers will see that and emit one piece of code to calculate both.

reverse(s) unnecessary

Just build string form the end toward the front.

Support functions should be static

Once code is segmented into myprintf() and the rest, functions like lltoa2() should be static to minimize names space pollution.

Odd code

d[0] = '\0'; strcat(d,abs); is same as simpler strcpy(d,abs);

Why "\0" vs ""?

// static char a[MAX_LENGTH] = "\0";
static char a[MAX_LENGTH] = "";

Use of int vs. size_t

size_t is neither too narrow nor too wide for array indexing and sizing. int , in general, may be insufficient. Code like int j = 0; while (j < strlen(a) ) a[j++] = '\0'; raises distracting warnings about mixing int/unsigned math. Instead:

size_t j = 0; while (j < strlen(a) ) {
  a[j++] = '\0';
}

Avoid calling strlen() in a loop.

A weak compiler will call strlen(a) over and over. Each call costs the length of the string.

// int j = 0;
//while (j < strlen(a) ) a[j++] = '\0';
// instead
size_t j = 0;
while (a[j]) {
  a[j++] = '\0';
}
// or simply
memset(a, 0, strlen(a));

On 2nd thought: while (j < strlen(a) ) a[j++] = '\0'; does nothing useful after static char a[MAX_LENGTH] = "\0"; as strlen(a) is 0.

Review printf() signature

printf() uses restrict. This may be advanced for OP at this time, but worthy of research.

int myprintf(char const * s,...)
int printf(const char * restrict format, ...);

---

Recommendation:

You are attempting one of the most challenging function re-writes - a bold task. I commend you on your bravery.

Try again with just integer types. After completing that, move on to floating-point.

Answer 2

Some small items, additional to the great review by Chux.

• You don't seem to use anything from <assert.h> or <float.h>, so no need to include those.
• The missing features are not documented anywhere, so we have to guess what subset of printf() is supported.
• Test cases are missing for ±∞ and not-a-number values with the %f specifier.

• Precision specifiers don't work properly with %s:

  myprintf("%.5s", "abcdefg");  // crashes
  

• Unnecessary obfuscation: *(s_p+1) is much more recognisable when written as s_p[1].

• The fallthrough from case '.' to default causes the second conversion specifier to be printed literally in this example (and could be even more serious if the second conversion were %%):

  myprintf("%.1f%f\n", 1.0, 0.0);