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

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 */
     }