9
\$\begingroup\$

I've been thinking about a way to make it easier to safely use math operations with C's basic datatypes (e.g. using the CERT C coding standard). So far, I've come up with something like this:

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

#define safe_add(x, y) _Generic((x + y),                    \
                            unsigned int: safe_add_uint,    \
                           unsigned long: safe_add_ulong    \
                       )(x, y, __FILE__, __LINE__)

unsigned long
safe_add_ulong(unsigned long x, unsigned long y,
               const char* filename, int line_num)
{
    if (x < ULONG_MAX - y)
        return x + y;
    else {
        fprintf(stderr,
                "Integer wrap-around occurred, File: %s, Line: %d\n",
                filename, line_num);
        exit(EXIT_FAILURE);
    }
}

unsigned int
safe_add_uint(unsigned int x, unsigned int y,
              const char* filename, int line_num)
{
    if (x < UINT_MAX - y)
        return x + y;
    else {
        fprintf(stderr,
                "Integer wrap-around occurred, File: %s, Line: %d\n",
                filename, line_num);
        exit(EXIT_FAILURE);
    }
}

int
main()
{        
    /*  
     *  usual arithmetic conversions results in this calling
     *  the uint version of safe_add...
     */
    safe_add(2000000000, 3000000000u);

    printf("We shouldn't get here...(unless unsigned int uses more than 32 bits)\n");
}

Output of the above would be something similar to:

Integer wrap-around occurred, File: /.../main.c, Line: 41

and the program exits with a failure code (assuming the OS supports that).

Obviously, additional operations and types need to be added as well as handling signed arithmetic, but are there any problems with this overall method that I may be missing?

Is there perhaps an easier way to write this that I've overlooked?

\$\endgroup\$
8
\$\begingroup\$

Very impressive! A few notes:

  • I really like how you are using C11 stuff.

    #define safe_add(x, y) _Generic((x + y),                    \
                                unsigned int: safe_add_uint,    \
                               unsigned long: safe_add_ulong    \
                           )(x, y, __FILE__, __LINE__)
    

    It's good that you are using the more modern items the C standards have given us to use. My only nitpick with this #define is that your aligning is a bit off.

    #define safe_add(x, y) _Generic((x + y),                        \
                                    unsigned int: safe_add_uint,    \
                                    unsigned long: safe_add_ulong)  \
                                    (x, y, __FILE__, __LINE__)
    
  • @Morwenn touches on your error handling a bit, but I want to go deeper. For now, I want to talk about the style of error reporting I've come to adopt for such APIs. Options for error reporting from C functions include the following:

    • Return an error code from functions that can fail.

    • Provide a function like Windows GetLastError() or OpenGL's glGetError() to retrieve the most recently occurring error code.

    • Provide a global (well, hopefully, thread-local) variable containing the most recent error, like POSIX errno.

    • Provide a function to return more information about an error, possibly in conjunction with one of the above approaches, like POSIX's strerror function.

    • Allow the client to register a callback when an error occurs, like GLFW's glfwSetErrorCallback.

    • Use an OS-specific mechanism like structured exception handling.

    • Write errors out to a logfile, stderr, or somewhere else.

    • Just assert() or somehow else terminate the program when an error occurs.

    There are loads of tradeoffs among these options, and it's a matter of opinion as to which ones are better than others. Like all matters of style, unless a style is just plain unsuitable (like the last two options above might be for many uses), it's probably most important to apply the style you choose consistently.

    Let me illustrate the style of error handling I use for C APIs with the following re-written function of yours:

    unsigned int safe_add_uint(unsigned int x, unsigned int y,
          const char* filename, int line_num, char* error)
    

    Where we call it as:

    char* error = /* allocate memory */;
    safe_add_uint(x, y, __FILE__, __LINE__, &error)`
    

    Now your function can set the value of error within the functions, and then print the output in something such as an atexit() function.

    You might think that this could be a bit cumbersome for an error handling system in C, but here's the catch: this C error handling style that plays nice with C++ exceptions too. This can be nice when implementing an hourglass API.

  • You should probably be using exact-width integer types.

  • Always declare what parameters your function takes in, even if nothing.

    int main(void)
    

    You might wonder why we have to do this. Imagine we have the function foo() declared as such:

    int foo()
    

    In C, this is known as an identifier list and means that it "can take any number of parameters of unknown types". We can actually pass values to the function even though we don't mean to or intend to. If the caller calls the function giving it some argument, the behavior is undefined. The stack could become corrupted for example, because the called function expects a different layout when it gains control.

    Using identifier lists in function parameters is depreciated. It is much better to do something like:

    int foo(void)
    

    In C, this is known as a parameter type list and defines that the function takes zero arguments (and also communicates that when reading it) - like with all cases where the function is declared using a parameter type list, which is called a prototype. If the caller calls the function and gives it some argument, that is an error and the compiler spits out an appropriate error.

    The second way of declaring a function has plenty of benefits. One of course is that amount and types of parameters are checked. Another difference is that because the compiler knows the parameter types, it can apply implicit conversions of the arguments to the type of the parameters. If no parameter type list is present, that can't be done, and arguments are converted to promoted types (that is called the default argument promotion). char will become int, for example, while float will become double.

\$\endgroup\$
  • \$\begingroup\$ Wow, a very thoughtful response! I really like the summation of error handling techniques. The one you proffer reminds me of what I've seen in the glib library. I hadn't really considered implementing something like that myself but I think it is a great idea. \$\endgroup\$ – acarlow Aug 9 '14 at 21:30
  • \$\begingroup\$ Extra parenthesis removed. \$\endgroup\$ – acarlow Aug 9 '14 at 21:31
  • \$\begingroup\$ A question on exact-width integers: I'm probably going to use a version of this code with some embedded C projects. I was thinking that using the standard int, long, etc C types would "port" to an environment like that better, no? \$\endgroup\$ – acarlow Aug 9 '14 at 21:33
  • \$\begingroup\$ I wasn't really sure how best to do the indent/alignment on the #define. My thought was to line up the colons. Any chance you'd like to show how you prefer to see it done? Thanks again. \$\endgroup\$ – acarlow Aug 9 '14 at 21:40
  • \$\begingroup\$ @acarlow I would always go with exact-width integer types for embedded systems. I'll make the edits you requested; but please remember not to change the code in your question in a way that could invalidate reviews. The one way you did with your code I permitted because it was a simple typo that was the only thing preventing your code from working properly. \$\endgroup\$ – syb0rg Aug 10 '14 at 3:42
6
\$\begingroup\$

You will have to write many functions that look the same, but unless you use more macros to generate the code, this is how C works when you want genericity. You should consider factoring out the parts of the functions that are likely to be repeated, such as your error message:

unsigned int
safe_add_uint(unsigned int x, unsigned int y,
              const char* filename, int line_num)
{
    if (x < UINT_MAX - y) {
        return x + y;
    } else {
        exit_with_error("Integer wrap-around occurred", filename, line_num);
    }
}

_Noreturn void
exit_with_error(const char* error, const char* filename, int line_num)
{
    fprintf(stderr,
            "%s, File: %s, Line: %d\n",
            error, filename, line_num);
    exit(EXIT_FAILURE);
}

You could also use curly braces, even for one-line if statements. They always help to avoid errors, even though many think that they are useless for one line. Unfortunately, all I have in mind to simplify your task is some macro trickery to generate the code you need. While that may help, too much macro trickery often leads to code that can be too clever and hard to maintain; that is probably not a "reasonable" way to improve your code.

\$\endgroup\$
  • \$\begingroup\$ Thank you for your input. I too thought that adding more macro "simplification" would probably be going a step too far, but factoring out the error message part seems like a great idea. \$\endgroup\$ – acarlow Aug 9 '14 at 21:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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