return nothing from non-void function in C

Question

what is considered best practice in the following snippet:

int foo(struct data *bar, struct info bla) {
    if (!bar) {
        bla->status = 0;
        return;
    }
    ...
}

in fact, it works fine. but i'm feeling uncomfortable with gcc giving me a warning.

---

here is the actual code:

static int pop(struct stack **stack, struct info *info) {
        int ret;
        struct stack *tmp;

        if (!*stack) {
                info->error = 0;
                return;
        }

        ret = (*stack)->data;
        tmp = *stack;
        *stack = (*stack)->next;
        free(tmp);

        return ret;
}

Answer 1

Best practice is not to write code like that. If you can't return an integer of some sort at that point, you need to redesign your code. Note that the function as written will return a value of some sort to the calling code - you just don't know what that value will be.

The classic way round this is to return the value via a pointer parameter, with the actual function returning a status:

int f( int * p ) {
   if ( bad ) {
       return 0;   // fail indicator
   }
   else {
      * p = 42;    // integer return value
      return 1;    // success indicator
   }
}

Edit: In the code you posted you are manipulating a stack. Popping an empty stack is undefined behaviour for all stacks I know, so you can just return any integer that takes your fancy (I would return 0) and document the behaviour.

Answer 2

Type return 0; or return -1; if this is error and your non error integers is positive signed.

Answer 3

The behaviour is undefined, and the warning is there for a good reason! Return a value, or change the function to a void function.

Answer 4

If you can't return anything, you might think about throwing an exception, or, as already stated, redesigning your code.

Answer 5

int main(void)
{
    printf("hello world\n");
}

returns 10. The bash shell on Mac OS X confirms this. That is, every int returning function returns something back, same should be true for functions that return other types as well. If you don't return explicitly, something which you don't know of is returned. If you are not able to return something at the end of function, try returning void to see if it breaks the code. If it breaks, the functions needs more work, otherwise, continue using void return type.

Answer 6

I would consider adding a third parameter which in fact would be the "return value" and instead of returning the result of the function, just return its status, which could be an enum for example ( and you could have "STATUS_OK", "STATUS_FAIL", "STATUS_NO_RESULT" etc...).

This will be understandable for anyone using your function and at the same time provide the desired behaviour (ie, not returning a value would mean not touching the third parameter and returning "STATUS_NO_RESULT" ).

Answer 7

Under the assumption that the return value is not used for this specific case, simply return 0. If the return value is used, then there is a serious flaw in your program logic that needs to be fixed first.

Answer 8

Since you use info->error to say if the function failed or not, you can return whatever you want, since the caller should ignore the return value. So you can silent the warning with return -1, return 0, return MAGIC_NUMBER... ...

In general however function are coded in the "opposite way": the return value says if the function succeeded or not. If all int return values are good, you can write the function so that it returns failure or success, and on success you fill the data. In your case your info struct could hold a int data, or you can add another arg to the func.

This way the caller can do something like

if ( pop(stack, info) == SUCCESS ) {
  // ...
  printf("%d\n", info->data);
} else { /* failure */
  // info->data holds no data, but info->error could be an error code, e.g.
  fprintf(stderr, "can't pop: %s\n", error_msg[info->error]);
}

The usage in your case is less intuitive:

data = pop(stack, info);
if (info->error != ERROR) {
  // data is valid
} else {
  // data is garbage and we have to say an error occurred.
}

BTW, you do not set info->error to something different by 0, so your code is potentially bugged; e.g.

  info->error = 0;
  data = pop(stack, info);

would trigger always an error even though indeed stack is ok and so data is valid.