error: lvalue required as left operand of assignment in Macro

Question

I have downloaded the source code for the open-source gt package on fedora and am attempting to compile it on Debian. During compilation the following error in the title showed up for this piece of code in instrum.c:

#define READ_CHAR(thing) \
      thing = tmp[tmpindex++];

The full error report is as follows:

instrum.c: In function ‘load_instrument’:
instrum.c:944:13: error: lvalue required as left operand of assignment
  944 |       thing = tmp[tmpindex++];
      |             ^
instrum.c:1173:7: note: in expansion of macro ‘READ_CHAR’
 1173 |       READ_CHAR ((int8)sp->aps_parameter);
      |       ^~~~~~~~~
make[1]: *** [Makefile:335: instrum.o] Error 1
make[1]: Leaving directory '/home/classic/gt-0.4/src'
make: *** [Makefile:235: all] Error 2

the struct that sp is defined is as follows:

typedef struct {
  splen_t
    loop_start, loop_end, data_length;
  ...
  ...
  int32
    freq_scale, vibrato_delay;
  int8
    aps_parameter, sw_up, sw_down, sw_lokey, sw_hikey, sw_last;
} Sample;

Don't use a semicolon as the last character in a macro; it can lead to confusion. It may not be related to your problem, though. — Jonathan Leffler, May 25 '23 at 18:02
The result of a cast is not an l-value; you can't assign to it. So, the `(int8)` cast means you cannot assign to that. Why do you think the cast is remotely necessary? Now that you show the structure (which isn't [minimal](https://stackoverflow.com/mcve)!), `aps_parameter` is already an `int8` so the cast is totally superfluous — as well as erroneous! Use `READ_CHAR(sp->aps_parameter);` (and remove the semicolon so you don't get confused by the errors when you write `if (x == y) READ_CHAR(sp->aps_parameter); else puts("Busted!");` — Jonathan Leffler, May 25 '23 at 18:03
@WeatherVane yes that fixed it, must be some modern gcc quirk, as this code is from 2004. — Cheetaiean, May 25 '23 at 18:06
It's not a 'modern quirk'. Perhaps the code wasn't transcribed correctly, or didn't even compile originally. — Weather Vane, May 25 '23 at 18:07
The link in my upvoted comment is incorrect — the one in my answer is correct. — Jonathan Leffler, May 25 '23 at 18:28

Jonathan Leffler · Accepted Answer · 2023-05-25T18:28:02.720

The result of a cast is not an l-value; you can't assign to it. So, the (int8) cast means you cannot assign to that. Why do you think the cast is remotely necessary? Now that you show the structure (which isn't minimal!), aps_parameter is already an int8 so the cast is totally superfluous — as well as erroneous! Use:

READ_CHAR(sp->aps_parameter);

Don't use a semicolon as the last character in a macro; it can lead to confusion. It is not directly related to your problem, though. If you don't remove the semicolon, you will be confused by the errors when you write:

if (x == y)
    READ_CHAR(sp->aps_parameter);
else
    puts("Busted!");

See Why use apparently meaningless do-while and if-else statements in macros? if you don't immediately see why the trailing semicolon is bad. You can claim that "I would always use braces around the body of every if, else or loop statement — that would neutralize the bug, but it doesn't alter the fact that the trailing semicolon is a bug.

error: lvalue required as left operand of assignment in Macro

1 Answers1