Array of structures and passing pointers to structs

[Vendrus]

Background: I'm writing a convolutional encoder (and decoder, eventually) for a microprocessor (PIC24), for which I'm using structs and pointers to move from state to state. So far as I'm aware, everything I'm using in the PIC involves nothing other than ANSI C.

I have a little experience with structures, having written a linked-list program for a class a couple years back, but nothing since and never used structure arrays. I have the feeling I'm missing something basic here, which is what's so frustrating. The most confusing error (and I suspect the root of most of them) is the 'state undeclared', which I just can't figure.

Any help would be very much appreciated.

The errors I'm getting are:
encoder.c:11: warning: 'struct memstate' declared inside parameter list
encoder.c:11: warning: its scope is only this definition or declaration, which is probably not what you want
encoder.c: In function 'state_init':
encoder.c:22: error: two or more data types in declaration specifiers
encoder.c:25: error: 'state' undeclared (first use in this function)
encoder.c:25: error: (Each undeclared identifier is reported only once
encoder.c:25: error: for each function it appears in.)
encoder.c:48: error: incompatible types in assignment
encoder.c: In function 'main':
encoder.c:58: error: storage size of 'current_state' isn't known
encoder.c:58: warning: unused variable 'current_state'
encoder.c:58: error: storage size of 'current_state' isn't known

Code:

//Includes
#include <stdlib.h>


//------------------------------------------------------------------------------
//Creates state machine and passes back pointer to 00 state
void state_init(struct memstate* startpoint)
{          
    extern struct memstate
    {
        char output0;                    //output if next input is 0
        char output1;                    //output if next input is 1
        struct memstate *prev;            //prev if input is 0, next if 1
        struct memstate *next;    
    }
        
    //Initialise states
    struct memstate states[4];
    
    //Configure and link state 00
    states[0].next = state[1];
    states[0].prev = state[0];
    states[0].output0 = 0b00;
    states[0].output1 = 0b11;


    //Configure and link state 01
    states[1].next = state[3];
    states[1].prev = state[2];
    states[1].output0 = 0b10;
    states[1].output1 = 0b01;


    //Configure and link state 10
    states[2].next = state[1];
    states[2].prev = state[0];
    states[2].output0 = 0b11;
    states[2].output1 = 0b00;


    //Configure and link state 11
    states[3].next = state[3];
    states[2].prev = state[2];
    states[2].output0 = 0b01;
    states[2].output1 = 0b10;
    
    startpoint = states[0];
}




//------------------------------------------------------------------------------
//Main encoding function
int main (void)
{
    //1 is next, 0 is prev
    char output, start = 1, input;
    static struct memstate current_state;


    if(start == 1)
     {
        state_init(*current_state);        //Start point is 00 state
        start--;
    }


    do{
        //Output and next state for input of 1
        if(input == 1)
        {
            output = current_state.output1;
            current_state = current_state->next;
        }


        //Output and next state for input of 0
        if(input == 0)
        {
            output = current_state.output0;
            current_state = current_state->prev;
        }    
    }
    while(1);


}

NB: I'm aware that at the moment, this code will do nothing except spin round that do-while loop. Once it's actually compiling I'll drop in some simple button-based test code so it'll check for the correct output.

[AndiPersti]

void state_init(struct memstate* startpoint)
{          
    extern struct memstate
    {
        char output0;                    //output if next input is 0
        char output1;                    //output if next input is 1
        struct memstate *prev;            //prev if input is 0, next if 1
        struct memstate *next;    
    }

You should declare your struct outside any function otherwise it's not visible to other parts of the program.

Bye, Andreas

[std10093]

Change your code to this. You try to use something you haven't yet defined.

struct memstate
    {
        char output0;                    //output if next input is 0
        char output1;                    //output if next input is 1
        struct memstate *prev;            //prev if input is 0, next if 1
        struct memstate *next;   
    };
 
//------------------------------------------------------------------------------
//Creates state machine and passes back pointer to 00 state
void state_init(struct memstate* startpoint)
{         
         
    //Initialise states
    struct memstate states[4];
     ...

I also removed the extern, but if you believe that you need it for some reason, say it.

Also, in the function, what is state? You wanted to say states?

Also the comment // Initialise states should be // Initialize states (but of course this does not cause any error or warning.

You had

startpoint = states[0];

but startpoint is a pointer to the struct and states[0] is a struct, so you might want to use it that way

startpoint = &states[0];

You have this

static struct memstate current_state;
 
 
    if(start == 1)
     {
        state_init(*current_state);

But the function requires a pointer, so you have to call the function like this

state_init(&current_state);

Then you have this

output = current_state.output1;
            current_state = current_state->next;

and you are confused because next is a pointer. However, the current_state is the key to decide if you are going to use the arrow operator -> or the dot operator . .Since current_state is not a pointer, you need to write this

output = current_state.output1;
            current_state = current_state.next;

Likewise for the prev.

Ok then, you have, inside the function again, this ( assuming you meant states instead of state)

states[0].next = states[1];

as another similar thing I said before, you have a pointer to the struct and the struct, so you need to assign the address of the struct, like this

states[0].next = &states[1];

Same thing stands for some other equivalent lines of code in the function.

Now back in main, we have done this change

current_state = current_state.next;

But what you want is to go the next struct. For this I would suggest you to use a pointer. If you read my post, then you might be able to figure out what changes you need to make.

Also about b01 inside the function, are you trying to use octal numbers?

[Vendrus]

Thank you to both of you - that missing s would explain rather a lot...
I'm going to go through and implement/understand your changes, but for now, I was under the impression that 0b01 would indicate a two-bit binary value, which is what I'm chucking around. This is coming from a comms background, so what I'm handling is a conversion from unrelated single bits to interlinked bit pairs which can be error-corrected after a wireless transmission. I can easily switch those to hex or dec values, but it was easier for initial reference.

[std10093]

I would suggest you to use normal integers for now. Then, when you see that your code runs ok, change to two-bit binary values. Also, welcome to the forum

[Vendrus]

Okay, that solved most of the problems, but I've failed to get the current_state pointer conversion right. I tried what I thought would work, but now I think I'm just throwing punctuation at it and hoping. From what I've just read, the error I'm getting means that I'm not declaring a structure called current_state, but if I'm declaring it as a pointer to the structure, does that mean I need to create a structure separately? I'm kinda lost.

(And I've clued in that if I don't include all of the code, line numbers for the errors won't match up. Derp.)
((Extern was because I though that I wouldn't be able to use the struct type in main as well as state_init. Compiler didn't complain when I took it out.))

Errors:
encoder.c:11: warning: 'struct memstate' declared inside parameter list
encoder.c:11: warning: its scope is only this definition or declaration, which is probably not what you want
encoder.c: In function 'state_init':
encoder.c:22: error: two or more data types in declaration specifiers
encoder.c: In function 'main':
encoder.c:62: warning: passing argument 1 of 'state_init' from incompatible pointer type
encoder.c:70: error: dereferencing pointer to incomplete type
encoder.c:71: error: dereferencing pointer to incomplete type
encoder.c:77: error: dereferencing pointer to incomplete type
encoder.c:78: error: dereferencing pointer to incomplete type

//------------------------------------------------------------------------------//File:                 encoder.c
//Description:          Wireless comms data encoder (convolutional)
// -----------------------------------------------------------------------------


//Includes
#include <stdlib.h>


//------------------------------------------------------------------------------
//Creates state machine and passes back pointer to 00 state
void state_init(struct memstate* startpoint)
{          
	struct memstate
	{
		char output0;					//output if next input is 0
		char output1;					//output if next input is 1
		struct memstate *prev;			//prev if input is 0, next if 1
		struct memstate *next;	
	}
		
	//Initialise states
	struct memstate states[4];
	
	//Configure and link state 00
	states[0].next = &states[1];
	states[0].prev = &states[0];
	states[0].output0 = 0;
	states[0].output1 = 3;


	//Configure and link state 01
	states[1].next = &states[3];
	states[1].prev = &states[2];
	states[1].output0 = 2;
	states[1].output1 = 1;


	//Configure and link state 10
	states[2].next = &states[1];
	states[2].prev = &states[0];
	states[2].output0 = 3;
	states[2].output1 = 0;


	//Configure and link state 11
	states[3].next = &states[3];
	states[2].prev = &states[2];
	states[2].output0 = 1;
	states[2].output1 = 2;
	
	startpoint = &states[0];
}




//------------------------------------------------------------------------------
//Main encoding function
int main (void)
{
	//1 is next, 0 is prev
	char output, start = 1, input;
	static struct memstate* current_state;


	if(start == 1)
 	{
		state_init(current_state);		//Start point is 00 state
		start--;
	}


	do{
		//Output and next state for input of 1
		if(input == 1)
		{
			output = current_state->output1;
			current_state = current_state->next;
		}


		//Output and next state for input of 0
		if(input == 0)
		{
			output = current_state->output0;
			current_state = current_state->prev;
		}	
	}
	while(1);


}

[Tclausex]

If you want to express your ideas in binary, use hex. It is natural shorthand for it.


Edit:
As to your latest code:

1. You need to define your struct memstate before your functions. The compiler needs to see it before you try to define init_state() with a parameter of that type.
2. You need a semicolon after the struct memstate definition.
3. You are trying to modify current_state within init_state, but startpoint is a local copy of current_state (which is a pointer pointing to who-knows-what). Like any variable, if you want to modify something outside of its scope, you need to pass its address to the function and the function needs to have a parameter that is a pointer to that type.
4. Input is used without initializing it.
5. As it stands, the if(start == 1) is unnecessary - main() is the entry point to your program - you could simply have the call to state_init(&current_state) as your first instruction. (Of course, I have no idea what else you're putting in there later).

[std10093]

Read again my post (#3).

[Vendrus]

It's up and running now, so many thanks for all the (incredibly prompt) help!

[std10093]

You are welcome. As a final adjustment, make your main of this form

int main(void)
{
    // your code
    return 0;
}