# Simpletron in C without initial storing [closed]

I'm studying C with a Deitel's book. The book asks me to write a program that simulates a virtual PC (with virtual memory in an array called memory[100]). Then the book asks me to write a program that sums 10 numbers. I did it but I had to initialize some variables (memory[20], memory[21], memory[22]) before running the program. There is any way to make this program without initially storing the variables and using the same number of instructions?

These are valid instructions:

#define READ 10;
#define WRITE 11;
#define STORE 21;
#define SUBTRACT 31;
#define DIVIDE 32;
#define MULTIPLY 33;
#define BRANCH 40;
#define BRANCHANG 41;
#define BRANCHZERO 42;
#define HALT 43;


And the code:

int main(){
int memory[100] = {2022,1022,3022,2122,2021,3120,2121,4209,4000,1122,4300}; // my set of instruction for sum 10 numbers
memory[99] = -99999;
memory[20] = 1; // the variables initializations that i want to remove
memory[21] = 10;
memory[22] = 0;
int accumulator = 0000;
int instCounter = 00;
int instRegistrer = 0000;
int operationCode = 00;
int operand = 00;

while(memory[instCounter] >= 0){
printf("\n**%d**  ", instCounter);
instRegistrer = memory[instCounter];
operationCode = instRegistrer / 100;
operand = instRegistrer % 100;
switch(operationCode){
scanf("%d", &memory[operand]);
instCounter++;
break;
case WRITE:
printf("WRITE ");
printf("%d", memory[operand]);
instCounter++;
break;
accumulator = memory[operand];
instCounter++;
break;
case STORE:
printf("STORE ");
memory[operand] = accumulator;
instCounter++;
break;
accumulator += memory[operand];
instCounter++;
break;
case SUBTRACT:
printf("SUBTRACT ");
accumulator -= memory[operand];
instCounter++;
break;
case DIVIDE:
printf("DIVIDE ");
accumulator /= memory[operand];
instCounter++;
break;
case MULTIPLY:
printf("MULTIPLY ");
accumulator *= memory[operand];
instCounter++;
break;
case BRANCH:
printf("BRANCH ");
instCounter = operand;
break;
case BRANCHANG:
printf("BRANCHANG ");
if(accumulator < 0)instCounter = operand;
else instCounter++;
break;
case BRANCHZERO:
printf("BRANCHZERO ");
if(accumulator == 0)instCounter = operand;
else instCounter++;
break;
case HALT:
printf("HALT ");
instCounter = 99;
break;
}
}
return 0;
}

• @1201ProgramAlarm Can you illuminate us? What's the error? – Reinderien Dec 24 '18 at 18:05
• @Reinderien As you mention in your answer, all those semicolons in the #define macros, which will expand to case 10;: etc. which won't compile. – 1201ProgramAlarm Dec 25 '18 at 0:42
• Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. Feel free to post a follow-up question linking back to this one instead. – Mast Dec 25 '18 at 7:42
• Closing this question as broken, since the code does not compile. Please refrain from changing the code after receiving answers as outlined in the policy above. – Vogel612 Dec 25 '18 at 16:03
• Note that this answer is currently being discussed on meta – Vogel612 Dec 25 '18 at 23:20

#define READ 10;
#define WRITE 11;
#define STORE 21;
#define SUBTRACT 31;
#define DIVIDE 32;
#define MULTIPLY 33;
#define BRANCH 40;
#define BRANCHANG 41;
#define BRANCHZERO 42;
#define HALT 43;


has a few issues. First of all, you shouldn't add semicolons after those defines. Also, this would be better represented as an enum, since op codes are mutually exclusive.

int memory[100] = {2022,1022,3022,2122,2021,3120,2121,4209,4000,1122,4300}; // my set of instruction for sum 10 numbers


also needs a few adjustments. Since none of these values will exceed 9999, you should be storing them as int16_t, not int. Furthermore, you shouldn't be writing raw machine code in that array. You should be constructing those values like

{
// ...
}


Also, it seems like you store -99999 as a magic value to indicate program termination, when this is technically not necessary. I don't have the specifications for your VM, but it would make more sense to

• have all of the memory contents be of type int16_t
• on HALT, simply terminate the program

rather than

• on HALT, jump to the end of memory
• at the end of memory, have a magic negative value.

There's a typo in instRegistrer. It's "register", not "registrer".

I propose that, instead of having to manually write a different printf string for each instruction,

1. declare an array of 44 character pointers
2. define a macro that indexes into the array based on the opcode value, and initializes that element to a stringized (#) version of the opcode
3. call the macro for every opcode that you have

You could take this strategy further and store a struct at each element of the array, containing a string name and an operation function pointer. This would nicely generalize your code and make execution of long programs faster.

You're also keen on avoiding separate assignment of memory contents for use by variables. This can be done by shifting your "data" memory to the address right after your "program" memory, and then doing the initialization in your array literal.

The following incorporates all of the above. It seems you're still learning basic C, so some of these concepts may require explanation - please let me know in the comments.

#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>

typedef enum
{
WRIT = 11,
STOR = 21,
SUB = 31,
DIV = 32,
MUL = 33,
BRA = 40,
BRNG = 41,
BRZR = 42,
HALT = 43
} OpCode;

typedef struct
{
int16_t mem[100], acc;
uint8_t counter;
} VM;

typedef struct
{
const char *name;
} Operation;

static uint16_t makeOp(OpCode code, uint8_t addr)
{
}

#define DEFOP(op) ops[op] = (Operation){.name=#op, .execute=exc_##op}

int32_t x;
if (scanf("%d", &x) != 1)
{
exit(1);
}
if (x >= 10000 || x <= -10000)
{
fprintf(stderr, "Invalid value %d", x);
exit(1);
}
vm->counter++;
}
static void exc_WRIT(VM *vm, uint8_t addr) {
vm->counter++;
}
vm->counter++;
}
static void exc_STOR(VM *vm, uint8_t addr) {
vm->counter++;
}
vm->counter++;
}
static void exc_SUB(VM *vm, uint8_t addr) {
vm->counter++;
}
static void exc_DIV(VM *vm, uint8_t addr) {
vm->counter++;
}
static void exc_MUL(VM *vm, uint8_t addr) {
vm->counter++;
}
static void exc_BRA(VM *vm, uint8_t addr) {
}
static void exc_BRNG(VM *vm, uint8_t addr) {
if (vm->acc < 0)
else
vm->counter++;
}
static void exc_BRZR(VM *vm, uint8_t addr) {
if (!vm->acc)
else
vm->counter++;
}
static void exc_HALT(VM *vm, uint8_t addr) {
exit(0);
}

int main()
{
const uint8_t X = 13, I = 12, DELTA = 11;

Operation ops[HALT+1];
DEFOP(WRIT);
DEFOP(STOR);
DEFOP(SUB);
DEFOP(DIV);
DEFOP(MUL);
DEFOP(BRA);
DEFOP(BRNG);
DEFOP(BRZR);
DEFOP(HALT);

VM vm = {.counter = 0,
.acc = 0,
.mem = {
makeOp(STOR, X),
makeOp( SUB, DELTA),
makeOp(STOR, I),
makeOp(BRZR, 9),
makeOp( BRA, 0),
makeOp(WRIT, X), // 9: exit loop
makeOp(HALT, 0),
1,  // 11: DELTA
10, // 12: I
0   // 13: X
}};

for (;;) {
int16_t current = vm.mem[vm.counter];
uint8_t code = current/100,

• An enum is basically an integer, but its values are constrained to a certain named list. Be careful about your 4-digit notation, because it sometimes doesn't do what you expect: for instance, 0244 is interpreted as octal instead of decimal. – Reinderien Dec 24 '18 at 19:40