• State Not Answered
  • Locked Locked
  • Replies 4 replies
  • Subscribers 21 subscribers
  • Views 1264 views
  • Users 0 members are here
2025 survey

We are running a survey to help us improve the experience for all of our members. If you see the survey appear, please take the time to tell us about your experience if you can.

Options
Related
How was your experience today?
This discussion has been locked.
You can no longer post new replies to this discussion. If you have a question you can start a new discussion

Definition misbehaves?

Former Member
Former Member

The definition 'Parameter_Table' has a starange behavior in my code.
Is there an error in this code or a bug in C51?

In cases 0x20 and 0x21 this code returns quite different (wrong) values compared to cases 0x10 and 0x11 ( or 0x30 and 0x31 ) respectively.
Instead of Parameter_Table(PARAMETER_NUMBER-x) it returns something like 'Parameter_Table(-x)+PARAMETER_NUMBER';

#define	PARAMETER_SIZE	13
#define	PARAMETER_NUMBER	8

union parameter_type {
	struct {
		unsigned char	start_1;
		unsigned char	params_1[PARAMETER_SIZE];
		unsigned int	check_1;
	} parst;
	unsigned char	table[PARAMETER_NUMBER][PARAMETER_SIZE+3];
};

#define Parameter_Start		(0x1000)
#define Parameter_Table(n)	(Parameter_Start+n*(PARAMETER_SIZE+3))
#define Parameter_Check		(Parameter_Start+sizeof(union parameter_type))


unsigned int test_defines (unsigned char tmp) {
//
	unsigned int	tmpw;

	switch (tmp) {
	case 0:
		tmpw = Parameter_Start;
		break;
	case 0x10:
		tmpw = Parameter_Table(0);
		break;
	case 0x11:
		tmpw = Parameter_Table(1);
		break;
	case 0x20:
		tmpw = Parameter_Table(PARAMETER_NUMBER-8);
		break;
	case 0x21:
		tmpw = Parameter_Table(PARAMETER_NUMBER-7);
		break;
	case 0x27:
		tmpw = Parameter_Table(PARAMETER_NUMBER-1);
		break;
	case 0x30:
		tmp = PARAMETER_NUMBER-8;
		tmpw = Parameter_Table(tmp);
		break;
	case 0x31:
		tmp = PARAMETER_NUMBER-7;
		tmpw = Parameter_Table(tmp);
		break;
	}
	return tmpw;
}

Here is a part of the listing of the code.
    37:         case 0x10:
    38:                 tmpw = Parameter_Table(0);
C:0x0DD2    7C10     MOV      R4,#koe(0x10)
C:0x0DD4    7D00     MOV      R5,#B?SMEM(0x00)
    39:                 break;
C:0x0DD6    8024     SJMP     C:0DFC
    40:         case 0x11:
    41:                 tmpw = Parameter_Table(1);
C:0x0DD8    7410     MOV      A,#koe(0x10)
C:0x0DDA    FC       MOV      R4,A
C:0x0DDB    FD       MOV      R5,A
    42:                 break;
C:0x0DDC    801E     SJMP     C:0DFC
    43:         case 0x20:
    44:                 tmpw = Parameter_Table(PARAMETER_NUMBER-8);
C:0x0DDE    7C0F     MOV      R4,#0x0F
C:0x0DE0    7D88     MOV      R5,#TCON(0x88)
    45:                 break;
C:0x0DE2    8018     SJMP     C:0DFC
    46:         case 0x21:
    47:                 tmpw = Parameter_Table(PARAMETER_NUMBER-7);
C:0x0DE4    7C0F     MOV      R4,#0x0F
C:0x0DE6    7D98     MOV      R5,#SCON0(0x98)
    48:                 break;
C:0x0DE8    8012     SJMP     C:0DFC
    49:         case 0x27:
    50:                 tmpw = Parameter_Table(PARAMETER_NUMBER-1);
C:0x0DEA    7C0F     MOV      R4,#0x0F
C:0x0DEC    7D98     MOV      R5,#SCON0(0xF8)
    51:                 break;
C:0x0DEE    8012     SJMP     C:0DFC

  • 0

    Take a look at the preprocessor output file - I'd guess you might have problems with the macro expansions, possibly due to insufficient parentheses.

    Stefan

  • 0 

    #define Parameter_Table(n) Parameter_Start+n*(PARAMETER_SIZE+3))

    This is missing a pair of parentheses around the n. Make it

    #define Parameter_Table(n)	(Parameter_Start+(n)*(PARAMETER_SIZE+3))

    and it'll work. Note that this is a general C preprocessor macro issue, not particular to Keil C51: always enclose any reference to a macro argument in the body by some kind of parentheses.

  • 0

    There's no need for the macros at all; they're getting in the way and causing bugs. Let the compiler handle it.

    #define ConfigBaseAddress 0x1000

typedef ParamCheckType ???; // not mentioned in original code posted

typedef struct
   {
   U8 start;
   U8 value[PARAMETER_SIZE];
   U16 check;
   } Parameter;

typedef struct
   {
   Parameter   params[PARAMETER_NUMBER];
   ParamCheckType   check;
   } ParameterBlock;

ParameterBlock config _at_ ConfigBaseAddress ;

    Then, the assignments become:

    tmpw = &config.params[n];

    and there's no way to make a mistake on the base address, or the size, or the fact that check comes just past the end of the parameter table, or change one of the macros and not another when you update a struct or the number or size of parameters.

    I'm not certain why you might want to union the structured table with an array of char. Rather than add an extra level of naming for the union, it's probably easier to cast a pointer to the table (or use its base address) when you want to memcpy() or checksum it. Those uses are probably rare compared to the usual access to the table.

    config.check = Checksum((U8*)&config);

    I'm not sure why each parameter has a checksum as well as the whole table, but that seems to be the intent of the original code, so I've kept that layout.

    There are a number of threads on how to locate an initialized variable. C51 does not allow initializers with _at_.

  • Former Member
    0 Former Member in reply to Drew Davis

    Thank all of you for the help.

    Actually this is a slightly simplified part of a handler for serial EEPROM in an old project.
    That's why the parameters are in blocks.

    The more recent version of the project uses the memory class HDATA, the XBANKING.A51-like interface to an EEPROM-handler (C51).