How do I resolve the "control reaches end of non-void function" warning in the case with a custom error handler?

Question

Consider the following example:

a.c:

#include <stdlib.h>
#include <fcntl.h>
#include <stdarg.h>
#include <stdio.h>

char *invocation_name;

static void handle_error(const char* fmt, ...) {
    va_list args;
    va_start(args, fmt);
    fprintf(stderr, "%s: ", invocation_name);
    vfprintf(stderr, fmt, args);
    puts("");
    va_end(args);
    exit(1);
}

static int open_a_file(void) {
    int fd = open("non-existent", 0);
    switch (fd) {
        default: return 0;
        case -1: handle_error("something happened"); 
    }
}

int main(int argc, char **argv)
{
    (void) argc; /* unused */
    invocation_name = argv[0];
    open_a_file();
}

The example is a bit far-fetched, but this way it's smaller. In a real program I had a function that accepted an int and returned a string representation, or exited with an error if the int was out of range.

When I compile it:

$ gcc -Wall -Wextra -pedantic -Wmissing-prototypes -Wstrict-prototypes \
    -Wold-style-definition -std=c99 -O3 \
    a.c
a.c: In function 'open_a_file':
a.c:24:1: warning: control reaches end of non-void function [-Wreturn-type]
   24 | }
      | ^

How do I make gcc happy?

UPD I guess I had to provide the function I had when I encountered the issue, not invent a new function for the question. But I wanted the question to be small and easily reproducible... Anyway imagine the following function in place of open_a_file():

static const char *attr_name(uint16_t type) {
    switch (type) {
        case ATTR_MAPPED_ADDRESS:      return "MAPPED-ADDRESS";
        case ATTR_XOR_MAPPED_ADDRESS:  return "XOR-MAPPED-ADDRESS";
        case ATTR_SOFTWARE:            return "SOFTWARE";
        case ATTR_FINGERPRINT:         return "FINGERPRINT";
        case ATTR_RESPONSE_ORIGIN:     return "RESPONSE-ORIGIN";
        default: handle_error("attr_name: unknown attr type (0x%02x)", type);
    }
}

I can't do return handle_error(..., because then I have to make handle_error() return int.

The program is in its early stages (and actually may remain just a prototype), so yeah, the error handling might be somewhat primitive or basic. And it might happen that attr_name() receives an unknown type. If the program receives a packet with an unknown type.

I'm indeed planning to make parsing a UDP packet return an error, and, if I won't abandon the development, make use of some libraries, so the warning might resolve itself in the near future. But at the moment that's what I've got.

Answer 1

Use the convenience macro noreturn, defined in <stdnoreturn.h>, which expands to _Noreturn:

#include <stdnoreturn.h>
noreturn static void handle_error(const char* fmt, ...) {
    ...
}

C23 (-std=c2x) introduces the attribute noreturn, so if you don't need compatibility you can use this:

[[noreturn]] static void handle_error(const char* fmt, ...) {
    ...
}

Answer 2

This is all about proper program design.

And proper program is to never have some middle layer code doing error decision making and closing down shop with exit() etc. Instead we should design the program so that all errors are returned to the caller and the error handler is located in the top layer of the application, which is the only party that should make the decision to close down.

One obvious reason for this is that there might be other clean-up code that needs to be executed beyond what the specific module is aware of. But also it helps maintenance a lot to centralize all error handling and decision making to one place. Some errors might not be critical enough to merit a close down, for example.

For this reason, all professional library design tends to reserve the return value of each API function for a result/error code. Just having true/false or 0/-1 as results is way too blunt in a real program - more information is needed and so usually an enum is used for that purpose. Any values that need to be returned are passed by parameters.

It does make sense to hide old 1970s skunkware like fctrl.h inside a wrapper function library - essentially what stdio.h was supposed to do if that one wasn't just as badly written as fctrl.h... One has to realize that most of the old Unix gunk is not some canonical model to look at for inspiration, but rather discouraging examples for how to never write code, pretty much as far from proper programming practices as C code gets.

So we'd rewrite the code like this:

typedef enum
{
  FILE_OK,         // number 0 traditionally used for "no error"
  FILE_ERR_OPEN,
  /* add as many error codes as needed */
} file_err_t;

file_err_t open_a_file (int* fd) {  // we do need to return the file descriptior somewhere
    *fd = open("non-existent", 0);

    // in a real application we ought to check errno here and translate to an error code
    // but for now...:
    if(*fd == -1)
      return FILE_ERR_OPEN; 
    return FILE_OK;
}

Note that our wrapper library really ought to handle all of the file descriptor stuff as well as the 1970s errno error handling. Translating errno to our custom enum with either the same amount of detail or some simplified abstraction if that's what's required.

The next thing to do would be to move knowledge about the severity of the error away from the middle layer and into the error handler. In which case we might also place the knowledge about what message to display to the error handler.

And while we are at it we might note that another 1970s skunkware lib stdarg.h is a pox - it shouldn't be present in any program, because it has non-existent safety and we really don't need an error handler (or any other function) to have a variable number of arguments. The need for that comes from muddy program design. Chances are that our error handler will just be causing a lot of errors in itself and that's obviously not a great state of affairs.

Ideally we'd put the error handler code in a file of its own. With proper code prefixes for all functions belonging to that error handler. Maybe file_err_t should actually be in the header of the error handler, if this is a centralized error type for the whole project.

static const char* invocation;

void err_handler_init (const char* invocation_name)
{
  invocation = invocation_name;
}

void err_handler (file_err_t err) 
{
    switch(err)
    {
       /* all decision making is done here, not in at some random place in the program */
       case FILE_ERR_OPEN: 
         fprintf(stderr, "%s: Couldn't open file.", invocation_name);
         exit(1); // this particular error merits a close down
         break;
    }
}

And then main.c becomes something like this:

#include "file_handling.h"
#include "error_handler.h"

int main(int argc, char **argv)
{
    (void) argc; /* unused */
    err_handler_init(argv[0]);
    
    int fd;
    file_err_t result = open_a_file(&fd);
    if(result != FILE_OK)
    {
        err_handler(result);
    }
    /* do something with the file */
}

Answer 3

Compiler does not know what will happen if fd==-1.

Simply change the way you handle the condition:

static int open_a_file(void) 
{
    int fd = open("non-existent", 0);
    if(fd == -1) handle_error("something happened"); 
    return 0;
}

or add return statement at the end (it will not increase the code size)

static int open_a_file(void) 
{
    int fd = open("non-existent", 0);
    switch (fd) 
    {
        default: return 0;
        case -1: handle_error("something happened");  
    }
    return 0;
}