C struct bug?

Legacy Open AT
1

OS: 3.12, FW: 6.57

In headers:

typedef struct {
    bool                connected;
    u32                 opid;
    ascii *             opname;
} ma_op_t;

In code:

ma_op_t op;

void init(){ 
  op = adl_memGet( sizeof ( ma_op_t ) );
}

adl_atCmdCreate(..., Handler, NULL);

s16 Handler(adl_atResponse_t *paras){
   ascii * tmp = (ascii *) adl_memGet((u16) 20);
   // tmp filled in other place.

   op->connected = TRUE;
   op->opid = 22100;
   op->opname = tmp;

   adl_memRelease(tmp);

   // op->opid  returned 22100
}

// example timer, run after ma_op_t op is filled;
void My_TimerHandler ( u8 Id ){
   // in this place 
   // op->connected returned TRUE, filled value
   // op->opname returned good value, filled value

   // op->opid returned bad value = -369082370
}

op->opid returned bad value in My_TimerHandler,
where is problem?

C struct bug?

2

How are you printing out the values? For unsigned long (u32) you need to use %Lu.

3

Does this actually work? :open_mouth:

‘op’ is a structure of type ‘ma_op_t’, but adl_memGet returns a void* pointer…

Don’t you mean something like:

ma_op_t * op_ptr;

void init(){ 
  op_ptr = (ma_op_t *)adl_memGet( sizeof ( ma_op_t ) );
}
4

Why not? :slight_smile:

5

i’am use %d, (is debug)

strange error, if declare global

u32 stOpid = 0;

and replace all in code

op->opid

to

stOpid

all works fine.

6

Because, as I also said:

what do you expect to get if you assign the value of a pointer (ie, an address) to a structure…? :open_mouth:

7

with use:

ma_op_t * op_ptr; 

void init(){ 
  op_ptr = (ma_op_t *)adl_memGet( sizeof ( ma_op_t ) ); 
}

no changes, field u32 return invalid value

8

Hi usa !

To help you, we need to see what you realy do in your program :

Obviously it’s not the code you have written, as this would not compile. You cant access op with “->” if it is not a pointer. Another thing here :

This is dangerous ! You copy “tmp” pointer value to “opname” pointer value and then release tmp… the memory is not allocated anymore and can be overwritten (you should instead copy “tmp” array elements to “opname” array).

C boolean type doesn’t exist… I think the real test is :

booleanVars!=0

So, if “op->connected” return TRUE, it just means that the value is different than 0… (but this is often the case…)

9

Absolutely - which is why I asked earkier:

You need to post your code using copy-and-paste from your actual source file - any manual re-typing is likely to introduce differences between the code you are actually using and what’s seen on the forum.

10

found mistake in top message:

ma_op_t op;

is

ma_op_t * op;

it’s sample, but explanation you observation:

why boolean not support?

sorry i’am beginner in C dev, in package i’am not found documentation of C lang. (как могу так и пишу…)

11

That’s just the way that the ‘C’ language is defined!

So it might be an idea to do an introductory ‘C’ course first.
Or at least study a ‘C’ textbook, and practice on a PC.

Teaching the ‘C’ programming language is beyond the scope of a forum like this!

It does assume that you are already a competent ‘C’ programmer, I’m afraid.

Just like the user guide for a new car assumes that you can already drive! :wink:

12

My car is simple to drive :smiley:

return to the problem…

ma_op_t * op;

in init method:

op = (ma_op_t *)adl_memGet( sizeof ( ma_op_t ) );

in timer1:

if(op->connected != TRUE){
  // get vars and fill op
  op->connected = TRUE;
  op->opid = 22100;
  op->opname = tmp;
}

in timer2:

if(op->connected == TRUE){
  // check vars from op
  // op->opname return good value
  // op->opid return return bad value = -369082370, where my 22100?
}

where my 22100 in op->opid?

13

See colin-tfe’s warning above - are you still deallocating the memory before you check this value? :open_mouth:

14

Instead of

if(op->connected != TRUE)

use

if(!op->connected)

Likewise

if(op->connected == TRUE)

should be changed to

if(op->connected)

As for the rest… Can’t say much about it without more code…

15

Well, i dont really know what you want to achieve… but this is working code :

typedef struct { 
    bool                connected; 
    u32                 opid; 
    ascii *             opname; 
} ma_op_t;

ma_op_t * op;

char tempBuffer[200];

void init()
{ 
  op = adl_memGet( sizeof ( ma_op_t ) ); 
  op->connected=FALSE;
} 

void timerHandler1(u8 id)
{
	int i;
	ascii * tmp;
	if(!op->connected)
	{ 
		tmp = (ascii *) adl_memGet((u16) 20); 
		for(i=0;i<19;i++)
		{
			tmp[i] = 'A';
		}
		tmp[i]='\0';
		op->connected = TRUE; 
		op->opid = 22100;

		// here, instead of op->opname = tmp
		op->opname = adl_memGet(strlen(tmp));
		strcpy(op->opname, tmp);
      adl_memRelease(tmp);
	}
}

void timerHandler2(u8 id)
{
	if(op->connected)
	{
		memset(tempBuffer, '\0', 200);
		sprintf(tempBuffer, "timerHandler2() : opid = %lu, opname :", op->opid);
		memcpy(&tempBuffer[strlen(tempBuffer)], op->opname, strlen(op->opname));
		adl_atSendResponse(ADL_AT_UNS, tempBuffer);
		TRACE((1, tempBuffer));
	}
}


void mainFunction ()
{
	init();
	adl_tmrSubscribe(FALSE, 20, ADL_TMR_TYPE_100MS, timerHandler1);
	adl_tmrSubscribe(FALSE, 30, ADL_TMR_TYPE_100MS, timerHandler2);
}

Hope it helps… if not, explain exactly what you want to do and copy-paste your code !

16

Sorry colin-tfe

// here, instead of op->opname = tmp 
      op->opname = adl_memGet(strlen(tmp)); 
      strcpy(op->opname, tmp);

won’t work right — it will be better to write

// here, instead of op->opname = tmp 
      op->opname = adl_memGet(strlen(tmp) + 1); 
      strcpy(op->opname, tmp);

because strcpy is adding an null-byte after the copied string …

Regards
Ralf

17

well, that’s true… strcpy copy all char including \0, but strlen doesnt count \0 char… did it too quickly… :blush:

thanks for the correction !