Is there a better way of returning a value in c than having multiple return statements?

Question

In getFieldSignExtended(int,int,int), I have if-else statements inside if-else statements. I have int result as a global variable of this function. Depending on where program control flows, I want this function to return result2.

At first I had one return statement at the bottom of this function, that didn't work and I found out that scope in C is not like in Java. Thus I return 1; at the bottom of the function, and I have 8 return result2 statements in the if-else blocks.

Is there a better way to organize this function? I don't want to nest if-else blocks and I want as few return statements as possible.

This is homework, but it was already graded and I'm just correcting a few errors that came up.

getFieldSignExtended(int,int,int) gets a bitfield from value from hi to lo inclusive (hi and lo can be == to eachother, etc.) and sign extends it (based on testing the sign bit). All of this code deals with 2's complement.

If you find any other big C convention mistakes, I'll be glad to correct them.

Thanks in advance.

int getFieldSignExtended (int value, int hi, int lo) {
    unsigned int result = 0;
    int result2 = 0;
    unsigned int mask1 = 0xffffffff;
    int numberOfOnes = 0;

    if((hi == 31) && (lo == 0)) {
            result2 = value;
            return result2;
    }

    if((lo == 31) && (hi == 0)) {
            result2 = value;
            return result2;
    }
    else if(hi < lo) {
        // Compute size of mask (number of ones).
        numberOfOnes = lo-hi+1;
        mask1 = mask1 << (32-numberOfOnes);
        mask1 = mask1 >> (32-numberOfOnes);
        mask1 = mask1 << hi;
        result = value & mask1;
        result = result >> hi;
        if(result & (0x1 << (numberOfOnes-1))){
            // if negative
            int maskMinus = (0x1 << numberOfOnes);
            maskMinus = maskMinus -1;
            maskMinus = ~maskMinus;     
            result2 = maskMinus | result;
        }
    } else if(lo < hi) {
        // The number of ones are at the 'far right' side of a 32 bit number.
        numberOfOnes = hi-lo+1;
        mask1 = mask1 >> (32-numberOfOnes);
        mask1 = mask1 << lo;
        result = value & mask1;
        result = result >> lo;
        if(result & (0x1 << (numberOfOnes-1))){
            //if negative
            int maskMinus = (0x1 << numberOfOnes);
            maskMinus = maskMinus -1;
            maskMinus = ~maskMinus;     
            result2 = maskMinus | result;
            return result2;
        }
    }else{
        // hi == lo
        unsigned int mask2 = 0x1;

        // Move mask2 left.
        mask2 = mask2 << hi;
        result = mask2 & value;
        result = result >> hi;
        if(result == 0x1){
            result2 = 0xffffffff;
            return result2;
        }
        else{
            result2 = 0x0;
            return result2;
        }   
    }   

    return 1;
}

Answer 1

The general answer is to set aside a variable to store your return value (e.g., ret_val) and assign your return value to it in places where you are currently returning a value. You may have to adjust your control flow also since now you are not exiting your function with a value at those places.

Then, at the "bottom" of your function you can return once with the value of ret_val. I.e.,

 return ret_val;

once should be sufficient with the changes above.

Looking over your algorithm and rearranging your code, perhaps delegating some of the work to functions might be another approach to help simplify/clarify your code eliminating the need for multiple (or excessive, your judgment call) returns.

Answer 2

Just looking at the getFieldSignExtended function, the problem as I see it is pretty simple. You only have 2 places where it can return right now... lets simplify it a little to show you what I mean:

if (A)
  do something
if (B)
  do the same thing
else if (C)
  do something else
else if (D)
  do something else
else
  do something else 

So all you really need to do is combine A & B, by doing that there's only a single flow in your function, it can't go down an "if" and an "else if", only one of them. So if you do this:

if(((hi == 31) && (lo == 0)) || ((lo == 31) && (hi == 0))){
    result2 = value;
}else if(hi < lo){  
...
// the rest as is

Then every where you return result2; just delete that line, and at the end instead of return 1; just return result2;

Now you just have a single return statement for this function.

Answer 3

sorry too much reading, and i didn't do anything in c for ages, but you can create small function which swap lo and hi, and set hi as bigger value and lo as lower, then you will not need so many block which are basically doing same thing

Answer 4

You can use goto statement and at single point you can return result2 value. if nicely explained here

Answer 5

The result and result2 are not "global variables of the function", they are local variables. The problem is not in that "scope in C is not like in Java", but that some of the branches in your function fail to assign result2. Specifically, there is no assignment in the branch that does

printf(" result2 %08x \n",result2);

If you declare a variable without a value in Java, the compiler look for code paths that uses this variable before an assignment, and alerts you if there is one; in C you have to watch for these situations yourself.

If you put return result2 at the bottom, and make sure that all code paths assign result2 the right value, your code will work with a single return statement, just like your setField does.

Answer 6

You don't need the multiple return statements, not as your function works at the moment. Just initialize result2 to 1 and do return result2; at the end of the function.

Or instead of assigning to result2 and then directly returning, why not just do e.g. return value; or return maskMinus | result; etc.