25
\$\begingroup\$

This code allows printing in C with a style similar to C++ ostream. By using _Generic() (see GP()) to form a string specifier, the compiler handles the selection of printf() specifiers.

No more mis-matched specifiers!

Requested review goals:

  1. Design concept: Ways to improve?

  2. Implementation: What weakness & strengths exist?


4 parts:

  1. main.c to show a sample use. GPrintf() must end in GP_eol or NULL

  2. Output

  3. GPrint.h Key macro GP()

  4. GPrint.c void GPrintf(const char *format, ...)

main.c

#include "GPrint.h"
#include <limits.h>

int main(void) {
  double q = 1000000.0 / 7;
  int i = 42;
  GPrintf("42 --> base 10:", GP(i), 
      GP_setbase(2), " 2:", GP(i), 
      GP_setbase(36), " 36:", GP(i), GP_eol);

  GPrintf("million/7 = ", GP(q), GP_eol);
  GPrintf("million/7 = ", GP_setprecision(3), GP(q), GP_eol);
  GPrintf("million/7 = ", GP_fixed, GP(q), GP_eol);
  GPrintf("million/7 = ", GP_fixed, GP_setprecision(3), GP(q), GP_eol);

  for (int y = 1; y < 9; y++) {
    int x = y * y;
    GPrintf(GP_setw(2), GP(y), ",", GP(x), ":", NULL);
    GPrintf(GP_repeat(x), GP((char)'*'), GP_eol);
  }
  GPrintf(GP_setw(5), GP(" "), ":", NULL);
  for (int x = 0; x < 70; x += 5)
    GPrintf("----+", NULL);
  GPrintf(GP_eol);

  return 0;
}

Output

42 --> base 10:42 2:101010 36:16
million/7 = 1.4285714285714287e+05
million/7 = 1.429e+05
million/7 = 142857.1428571428696159
million/7 = 142857.143
 1, 1:*
 2, 4:****
 3, 9:*********
 4,16:****************
 5,25:*************************
 6,36:************************************
 7,49:*************************************************
 8,64:****************************************************************
     :----+----+----+----+----+----+----+----+----+----+----+----+----+----+

GPrint.h

/*
 * GPrint.c
 *
 *  Created on: Dec 23, 2015
 *      Author: chux
 */

#ifndef GPRINT
#define GPRINT 1

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

typedef enum {
  GP_none,
  GP__Bool,
  GP_char,
  GP_signed_char,
  GP_unsigned_char,
  GP_short,
  GP_unsigned_short,
  GP_int,
  GP_unsigned,
  GP_long,
  GP_unsigned_long,
  GP_long_long,
  GP_unsigned_long_long,
  GP_float,
  GP_double,
  GP_long_double,
  GP__Complex_float,
  GP__Complex_double,
  GP__Complex_long_double,
  GP_void_star,
  GP_char_star,
  GP_const_char_star,
  GP_eol_,
  GP_FILE_,
  GP_fixed_,
  GP_flush_,
  GP_repeat_,
  GP_resetios_flags_,
  GP_scientific_,
  GP_set_base,
  GP_set_precision,
  GP_set_w,
  GP_setios_flags_,
  GP_string_,
  // future showpos, uppercase, fixed, scientific, floatfield, 
  // showpoint, boolalpha, showbase,
  GP_unknown_,
  GP_format_N
} GP_type;

// format constants
const char GP_format[GP_format_N + 1][1];

// Functions to perform type conversions by the calling code
inline FILE *GP_check_FILE(FILE *f) {
  return f;
}
inline int GP_check_int(int i) {
  return i;
}
inline size_t GP_check_size_t(size_t sz) {
  return sz;
}
inline char *GP_check_string(char *s) {
  return s;
}
#undef inline

// utility functions (as macros)
#define GP_eol               GP_format[GP_eol_]
#define GP_FILE(f)           GP_format[GP_FILE_], (GP_check_FILE(f))
#define GP_fixed             GP_format[GP_fixed_]
#define GP_repeat(n)         GP_format[GP_repeat_], (GP_check_int(n))
#define GP_scientific        GP_format[GP_scientific_]
#define GP_setbase(base)     GP_format[GP_set_base], (GP_check_int(base))
#define GP_setprecision(pre) GP_format[GP_set_precision], (GP_check_int(pre))
#define GP_setw(width)       GP_format[GP_set_w], (GP_check_int(width))
#define GP_string(s, sz)     GP_format[GP_string_], (GP_check_string(s)),

// Macro to form (string, value) pair
#define GP(X) _Generic((X), \
  _Bool: GP_format[GP__Bool], \
  char: GP_format[GP_char], \
  signed char: GP_format[GP_signed_char], \
  unsigned char: GP_format[GP_unsigned_char], \
  short: GP_format[GP_short], \
  unsigned short: GP_format[GP_unsigned_short], \
  int: GP_format[GP_int], \
  unsigned: GP_format[GP_unsigned], \
  long: GP_format[GP_long], \
  unsigned long: GP_format[GP_unsigned_long], \
  long long: GP_format[GP_long_long], \
  unsigned long long: GP_format[GP_unsigned_long_long], \
  float: GP_format[GP_float], \
  double: GP_format[GP_double], \
  long double: GP_format[GP_long_double], \
  _Complex float: GP_format[GP__Complex_float], \
  _Complex double: GP_format[GP__Complex_double], \
  _Complex long double: GP_format[GP__Complex_long_double], \
  void *: GP_format[GP_void_star], \
  char *: GP_format[GP_char_star], \
  const char *: GP_format[GP_const_char_star] \
 ),(X)

// THE function
void GPrintf(const char *format, ...);

#endif

GPrint.c

/*
 * GPrint.c
 *
 *  Created on: Dec 23, 2015
 *      Author: chux
 */

#include "GPrint.h"
#include <assert.h>
#include <float.h>
#include <stdarg.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>

struct GP_style {
  char buf[sizeof(intmax_t) + 1 + 1];
  // char sign[2][2];
  int prec[3];
  int width;
  int repeat_m1;
  unsigned base;
  _Bool fixed;
};

static const char base36[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";

// Convert signed value to string depending on base
static char *utoa_base(struct GP_style *style, uintmax_t u) {
  char *p = &style->buf[sizeof *(style->buf) - 1];
  *p = '\0';
  uintmax_t x = u;
  do {
    *(--p) = base36[x % style->base];
    x /= style->base;
  } while (x);
  return p;
}

// Convert signed value to string depending on base
static char *itoa_base(struct GP_style *style, intmax_t i) {
  char *p = &style->buf[sizeof *(style->buf) - 1];
  *p = '\0';
  intmax_t x = i < 0 ? i : -i;
  do {
    *(--p) = base36[-(x % style->base)];
    x /= style->base;
  } while (x);
  if (i < 0) *(--p) = '-';
  return p;
}

// Return index of GP_format[]
static int GP_get_type(const char *format) {
  if (format >= GP_format[0] && format < GP_format[GP_format_N]) {
    return (int) (format - GP_format[0]);
  }
  return GP_format_N;
}

// External ............................................

const char GP_format[GP_format_N + 1][1] = { { 0 } };

void GPrintf(const char *format, ...) {
  va_list ap;
  va_start(ap, format);
  FILE *ostream = stdout;
  struct GP_style style = { 0 };

  style.prec[0] = FLT_DECIMAL_DIG - 1;
  style.prec[1] = DBL_DECIMAL_DIG - 1;
  style.prec[2] = LDBL_DECIMAL_DIG - 1;

  while (format) {
    switch (GP_get_type(format)) {
      case GP__Bool:
        fprintf(ostream, "%*d", style.width, (_Bool) va_arg(ap, int));
        break;
      case GP_char: {
        char c = (char) va_arg(ap, int);
        for (int i = -1; i < style.repeat_m1; i++) {
          fprintf(ostream, "%*c", style.width, c);
        }
        break;
      }
      case GP_int: {
        int i = va_arg(ap, int);
        if (style.base == 0 || style.base == 10) {
          fprintf(ostream, "%*d", style.width, i);
        } else {
          fprintf(ostream, "%*s", style.width, itoa_base(&style, i));
        }
        break;
      }
      case GP_unsigned: {
        unsigned u = va_arg(ap, unsigned);
        if (style.base == 0 || style.base == 10) {
          fprintf(ostream, "%*u", style.width, u);
        } else {
          fprintf(ostream, "%*s", style.width, utoa_base(&style, u));
        }
        break;
      }
      case GP_float:
        // (float) cast used to enforce float precision.
        fprintf(ostream, style.fixed ? "%*.*f" : "%*.*e", style.width,
                style.prec[0], (float) va_arg(ap, double));
        break;
      case GP_double:
        fprintf(ostream, style.fixed ? "%*.*f" : "%*.*e", style.width,
                style.prec[1], va_arg(ap, double));
        break;
      case GP_long_double:
        fprintf(ostream, style.fixed ? "%*.*Lf" : "%*.*Le", style.width,
                style.prec[2], va_arg(ap, long double));
        break;
      case GP_void_star:
        fprintf(ostream, "%*p", style.width, va_arg(ap, void *));
        break;
      case GP_char_star:
        fprintf(ostream, "%*s", style.width, va_arg(ap, char *));
        break;
      case GP_const_char_star:
        fprintf(ostream, "%*s", style.width, va_arg(ap, char *));
        break;
      case GP_set_w:
        style.width = va_arg(ap, int);
        break;
      case GP_set_precision:
        style.prec[2] = style.prec[1] = style.prec[0] = va_arg(ap, int);
                        assert(style.prec[0] >= 0);
        break;
      case GP_set_base: {
        int i = va_arg(ap, int);
        assert(i >= 2 && i <= 36);
        style.base = (unsigned) i;
        break;
      }
      case GP_fixed_:
        style.fixed = true;
        break;
      case GP_scientific_:
        style.fixed = false;
        break;
      case GP_FILE_:
        ostream = va_arg(ap, FILE *);
        break;
      case GP_repeat_:
        style.repeat_m1 = va_arg(ap, int) - 1;
        break;
      case GP_eol_:
        fputc('\n', ostream);
        goto early_end;
      case GP_flush_:
        fflush(ostream);
        break;
      case GP_unknown_:
        assert(0);
        // This case is future code expansion
        fputs("Unknown Type", ostream);
        fflush(stdout);
        goto early_end;
      default:
        if (format == NULL) goto early_end;
        fputs(format, ostream);
        break;
    }
  format = va_arg(ap, const char *);
  }
  early_end: va_end(ap);
}

Any printf() replacement can certainly involves many lines of code. The above is only a portion, yet functional, part a full implementation. E.g. Printing to char * not shown. _Complex types not yet handled.

This approach is like-wise applicable to scanf().

I have not seen this usage of _Generic() before. I came up with it over the past month.

Ref on _generic() extensibility

\$\endgroup\$
3
  • 1
    \$\begingroup\$ You are aware, that most modern C compilers are able to check type inconsistencies statically and apply appropriate warnings already? \$\endgroup\$ Commented Dec 27, 2015 at 0:16
  • \$\begingroup\$ @πάντα ῥεῖ Yes, many C compilers do as you commented. With add-on tools (lint), such detection available for decades. I see advantages here such as less initial coding errors, less maintenance as a variable's type changes, ability to print float, double in different %a precision (that code not implemented but easy to add). Did not consider detailing known pros and cons of the approach. Hmmm. \$\endgroup\$
    – chux
    Commented Dec 27, 2015 at 2:16
  • \$\begingroup\$ Although @Mat answer's brought up an important issue, I was hoping for a wider review. \$\endgroup\$
    – chux
    Commented Mar 21, 2016 at 17:39

3 Answers 3

7
+100
\$\begingroup\$

I see some things that may help you improve your code.

Cleanly separate interface from implementation

The header file contains a variable and four inline functions. The variable should be declared extern in the header. For the functions, because they're declared as inline they're subject to somewhat the reverse of the usual rule. That is, the header file contains the function implementations, as you have done it, but the .c file needs to declare them as extern or the files can't be compiled and linked separately. See this explanation which correctly notes that the code must not depend on whether the called function is inline or external. With that said, the next point may make it moot.

Eliminate unneeded functions

Because each of the four functions simply return what they're passed, they don't seem to be required at all. For example, this macro

#define GP_setbase(base)     GP_format[GP_set_base], (GP_check_int(base))

works just as well in its simplified form:

#define GP_setbase(base)     GP_format[GP_set_base], (base)

Perhaps you were concerned about macro evaluation and expansion? I don't see why these were introduced except perhaps as a reminder to the reader about the expected types.

Initialize the entire structure

I'd recommend changing style so that it initializes all fields to reasonable defaults. In particular, you could add the defaults to the struct declaration like this:

struct GP_style {
  char buf[sizeof(intmax_t) + 1 + 1];
  // char sign[2][2];
  int prec[3];
  int width;
  int repeat_m1;
  unsigned base;
  _Bool fixed;
} default_GP_style = 
{
  {0}, // buf
  { FLT_DECIMAL_DIG - 1, DBL_DECIMAL_DIG - 1, LDBL_DECIMAL_DIG - 1}, // prec
  0,  // width
  0,  // repeat_m1
  10, // base
  false  // fixed
};

Then instead of the current lines to initialize various parts of style, you could have just this:

struct GP_style style = default_GP_style;

Allow for output file flexibility

You probably already have it in mind, but I would probably prefer to have a GfPrintf which would allow me to write to a FILE * other than stdout.

Don't forget about const pointers

There is a provision for printing a void * type but not at const void * type. This is easy to fix and requires only another few lines in the code, but it does also demonstrate the problem that adding support for a particular type requires changes in three different places in two different files. It would be nice to consolidate that.

Consider the user

With C++, if I want to write a custom stream inserter, it's fairly simple to do. It would be nice to be able to allow the user of this to be able to allow custom printing routines for user-defined structures.

struct dummy {
    int lucky[3];
};

const char* dummyPrint(struct dummy d) {
    static char buffer[200];
    sprintf(buffer, "%d-%d-%d", d.lucky[0], d.lucky[1], d.lucky[2]);
    return buffer;
}

One way to approach that might be using a macro like this:

#define GP_Custom(X) _Generic((X), \
    struct dummy: dummyPrint(X) \
)

Because it returns a string pointer, we can use it without modification to GPrint:

struct dummy d = {{ 7, 5, 1 }};
GPrintf("My lucky numbers are: ", GP_Custom(d), GP_eol); 

However, it's not so nice because of the requirement to convert via a buffer. Also, _Generic requires that all results must be of the same type, so with more than one kind of structure, one would need something even uglier like this:

#define GP_Custom(X) _Generic((X), \
    struct dummy: dummyPrint((void *)&X) , \
    struct macho: machoPrint((void *)&X) \
)

Perhaps you can find a better way.

\$\endgroup\$
7
\$\begingroup\$

Technicality

Pedantically speaking, the GP_get_type function has undefined behavior if the pointer it gets isn't a pointer into (or one past the end of) GP_format (see §6.5.8/5 Relational operators).

Simple enough to fix: wrap all the arguments, including the first one and any string literal with GP.

You could enforce that with an assertion in GP_get_type (instead of the if, get rid of the default), but... you can't escape undefined behavior in that assertion :-)

I can't think of real-world circumstances on ordinary compilers/environments where this particular usage would be problematic though.

\$\endgroup\$
1
  • \$\begingroup\$ Solution to this UB and follow up here. \$\endgroup\$
    – chux
    Commented Jan 25, 2020 at 20:36
3
\$\begingroup\$

A self answer per: If you want to show everyone how you improved your code, but don't want to ask another question, then post an answer to your own question.

@Mat identified undefined behavior of using relational operators with unrelated pointers:

if (format >= GP_format[0] && format < GP_format[GP_format_N]) 

A fix:

  1. Make each GP_format[] string start with its index.
  2. Use the first character of the string as an index to test against GP_format[index].

Now code can detect if a string is a non-GP_format[] one or return the index of the GP_format[].

// GPrint.h
// const char GP_format[GP_format_N + 1][1];
const char GP_format[GP_format_N + 1][2];

// GPrint.c
// const char GP_format[GP_format_N + 1][1] = { { 0 } };
_Static_assert(GP_format_N == 043);
// Unfortunately have not come up with an elegant way to initialize
const char GP_format[GP_format_N + 1][2] = { 
   "\0", "\1", "\2", "\3", "\4", "\5", "\6", "\7", 
   "\10", "\11", "\12", "\13", "\14", "\15", "\16", "\17", 
   "\20", "\21", "\22", "\23", "\24", "\25", "\26", "\27", 
   "\30", "\31", "\32", "\33", "\34", "\35", "\36", "\37", 
   "\40", "\41", "\42", "\43",
};
// Could alternatively use hex above as in \0x..

// GPrint.c
// Return index of GP_format[]
static int GP_get_type(const char *format) {

  //if (format >= GP_format[0] && format < GP_format[GP_format_N]) {
  //  return (int) (format - GP_format[0]);
  //}
  char index = *format;
  if (index >= 0 && index < GP_format_N && format == GP_format[index]) {
      return index;
  }

  return GP_format_N;
}

This approach may expand to go beyond CHAR_MAX. Also only one GP_format string is really needed "\0\1\2\3....", but to show that much more code needs to change.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.