Warning 'return' with no value, in function returning non-void - What should it return?

Question

How do I solve the following throwing the warning in the title?

struct Nodes* InsertNode(unsigned int IP, unsigned short Port)
{
    if (!IP)
       return;
    if (!Port)
       return;
    // Above is what chucks the warnings
    {
        // do stuff & conditionally
           return &List[x];
    }
    // Different conditions & stuff
    {
       return &List[Other];
    }
}

In other words, in the case of giving up through missing data, what should it return? Or do I need to trawl through the entire body of code and have checks every time to see if it should be called or not? The program functions as intended just returning at that point, if I'm to continue using it (or upgrade the OS it's running on), fixing compiler warnings seems like a good idea, they tend to turn into errors when compiler versions get bumped.

There's a clue in this answer which answers someone asking about the same warning, the answer doesn't give me quite enough info to proceed though, nor do the other's I've read.

Extra information: The check on the values of IP & Port are there to sanitize the content of &List, such cases indicate datagrams from misconfigured clients or traffic from persons with malicious intent, sad but it happens. It's invalid data we don't care about at all, logging it seems pointless, it shouldn't delay processing the next one, and absolutely not halt the program. Until the switch from gcc 4.9 to 6.3 I didn't see a warning. The current return; appears to simply black-hole it, but I only understand bits of the code's intent.

Answer 1

TLDR

"Until the switch from gcc 4.9 to 6.3 I didn't see a warning." Try compiling with gcc -std=gnu90 to compile under similar conditions to those that worked before, when you were using gcc 4.9.

OK, I'm Listening

The reason that you see compiler warnings after changing compilers from gcc 4.9 to gcc 6.3 is that gcc 4.9 defaulted to C90 (really the gnu90 dialect of C90), but by gcc 5.5 the default was C11 (really gnu11).

The C90 Standard says in the Constraints section about the return statement that (C90 §6.6.6.4):

A return statement with an expression shall not appear in a function whose return type is void.

But the same Constraints section from the C11 Standard saysC11 §6.8.6.4:

A return statement with an expression shall not appear in a function whose return type is void. A return statement without an expression shall only appear in a function whose return type is void.

Now, the compiler must produce a diagnostic message for any constraint violation (§5.1.1.3). No constraint was violated when your code was compiled under C90, but the change to a more recent compiler means that the code now compiles under C11, where there is a constraint violation, hence the warning.

One option would be to simply compile with gcc -std=gnu90, allowing the code to be compiled using the same dialect of C you used before, even on the more recent compilers.

But, also note that the original code may have had undefined behavior, since (C90 §6.6.6.4):

If a return statement with an expression is executed, and the value of the function call is used by the caller, the behavior is undefined.

If the value returned by InsertNode() is used by the caller, and the return; statement is encountered in the function call, you have undefined behavior. The best choice would be to look at all of the calls to InsertNode() to see how they handle the return values. It is possible that return; is a typo, and that the code already handles returned null pointers, in which case changing to return NULL; would be all that is needed to fix the code. If the code does not already handle null pointers, @alk has provided several options for fixing the code.

Answer 2

in the case of giving up through missing data, what should it return?

As often it depends.

There are several scenarios

1. The function is not designed to return NULL as a valid value.

   Replace

   if (!IP)
     return;
   if (!Port)
     return;
   

   by

   if (!IP || !Port)
   {
     errno = EINVAL; /* Setting errno, allows the caller to log 
                        the failure using the perror() function. */
     return NULL;
   }
   

   Use it like this:

   struct Nodes * p = InsertNode (...);
   if (NULL == p)
   {
      perror("InsertNode() failed");
      /* exit or error logging/handling */
   }
   

2. IP and Port will never be 0 under normal operation. So if they were it would be a programming mistake.

   In those cases you probably no don't return but end the program.

   So instead of

   if (!IP)
     return;
   if (!Port)
     return;
   

   use

   assert((IP) && (Port));
   

   No specific usage necessary here as the program would simply end if the assertion isn't met.

   Please note that this approach requires extensive testing as the test will typically be removed in a production/release build!

3. The function may return NULL as valid value and IP and/or Port may be 0 under normal operation.

   Redesign the function to in one way or the other return a separate error status.

   This can generally be done in two ways:

   • Use the function's return value and pass back the result via a pointer being passed as parameter

     int InsertNode(unsigned int IP, unsigned short Port, struct Nodes** ppresult)
     {
       int error_state = 0;
     
       if (!IP || !Port || !ppresult)
       {
         errno = EINVAL; /* Setting errno, allows the caller to log 
                        the failure using the perror() function. */
         error_state = -1;
       }
       else
       {
         if (...)
         {
           *ppresult = &List[x];
         }
     
         ...
     
       }
     
       return error_state;
     }
     

     Use it like this:

     struct Nodes * p;
     if (-1 == InsertNode (..., &p))
     {
        perror("InsertNode() failed");
       /* exit or error logging/handling */
     }
     

   • Pass back the error state result via a pointer being passed as parameter

     struct Nodes * InsertNode(unsigned int IP, unsigned short Port, int * perror_state)
     {
       int error_state = 0;
     
       if (!IP || !Port || !perror_state)
       {
         errno = EINVAL; /* Setting errno, allows the caller to log 
                        the failure using the perror() function. */
         error_state = -1;
       }
       else
       {
         if (...)
         {
           *ppresult = &List[x];
         }
     
         ...
     
       }
     
       *perror_state = error_state;
     
       return NULL;
     }
     

     Use it like this:

     int result;
     struct Nodes * p = InsertNode (..., &result))
     if (-1 == result)
     {
       perror("InsertNode() failed");
       /* exit or error logging/handling */
     }