3

Woking with a simple function to print a similated 2D matrix, I simply want to be able to pass an array of pointers to type to the function as void**, along with the needed dimensions m x n, the sizeof type, a format string for printf, and a simple flag to distinguish floating-point from integer. The problem I am running into is handling scope for dereferencing the array of pointers so that each element can be properly printed as the original type. Below, the basic scheme is:

void mtrx_prnv (size_t m, size_t n, void **matrix,
                size_t sz, char *fmt, char fp)
{
    if (fp) {    // floating point
        if (sz == 4) {
            float **mtrx = (float **)matrix;
            ...
        }
        else if (sz == 8) {
            double **mtrx = (double **)matrix;
            ...
        }
    }
    else {
        if (sz == 1) {
            char **mtrx = (char **)matrix;
            ...
        }
        else if (sz == 2) {
            ...
        }
        ...
    }
}

The approach works fine, but the problem is that after testing for the floating-point flag 'fp' and sizeof type 'sz', any pointer creation to use for dereferencing is restricted to the scope of the test block and ends up requiring the basic verbatim duplication of the code needed to process the array of pointers in each block. The code ends up being longer than creating a lot of little functions for each type. e.g.:

void mtrx_prn_float (size_t m, size_t n, float **matrix) {}

void mtrx_prn_int (size_t m, size_t n, int **matrix) {}

Is there a better or standard way to pass an array of pointers to type as void**, the sizeof type, and whatever other flags are needed to properly dereference to type without so much duplication of code? If this is the way it has to be, that's fine, but I want to make sure I'm not missing a simple trick. So far my searches haven't produced anything useful (the overwhelming body of information returned by search is about how to get a single type back, not separate all types). How to write C function accepting (one) argument of any type wasn't on point here.

The full function (without the additional logic for int/unsigned separation) is below.

/* printf array of pointers to type as 2D matrix of
   size 'm x n' with sizeof type 'sz', format string
  'fmt' and floating point flag 'fp' (0 - int).
*/
void mtrx_prnv (size_t m, size_t n, void **matrix,
                size_t sz, char *fmt, char fp)
{
    register size_t i, j;

    if (fp) {    /* floating point */

        if (sz == 4) {
            float **mtrx = (float **)matrix;
            for (i = 0; i < m; i++) {
                char *pad = " [ ";
                for (j = 0; j < n; j++) {
                    printf (fmt, pad, mtrx[i][j]);
                    pad = ", ";
                }
                printf ("%s", " ]\n");
            }
        }
        else if (sz == 8) {
            double **mtrx = (double **)matrix;
            for (i = 0; i < m; i++) {
                char *pad = " [ ";
                for (j = 0; j < n; j++) {
                    printf (fmt, pad, mtrx[i][j]);
                    pad = ", ";
                }
                printf ("%s", " ]\n");
            }
        }
        else
            goto err;
    }
    else {      /* integer (no unsigned yet) */

        if (sz == 1) {
            char **mtrx = (char **)matrix;
            for (i = 0; i < m; i++) {
                char *pad = " [ ";
                for (j = 0; j < n; j++) {
                    printf (fmt, pad, mtrx[i][j]);
                    pad = ", ";
                }
                printf ("%s", " ]\n");
            }
        }
        else if (sz == 2) {
            short **mtrx = (short **)matrix;
            for (i = 0; i < m; i++) {
                char *pad = " [ ";
                for (j = 0; j < n; j++) {
                    printf (fmt, pad, mtrx[i][j]);
                    pad = ", ";
                }
                printf ("%s", " ]\n");
            }
        }
        else if (sz == 4) {
            int **mtrx = (int **)matrix;
            for (i = 0; i < m; i++) {
                char *pad = " [ ";
                for (j = 0; j < n; j++) {
                    printf (fmt, pad, mtrx[i][j]);
                    pad = ", ";
                }
                printf ("%s", " ]\n");
            }
        }
        else if (sz == 8) {
            long **mtrx = (long **)matrix;
            for (i = 0; i < m; i++) {
                char *pad = " [ ";
                for (j = 0; j < n; j++) {
                    printf (fmt, pad, mtrx[i][j]);
                    pad = ", ";
                }
                printf ("%s", " ]\n");
            }
        }
        else
            goto err;
    }

    return;

err:;

    fprintf (stderr, "%s() error: invalid size for fp_flag '%zu'.\n",
                __func__, sz);
}

As suggested in the comments, the declaration/definition is updated to:

void mtrx_prnv (size_t m, size_t n, void *matrix,
                size_t sz, char *fmt, char fp);

Macro Provided Solution

There were several excellent solutions suggested, primarily eliminate the verbatim duplication by creating a macro for the redundant text, and secondly the use of callbacks to print the value of the elements. The macro provided a near drop in solution:

#define PRINT_MATRIX(type) do { \
    type **mtrx = (type **)matrix;  \
    for (i = 0; i < m; i++) { \
        char *pad = " [ "; \
        for (j = 0; j < n; j++) { \
            printf (fmt, pad, mtrx[i][j]); \
            pad = ", "; \
        } \
        printf ("%s", " ]\n"); \
    } \
} while (0)

...

void mtrx_prnvm (size_t m, size_t n, void *matrix,
                size_t sz, char *fmt, char fp)
{
    register size_t i, j;

    if (fp) {    /* floating point */

        if (sz == 4) {
            PRINT_MATRIX(float);
        }
        else if (sz == 8) {
            PRINT_MATRIX(double);
        }
        else
            goto err;
    }
    else {      /* integer (no unsigned yet) */

        if (sz == 1) {
            PRINT_MATRIX(char);
        }
        else if (sz == 2) {
            PRINT_MATRIX(short);
        }
        else if (sz == 4) {
            PRINT_MATRIX(int);
        }
        else if (sz == 8) {
            PRINT_MATRIX(long);
        }
        else
            goto err;
    }

    return;

err:;

    fprintf (stderr, "%s() error: invalid size for fp_flag '%zu'.\n",
                __func__, sz);
}
Community
  • 1
  • 1
David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
  • 3
    The type of the `matrix` argument should really be just `void *` rather than `void **`, if you want this code to be properly portable. Changing it shouldn't have any noticeable effect, so you may as well, really. – davmac Oct 20 '15 at 21:10
  • As @davmac said. Any object pointer value can be safely round-tripped through a conversion to `void *`, but that does not necessarily mean that `void *` has the same *representation* as any particular other pointer type. As a result, although casting `void *` to `other_type **` is safe(-ish), casting `void **` to `other_type **` is *not* safe. – John Bollinger Oct 20 '15 at 21:15
  • That makes sense. I can always get my `other_type **` back if matrix is `void *` – David C. Rankin Oct 20 '15 at 21:18
  • Have you considered defining a macro for that duplicated code? – John Bollinger Oct 20 '15 at 21:22
  • No, I hadn't -- until now. That would be quite helpful. Thanks. – David C. Rankin Oct 20 '15 at 21:29

5 Answers5

0

This smells a bit like an XY problem and the code duplication isn't too fresh either. So I recommend using a different strategy altogether. Use callbacks instead.

The mtrx_prnv function iterates over all indices, but uses a provided callback to print the actual values. The callback receives the indices and the matrix as a void * (just like mtrx_prnv itself), but converts it to the appropriate type and prints the value.

void mtrx_prnv(size_t n, size_t m, void * mtrx, void (*prncb)(size_t i, size_t j, void * mtrx)){
    size_t i, j;
    for (i = 0; i < m; i++) {
        printf(" [ ");
        for (j = 0; j < n; j++) {
            prncb(i, j, mtrx); /* Use callback to print value */
            printf(", "); /* fix the trailing comma yourself ;) */
        }
        printf (" ]\n");
    }
}

Example callbacks for ints and floats:

void prnint(size_t i, size_t j, void * vmat){
    int ** mtrx = vmat;
    printf("%d", mtrx[i][j]);
}

void prnflt(size_t i, size_t j, void * vmat){
    float ** mtrx = vmat;
    printf("%f", mtrx[i][j]);
}

It is up to the caller to provide the correct callback for the matrix type, but that means that mtrx_prnv does not need to concern itself with the actual types and can even deal with more exotic content, you only need to provide the proper callback.

For readability you could typedef the function pointer:

typedef void (prncb_f)(size_t i, size_t j, void * mtrx);

And change mtrx_prnv:

void mtrx_prnv(size_tn, size_t m, void * mtrx, prncb_f * cb)

ADDIT

If you need to stay with the signature of mtrx_prnv you could do just selector logic in there and have it pass the proper callbacks:

(Rename mtrx_prnv from before to mtrx_prnv_helper)

void mtrx_prnv (size_t m, size_t n, void *matrix,
            size_t sz, char *fmt, char fp){
    if(fp){
        if(sz == sizeof(float)){
            mtrx_prnv_helper(m, n, matrix, prnflt);
        }else if(sz == sizeof(double)){
            ...
        }
    }else{
        if(sz == sizeof(int)){
            mtrx_prnv_helper(m, n, matrix, prnint);
        }else{
            ...
        }
    }
}

But you'll lose the potential for other data types in the matrix.

There is, of course, some function call overhead for calling the callback for every cell in the matrix, but in this case it's most likely overshadowed by the I/O-overhead of printing.

Kninnug
  • 7,992
  • 1
  • 30
  • 42
  • 1
    I like that. It's clean. The few caveats I see are that function calls are comparatively expensive, and that the compiler cannot optimize indirect function calls. In the unlikely event that performance of this output function were important, the reliance on callbacks could therefore introduce unacceptable overhead. – John Bollinger Oct 20 '15 at 21:35
  • 1
    @JohnBollinger a valid point, though in this specific case the I/O will probably be a much bigger (tighter?) bottleneck. – Kninnug Oct 20 '15 at 21:41
  • It took a minute to digest, but I agree. I had considered a callback, but got wrapped around the axle trying to think of a way to have it do all the separation rather than using multiple callbacks. This may also lead to a wrapper for the whole `mtrx_prnv` whose purpose is the take the size and fp_flag and call `mtrx_prn` with the proper callback. Some what of a selector function that could take a pointer to an array of function pointers holding the callbacks, sift through the size/fp_flag logic and pass the proper callback to the print function. – David C. Rankin Oct 20 '15 at 21:44
0

Considering that you are repeating the bulk of each block verbatim, one obvious solution would be to define and use a macro representing that code (as opposed to a macro for generating per-type functions, as another answer suggests). That would limit physical code duplication, and would not require you to change the structure of your program:

#define PRINT_MATRIX(type) do { \
    (type) **mtrx = ((type) **)matrix;
    for (i = 0; i < m; i++) { \
        char *pad = " [ "; \
        for (j = 0; j < n; j++) { \
            printf (fmt, pad, mtrx[i][j]); \
            pad = ", "; \
        } \
        printf ("%s", " ]\n"); \
    } \
} while (0)

void mtrx_prnv (size_t m, size_t n, void *matrix,
                size_t sz, char *fmt, char fp)
{
    if (fp) {    // floating point
        if (sz == 4) {
            PRINT_MATRIX(float);
        }
        else if (sz == 8) {
            PRINT_MATRIX(double);
        }
    }
    else {
        if (sz == 1) {
            PRINT_MATRIX(char);
        }
        else if (sz == 2) {
            PRINT_MATRIX(short);
        }
        ...
    }
}
John Bollinger
  • 160,171
  • 8
  • 81
  • 157
  • John, thanks. Your macro suggestion led to a full set of macros for printing, add, multiply, transpose, transpose_in_place, etc.. Worked like a champ and actually resulted in a general template format (similar to the original scheme) for any type matrix operation that simply handles the conditional size and floating-point flag and calls a macro to handle the individual type. Code duplication -- gone. – David C. Rankin Oct 21 '15 at 06:13
0

Although the question is already answered, I wanted to show an alternate. This too uses a macro to simplify the code, but it is used within a row loop (since printing anything, even a single number, takes much longer than the conditionals). Also, the function parameters differ a bit.

#include <stdio.h>

typedef enum {
    UNSIGNED_CHAR,
    SIGNED_CHAR,
    UNSIGNED_SHORT,
    SIGNED_SHORT,
    UNSIGNED_INT,
    SIGNED_INT,
    UNSIGNED_LONG,
    SIGNED_LONG,
    FLOAT,
    DOUBLE
} element_type;

void fmatrix(FILE *const        out,
             void *const        origin,
             const int          rows,
             const int          cols,
             const long         rowstride,
             const long         colstride,
             const element_type type,
             const int          decimals)
{
    int row, col;

    for (row = 0; row < rows; row++) {

        switch (type) {
#define PRINTROW(type, argtype, spec) \
            do { \
                const unsigned char *const data = (unsigned char *)origin + row * rowstride; \
                for (col = 0; col < cols - 1; col++) \
                    fprintf(out, spec ", ", decimals, (argtype)( *(const type *)(data + col * colstride) )); \
                fprintf(out, spec "\n", decimals, (argtype)( *(const type *)(data + (cols - 1) * colstride) )); \
            } while (0)

        case UNSIGNED_CHAR:  PRINTROW(unsigned char,  unsigned int,  "%*u");  break;
        case UNSIGNED_SHORT: PRINTROW(unsigned short, unsigned int,  "%*u");  break;
        case UNSIGNED_INT:   PRINTROW(unsigned int,   unsigned int,  "%*u");  break;
        case SIGNED_CHAR:    PRINTROW(signed char,    int,           "%*d");  break;
        case SIGNED_SHORT:   PRINTROW(signed short,   int,           "%*d");  break;
        case SIGNED_INT:     PRINTROW(int,            int,           "%*d");  break;
        case UNSIGNED_LONG:  PRINTROW(unsigned long,  unsigned long, "%*lu"); break;
        case SIGNED_LONG:    PRINTROW(long,           long,          "%*ld"); break;
        case FLOAT:          PRINTROW(float,          double,        "%.*f");  break;
        case DOUBLE:         PRINTROW(double,         double,        "%.*f");  break;

#undef PRINTROW
        }
    }
}

origin points to the element at the upper left corner of the matrix. The matrix has rows rows and cols columns.

colstride and rowstride define the number of bytes between successive members in the matrix. For standard C arrays, rowstride = cols * colstride, and colstride = sizeof (type), for matrix elements of type type.

If you want output to be transposed, just swap rows and cols, and rowstride and colstride.

eltype specifies the element type, and decimals the number of decimal digits for FLOAT and DOUBLE types (default is 6), or the minimum width for integer types. Use -1 if you don't want to specify (as negative precision is treated as if it was omitted).

As a practical example,

float m[2][3] = { { 1, 2, 3 }, { 4, 5, 6 }};

fmatrix(stdout, m, 2, 3, 3 * sizeof m[0][0], sizeof m[0][0], FLOAT, 2);

outputs

1.00, 2.00, 3.00
4.00, 5.00, 6.00

and

fmatrix(stdout, m, 3, 2, sizeof m[0][0], 3 * sizeof m[0][0], FLOAT, 1);

outputs

1.0, 4.0
2.0, 5.0
3.0, 6.0

The PRINTROW macro I used is a bit ugly and complicated (it could be written better), but this should be fully portable code, carefully written to explicitly do the correct casts/promotions, so hopefully it is clear enough to understand.

Nominal Animal
  • 38,216
  • 5
  • 59
  • 86
  • Thank you. I have a similar routine for sequential 1D arrays in memory that allows representation of all valid combinations of row x cols for each valid stride (rowstrde) given the total number of elements. What I hadn't done there is incorporate a colstride in order to make it generic to type. What prompted this question was an input routine to flexibly read a given file containing values with any separator, the opened the rabbit-hole of flexibly handling any storage type of input. Thanks for your suggestions. – David C. Rankin Oct 21 '15 at 06:08
  • @DavidC.Rankin: I've found that the overhead of having a multiplier for both the row and the column number is balanced out by the flexibility it provides. I use a separate reference-counted "owner" structure to hold the actual data, with matrices having a pointer to both the owner, as well as the pointer to the initial element. This allows real-time views (transposes, submatrices, row vectors, column vectors, even diagonal vectors, mirrored vectors, and so on) with really easy dynamic memory management. Let me know if you are interested in the details. – Nominal Animal Oct 22 '15 at 02:55
-1

The easiest solution is probably to define a macro which when called defines a function for a specified type (but I'd personally still be inclined to just write separate functions for each type; macros always felt like a cheap hack to me). Something like:

#define define_matrix_print(type,fmt) \
    void mtrx_prn_##type(size_t m, size_t n, type **matrix) { \
       for (int i = 0; i < m; i++) { \
           char *pad = " [ "; \
           for(int j = 0; j < n; j++) { \
               printf(fmt, pad, matrix[i][j]); \
               pad = ", "; \
           } \
           printf(" ]\n"); \
       } \
     }

define_matrix_print(char, "%c")
/* above defines 'mtrx_prn_char(size_t, size_t, char **)' */
define_matrix_print(int, "%d")
/*               'mtrx_prn_int(size_t, size_t, int **)'   */
/*    and so on.                                           */

Another possibility is to use a function pointer, something like:

void (*print_func)(void *mtrx, int i, int j, char *fmt, char *pad);
switch (sz) {
    case 1:
        print_func = print_char_mtrx;
        break;
    case 2:
        print_func = print_short_mtrx;
        break;
    // others omitted for brevity
}
for (i = 0; i < m; i++) {
    char *pad = " [ ";
    for (j = 0; j < n; j++) {
        print_func(matrix, i, j, fmt, pad);
        pad = ", ";
    }
    printf ("%s", " ]\n");
}

Then you declare:

void print_char_mtrx(void *mtrx, int i, int j, char *fmt, char *pad)
{
    char ** mtrx_c = mtrx;
    printf(fmt, pad, mtrx_c[i][j]);
}

... and so on for the other types. You're only saving a little bit of typing/duplication this way, however. You might of course opt to combine the two techniques and define a macro that you can use to define various functions to print an element from a matrix of various types.

davmac
  • 20,150
  • 1
  • 40
  • 68
  • 1
    Yes, this is where templates make life much easier, but fortunately/unfortunately we are C only. The macro idea is probably going to be the best option. That would limit the duplication and basically reduce the function to the logic required to determine the type. – David C. Rankin Oct 20 '15 at 21:33
  • @DavidC.Rankin indeed, if you're stuck with C, macros are probably the way to go here **shudder** :) – davmac Oct 20 '15 at 21:39
-2

Following implementation avoids the duplication by using a buffer variable.

As long as it is the last argument to printf, any type length can be supported in the same way. t must be of the largest type.

void mtrx_prnv(size_t m, size_t n, void *matrix,
               size_t sz, char *fmt, char fp)
{
    size_t i, j;

    if (sz != 4 && sz != 8 && (fp || (sz != 1 && sz != 2))) {
        fprintf(stderr, "%s() error: invalid size for fp_flag '%zu'.\n",
            __func__, sz);
        return;
    }

    for (i = 0; i < m; i++) {
        char *p = ((char **)matrix)[i];
        double t;

        memcpy(&t, p, sz);
        printf(fmt, " [ ", t);
        for (j = sz; j < n * sz; j += sz) {
            memcpy(&t, p + j, sz);
            printf(fmt, ", ", t);
        }
        printf(" ]\n");
    }
}

The use of memcpy instead of a direct cast is to avoid corner-cases like the larger value being in the border between two pages, which might cause a segfault. It could be optimized to be used conditionally and just in the last element of each page.

Ismael Luceno
  • 2,055
  • 15
  • 26
  • 1
    The problem is that you can't dereference a `void *` so you can't pass a specific value/cell in the matrix to `printf`. `matrix[i][j]` is illegal and `((char**)matrix)[i][j]` works only when the matrix is made up of `char`s. – Kninnug Oct 20 '15 at 21:45
  • 1
    No. `printf()` being variadic does not make this do the right thing: `((char **)matrix)[i][j]`. That expression is evaluated before the call, and then default argument promotion is applied. The result is correct only when the actual underlying type is `char`. Moreover, this does not address the OP's main problem, which was to avoid all the code duplication. – John Bollinger Oct 20 '15 at 21:45
  • I sent it accidentally before finishing. – Ismael Luceno Oct 20 '15 at 21:56
  • 1
    Your revision does not rescue this answer. Instead, it seems to reflect a serious misconception about how variadic functions work. I'm having trouble guessing exactly what you think should happen, but do remember that C has only pass-by-value, including for variadic functions. Substituting a pointer to the intended argument for the argument itself is never the right thing to do. – John Bollinger Oct 21 '15 at 12:57