3

I am working on a C program that uses a Union. The union definition is in FILE_A header file and looks like this...

// FILE_A.h****************************************************
xdata union  
{
long position;
char bytes[4];
}CurrentPosition;

If I set the value of CurrentPosition.position in FILE_A.c and then call a function in FILE_B.c that uses the union, the data in the union is back to Zero. This is demonstrated below.

// FILE_A.c****************************************************
int main.c(void)
{
    CurrentPosition.position = 12345;
    SomeFunctionInFileB();
}

// FILE_B.c****************************************************
void SomeFunctionInFileB(void)
{
    // After the following lines execute I see all zeros in the flash memory.
    WriteByteToFlash(CurrentPosition.bytes[0];
    WriteByteToFlash(CurrentPosition.bytes[1];
    WriteByteToFlash(CurrentPosition.bytes[2];
    WriteByteToFlash(CurrentPosition.bytes[3];
}

Now, If I pass a long to SomeFunctionInFileB(long temp) and then store it into CurrentPosition.bytes within that function, and finally call WriteBytesToFlash(CurrentPosition.bytes[n]... it works just fine.

It appears as though the CurrentPosition Union is not global. So I tried changing the union definition in the header file to include the extern keyword like this...

extern xdata union  
{
long position;
char bytes[4];
}CurrentPosition;

and then putting this in the source (.c) file...

xdata union  
{
    long position;
    char bytes[4];
}CurrentPosition;

but this causes a compile error that says:

C:\SiLabs\Optec Programs\AgosRot\MotionControl.c:76: error 91: extern definition for 'CurrentPosition' mismatches with declaration. C:\SiLabs\Optec Programs\AgosRot\/MotionControl.h:48: error 177: previously defined here

So what am I doing wrong? How do I make the union global?

timrau
  • 22,578
  • 4
  • 51
  • 64
PICyourBrain
  • 9,976
  • 26
  • 91
  • 136
  • I think - though am not sure - that the way union works is that if you access the data through one of its variables then the others are effectively invalidated. In other words, I do not believe you can set the long and then use the bytes to access that memory. You would have to access it through the long. I'm not positive about that though - hence the comment. I'm curious to see what those more experienced with unions say. – Daniel Bingham May 04 '10 at 19:52
  • 1
    @Daniel - incorrect - exactly the opposite is true, it was the original purpose of a union to get access to the bytes of long or an int or a float - dangerous and machine dependent they are..... – KevinDTimm May 04 '10 at 19:55
  • 5
    I swear I read the title as 'Trouble with Unicorns in C program'. I'm spending too much time on Meta. – George Stocker May 04 '10 at 19:57
  • 1
    @George - Unicorns are ALWAYS trouble in C programs – KevinDTimm May 04 '10 at 20:00
  • 1
    This really isn't about unions at all - it's about scoping and declaration vs. definition of a variable. Re-title the question? Re-tag it? – Stephen P May 04 '10 at 20:01
  • 1
    It's not about unions at all, but unions are for both things: conserving precious bits, and accessing one field from a field of a different type. Both are sometimes useful, but usually fraught with dangers best avoided. – WhirlWind May 04 '10 at 20:04
  • 1
    @George - I like that you thought this post was about Unicorns and still decided to click on it! – PICyourBrain May 04 '10 at 20:05
  • @KevinDTimm Thanks for the clarification. Been a long time since I've had to deal with unions. That thought was bubbling up out of deep memory. Not surprised to learn it was wrong ;) – Daniel Bingham May 04 '10 at 21:35
  • Just a tip. `long` is not guaranteed to be 4 bytes. I would import `stdint.h` and use `int32_t` instead. – Cole Tobin Aug 15 '12 at 00:01

4 Answers4

7

Is FILE_A.h really MotionControl.h? If so I think the fix is to define a union type in the header:

typedef
union xdata
{
    long position;
    char bytes[4];
} xdata;

And declare a global variable of that type elsewhere in a header file (maybe the same one):

extern xdata CurrentPosition;   // in a header file

Finally define the global variable in a C file exactly once. Maybe in file_a.c:

xdata CurrentPosition;

Of course a better fix might be to pass the xdata variable you want to write out to flash to SomeFunctionInFileB() so you don't have to depend on a global variable, which are well known to be problematic when not very, very carefully used. And there seems to be no good reason to not pass the data as a parameter:

// in a header file
void SomeFunctionInFileB( xdata const* pPosition);


void SomeFunctionInFileB( xdata const* pPosition)
{
    // After the following lines execute I see all zeros in the flash memory.
    WriteByteToFlash(pPosition->bytes[0];
    WriteByteToFlash(pPosition->bytes[1];
    WriteByteToFlash(pPosition->bytes[2];
    WriteByteToFlash(pPosition->bytes[3];
}

And call it like so:

int main.c(void)
{
    CurrentPosition.position = 12345;
    SomeFunctionInFileB( &CurrentPosition);
}
Michael Burr
  • 333,147
  • 50
  • 533
  • 760
  • In the definition of the union, why do you put 'xdata' in two places? Doesn't this make the name of the union 'xdata'? – PICyourBrain May 04 '10 at 20:10
  • My intent was to use the xdata keyword to tell the compiler to put the variable in the slower external memory. This program is for an embedded device. – PICyourBrain May 04 '10 at 20:16
  • Oh - I thought that `xdata` was intended to be the name of the union type. If `xdata` is a keyword/attribute for the compiler to put data in specific place, adjust accordingly (give the union type an appropriate name, and put the `xdata` attribute on the variable). – Michael Burr May 04 '10 at 20:21
  • @JordanS: the `xdata` name is used twice in the union typedef to give it a slightly easier name tio use (as mentioned in the previous comment, I wasn't aware that `xdata` wasn't supposed to be the name of the union type). See the following for details: http://stackoverflow.com/questions/252780/why-should-we-typedef-a-struct-so-often-in-c – Michael Burr May 04 '10 at 20:24
2

Ideally you need a typedef for the union and an extern declaration in FILE_A.h and the actual definition of the union in FILE_A.c.

-

// FILE_A.h

typedef union  
{
    long position;
    char bytes[4];
} Position;

extern Position CurrentPosition; // declaration

-

// FILE_A.c

#include "FILE_A.h"

Position CurrentPosition; // definition

int main(void)
{
    CurrentPosition.position = 12345;
    SomeFunctionInFileB();
    return 0;
}

-

// FILE_B.c

#include "FILE_A.h"

void SomeFunctionInFileB(void)
{
    // now there will be valid data in the flash memory.
    WriteByteToFlash(cp.bytes[0];
    WriteByteToFlash(cp.bytes[1];
    WriteByteToFlash(cp.bytes[2];
    WriteByteToFlash(cp.bytes[3];
}

-

Paul R
  • 208,748
  • 37
  • 389
  • 560
1

You haven't instantiated the union.
You need :

// FILE_A.c****************************************************

#include "File_a.h"
CurrentPosition cp;
int main(void)
{
    cp.position = 12345;
    SomeFunctionInFileB();
}

// FILE_B.c****************************************************
#include "File_a.h"
extern CurrentPosition cp;
void SomeFunctionInFileB(void)
{
    // now there will be valid data in the flash memory.
    WriteByteToFlash(cp.bytes[0];
    WriteByteToFlash(cp.bytes[1];
    WriteByteToFlash(cp.bytes[2];
    WriteByteToFlash(cp.bytes[3];
}
KevinDTimm
  • 14,226
  • 3
  • 42
  • 60
  • [whiny rant]bummer - I see I need to post my incomplete answers first, then flesh them out....[/whiny rant] – KevinDTimm May 04 '10 at 20:02
0

If sizeof(long) is not 4, then endianess comes into play...

consider

union{
   long position
   char bytes[sizeof long];
}
dmckee --- ex-moderator kitten
  • 98,632
  • 24
  • 142
  • 234