compiling c code: error: lvalue required as left operand of assignment

Question

When I try to compile some code on gcc 4.4.5, gcc runs into an error on this line:

ORG(e) = DEST(a);

These 2 macros are defined as:

#define ORG(e) ((site_struct *) ODATA(e))
#define DEST(e) ((site_struct *) DDATA(e))

I did not have a problem compiling this on solaris with gcc 3.4.5.

I've been trying to figure out why it won't compile for quite a while but to no avail. Can anyone point me in the right direction?

---

From a comment:

ODATA and DDATA are defined as:

#define ODATA(e) ((edge_struct *)((e)&0xfffffffcu))->data[(e)&3]
#define DDATA(e) ((edge_struct *)((e)&0xfffffffcu))->data[((e)+2)&3]

Answer 1

I find it hard to believe you could compile it on Solaris. The left-hand side of your assignment is a result of a cast. The results of casts in C language are always rvalues. You can't assign anything to an rvalue. It simple makes no sense. Rvalues are not objects. They are not stored in memory, which is why trying to assign anything to an rvalue makes no sense. This is a very fundamental fact of C, which is why I can't believe you could compile it with any C compiler.

For example, this code

int i;
(int) i = 5; /* ERROR */

does not compile for the very same reason your code does not compile.

It is hard to "point you in the right direction" because it is totally not clear what you were trying to do. What is that assignment supposed to mean? Why do you want to have casts on both sides of the assignment?

Answer 2

Your ODATA() and DDATA() macros evaluate to lvalues (the cast in those macros is to a pointer that's dereferenced to obtain the lvalue). So what needs figuring out is what the original ORG() and DEST() macros do:

#define ORG(e) ((site_struct *) ODATA(e))
#define DEST(e) ((site_struct *) DDATA(e))

Basically they take the lvalues produced by the ODATA() and DDATA() macros and treat them as pointers. GCC 3.4.5's cast-as-lvalue language extension, which was deprecated in 3.4.5 and removed in 4.0, allows the result of a cast to be used as an lvalue if the operand to the cast is an lvalue. We can emulate that by performing our own pointer/address manipulation and dereferencing. Change the ORG() and DEST() macros to take the address of the lvalues produced by the ODATA() and DDATA() macros, then dereference that address as a pointer to the desired type. This should produce equivalent results in GCC 4.x as you were getting in GCC 3.4.5 with your existing macros:

#define ORG(e) (*((site_struct **) &(ODATA(e))))
#define DEST(e) (*((site_struct **) &(DDATA(e))))

I think that should produce the same behavior that the original code had in GCC 3.4.5 - presumably that's the correct behavior for this application.

A couple of notes:

• as other answers have mentioned - these macros are a mess, and should be refactored to something more maintainable at the earliest opportunity. But sometimes you just gotta be pragmatic and get a port working.

• I'd make a minor modification to the ODATA() and DDATA() macros by enclosing them in parens. I'm not sure it necessary since the -> and [] operators have a very high precedence, but I get paranoid about expression macros that aren't fully parenthesized:

  #define ODATA(e) (((edge_struct *)((e)&0xfffffffcu))->data[(e)&3])
  #define DDATA(e) (((edge_struct *)((e)&0xfffffffcu))->data[((e)+2)&3])
  

Answer 3

The result of a cast is an rvalue, not an lvalue. IOW, the result is just the "raw" value, not the original object that was the source of the value.

It's not entirely apparent how to fix things in your case. In particular, without knowing what the ODATA(e) and DDATA(e) are/do, it's hard to guess how to proceed from there.

If you're trying to assign one struct to another, you can do something like:

*(site_struct *)a = *(site_struct *)b;

Note that even though we have casts on both sides of the assignment, we're not trying to assign to the result of the cast -- rather, we're casting a pointer to the correct type, and then dereferencing that pointer, and assigning to the object that pointer refers to.

Edit: okay, after the macros are expanded, we end up with something like this:

((site_struct *)((edge_struct *)((e)&0xfffffffcu))->data[(e)&3]) =
((site_struct *)((edge_struct *)((a)&0xfffffffcu))->data[((a)+2)&3]);

Based on the sheer ugliness of that, I think I'd try to back up at least a couple levels of abstraction and try to figure out the intent of all this. This is pretty close to write-only coding. Although you've provided the definitions of ODATA and DDATA as requested, it looks like figuring this out will take more than that -- you'll need to look closely at the definition of edge_struct, and possibly site_struct as well. You'll almost have to figure out the meaning of the subscripts into the data member of the edge_struct.

To make a long story short, this looks like a fairly serious case of reverse engineering, and it's almost impossible to predict how much code will need to be examined and understood before you can rewrite/update it to the point that you'll get the right result from a modern compiler.