invalid lvalue in increment error at pointer

Question

after changing GCC version 3.0 to 4.1 i am getting invalid lvalue in increment error

#include <vscreen.h>
#include "vscreen_internal.h"

extern UDINT colPalette[256];

void  memset_f(void *p,USINT value, UDINT len)
{
    register    UDINT longValue = colPalette[value];

    while(len)
    {
        if ( ((UDINT)p&3) == 0 )   /* even address*/
        {
            if (len > 32) /*and more than 32 bytes to fill */
            {
                *((UDINT*)p)++ = longValue;        /* lvalue in increment error*/
                *((UDINT*)p)++ = longValue;        /* lvalue in increment error*/
                *((UDINT*)p)++ = longValue;        /* lvalue in increment error*/
                *((UDINT*)p)++ = longValue;        /* lvalue in increment error*/
                *((UDINT*)p)++ = longValue;        /* lvalue in increment error*/
                *((UDINT*)p)++ = longValue;        /* lvalue in increment error*/
                *((UDINT*)p)++ = longValue;        /* lvalue in increment error*/
                *((UDINT*)p)++ = longValue;        /* lvalue in increment error*/
                len-=32;
                continue;
            }
        }   
        *(USINT*)p++ = (USINT)longValue;         /* lvalue in increment error*/
         len--;
    }
}

#endif

Perhaps do the cast `((UDINT*)p)` once to a local variable at the beginning for readability — Ôrel, Jun 03 '20 at 07:21
i did ((UDINT*)p) as well as *((UDINT*)p++) = longValue after that i didn't get any error but meaning of statement goes change because of that i don't get proper result. — Rohit Patil, Jun 03 '20 at 07:34
You need to dereference the result of the `++` operation, not try and assign to it. — tadman, Jun 03 '20 at 07:36
_"I did ((UDINT*)p) as well as ((UDINT)p++) = longValue"_ @kaylum suggestion was `*((UDINT*)p++) = longValue;` meaning: (1) cast p to`UDINT*`, (2) dereference it and store `longvalue` to that specific memory location, (3) increase address contained in `p` by `sizeof int`. — Roberto Caboni, Jun 03 '20 at 07:53

score 0 · Answer 1 · answered Jun 03 '20 at 10:06

The result of a cast is not a "lvalue" so you cannot increment that one. C has been like that since the dawn of time, so your previous code must have relied on some fishy non-standard extension. How about using the right type in a temporary pointer (like uint32_t*) and simply do *ptr++ = ... instead?

Overall, this code is in pretty bad shape and would need to be rewritten from scratch, then get carefully reviewed and disassembled after that. I'm not quite sure of the purpose, optimize to 32 byte chunks somehow? Because this would make cache happier? I'm not convinced that the code is all that effective (any longer).

You need to ditch home-brewed integer types, get rid of needlessly complex *((UDINT*)p)++, get rid of stone age register, replace if(this) if(that) { ...continue; } else with if(that && that) {} else and so on. Manual loop unrolling is pretty old stuff, nowadays its most often a pre-mature optimization.

Also notably, *((UDINT*)p) is certainly a strict aliasing violation, so you must compile this with specialized compiler options such as -fno-strict-aliasing. I'd expect a comment about such in the source code to demonstrate that the programmer considered it.

invalid lvalue in increment error at pointer

1 Answers1