add operation on a pointer value C

Question

I'm having problems trying to create an assembly emulator program in C. There are 5 registers: REGA, REGB, REGC, REGX AND INSP, and 10 instructions: NOP, SET, AND (bitwise &), OR (bitwise |), ADD, SUB, SHL(<< left), SHR(>>), JMP.

The program reads instructions from a file; with lines containing the instruction and 2 arguments. In most cases the 1st argument is a register name (e.g. REGA) and the 2nd argument can be a register name or an integer.

I'm using sscanf to get the instructions from the file.

I'm having trouble with the ADD, SUB, SHL and SHR functions. My ADD function is:

int opcode_add(char* opcode, char *arg1, char *arg2){
    int i, j;
    for(i = 0; i < MAX_REGISTER; i++){ 
        if (strcmp(register_str[i],arg1) == 0){ 
            for(j = 0; j <=MAX_REGISTER; j++){ 
                if(strcmp(register_str[j],arg2) == 0){ 
                    *register_ptr[i] = *register_ptr[i] + *register_ptr[j]; 
                    break;
                }else {
                    *register_ptr[i] = *register_ptr[i] + atoi(arg2); 
                }       
            }

        }
    }
    INSP++; 
    return (0); 
}

The function works if 2 register arguments are passed. For example:

SET REGA 1
SET REGB 2
ADD REGA REGB

but not if a register and an integer are passed. For example:

SET REGA 2
ADD REGA 1

The problem is at this line:

*register_ptr[i] = *register_ptr[i] + atoi(arg2);

I tried doing this:

int y = *register_ptr[i];
int k = atoi(arg2);
int result = y+k;
*register_ptr[i] = result;

but it didn't work.

This looks suspicious: for(j = 0; j <=MAX_REGISTER; j++) The outer for's terminating condition is i < MAX_REGISTER and both loops index register_str. — hmjd
– hmjd, Commented Jun 4, 2012 at 14:53

Aasmund Eldhuset · Accepted Answer · 2012-06-04 14:54:05Z

1

You didn't say what didn't work - however, there is at least one error in the way you have structured the inner loop and the if. Whenever the inner loop sees a register that does not match the second operand, the else block will be executed - so all registers before the one specified by the second operand will be added to (and if the second operand is an int, all registers will be added to). The contents of the else block must be moved after the loop, and they must only be executed if the inner loop didn't find the register.

answered Jun 4, 2012 at 14:54

Aasmund Eldhuset

38.1k4 gold badges74 silver badges85 bronze badges

Sign up to request clarification or add additional context in comments.

7 Comments

Maj Over a year ago

Sorry. I didn't receive any error messages. The problem is that the calculation isn't correct. For example, if I: SET REGA 1; then ADD REGA 1 - the else statement should be executed (because arg2 isn't a register, it's an int). This it does; however, the result from the calculation is wrong. In the example, the result I receive is incorrect.

Aasmund Eldhuset Over a year ago

@Maj: Yes, because the else will be executed multiple times because of the error I described. Make sure you understand why that happens (either by debugging or by inserting cout statements that show when the if part and when the else part are being executed) before you try to fix it.

Maj Over a year ago

Thanks for your help. I haven't used the debugger; I've not tried it yet. I managed to fix it by moving the contents of the else statement after the first if.

Aasmund Eldhuset Over a year ago

@Maj: So the same else block is now after if (strcmp(register_str[i],arg1) == 0)? Then I think you have introduced another error: For instance, ADD REGC 1 will now add 1 to all registers before C, and ADD REGC REGA will crash or produce a wrong result.

Maj Over a year ago

Yep, I put it there. But, I didn't put as an else block; just the code that was in it - within the if, before the 2nd loop. It seems to work. I just did SET REGC 1, ADD REGC 2; the result was indeed 3.

|

Shahbaz · Accepted Answer · 2012-06-04 15:10:15Z

The problems are here:

for(j = 0; j <=MAX_REGISTER; j++){ 
    if(strcmp(register_str[j],arg2) == 0){ 
        *register_ptr[i] = *register_ptr[i] + *register_ptr[j]; 
        break;
    }else {
        *register_ptr[i] = *register_ptr[i] + atoi(arg2); 
    }       
}

First of all, you should check for j < MAX_REGISTER because your array (apparently) doesn't include MAX_REGISTER itself. But what is even more important, is that you shouldn't try atoi as soon as one register name mismatches.

What you have done now is that, if the second argument is not REGA, you immediately go to else and try atoi. What you should do is to check for all registers, if arg2 was none of them, then try atoi:

bool found = false;
for(j = 0; j < MAX_REGISTER; j++){ 
    if(strcmp(register_str[j],arg2) == 0){ 
        *register_ptr[i] = *register_ptr[i] + *register_ptr[j]; 
        found = true
        break;
    }       
}
if (!found)
    *register_ptr[i] = *register_ptr[i] + atoi(arg2);

Collectives™ on Stack Overflow

add operation on a pointer value C

2 Answers 2

7 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

7 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related