unsequenced modification and access to pointer

Question

I'm getting a warning for this C expression:

*p0++ = mult(*p0, psign[i1]); 

The warning is:

unsequenced modification and access to 'p0' [-Wunsequenced]

I think the expression should be modified to this:

*p0 = mult(*p0, psign[i1]);
p0++;

Is the behavior (after modification) going to be as intended? I would think that the pointer increment should happen after the value pointed to by p0 has been updated.

Roark

Does the warning go away? – Incandescent

The warning goes away, but I am not sure the behavior is the same. So I was looking for a confirmation on that – Roark

What behavior do you want? The warning was because the behavior you're going to get might not be what you want (it's undefined), where the change you propose makes it pretty clear that p0 will be the return of mult() and then incremented. – Sideslip

@mah: I am not sure because this is code that I didn't write. It's G.729 source code. – Roark

@Roark I believe it's safe to make the change since the alternative, leaving the code as it is, yields undefined behavior. That said, you should certainly test well after such a change if its impact is important to you. – Sideslip

Is mult() a macro here? – Paphian

No - it's a function. The source is here --> g729.googlecode.com/svn/trunk/acelp_ca.c – Roark

@user1884325; I think there is a serious bug in the code you have posted. Several lines in that code have the same problem that I mentioned in my answer. It need to be fixed. – Passionless

@haccks: Yes, there are like 6-8 cases.. – Roark

Ridiculous code. Why would anyone write garbage like that? – Kassala

Answer 1

The snippet you have provided above invokes undefined behavior. According to C standard

C11: 6.5 Expressions:

If a side effect on a scalar object is unsequenced relative to either a different side effect on the same scalar object or a value computation using the value of the same scalar object, the behavior is undefined. If there are multiple allowable orderings of the subexpressions of an expression, the behavior is undefined if such an unsequenced side effect occurs in any of the orderings.⁸⁴⁾.

In the expression *p0++ = mult(*p0, psign[i1]), the modification to p0 on left side of the assignment operator is not sequenced before or after the use of p0 on right hand side of the expression. Therefore, the snippet

*p0++ = mult(*p0, psign[i1]);   

is not equivalent to

*p0 = mult(*p0, psign[i1]);
p0++;                       // Side effect to p0 is guaranteed after the use  
                            // of p0 in mult function

Passionless

In layman's terms, you know that you need to read *p0 in order to form the input parameter to mult(), and the problem is it is not defined when that address will be read: before the incrementation, or after. Compile the same code using different compilers (even different versions of the same compiler) and it's possible you'll get different results. – Sideslip

but most likely what the author intended was to multiply 2 numbers using the value pointed to by p0 and after the result has been stored in the memory location which p0 points to, then as a final operation increment the pointer by 1?? – Roark

@Roark that's how it seems to me – Sideslip

@user1884325; Which part of the answer is not clear to you? I think you know what is meant by "unsequenced" in this context. – Passionless

@user1884325: In layman's terms: the old code was 'undefined behaviour' and you can't predict what would actually happen. The proposed revised code has fully defined behaviour (or, at the least, the use of *p0 is clean, and the modification of p0 is also clean), so it is an improvement over the original. Since the original code had undefined behaviour, you won't be able to write code 100% compatible with it. You can, however, write code that does what the author probably intended, and that's what the proposed rewrite does. So, assuming the warning is no longer emitted, you're good to go. – Tiphany

@Passionless : Yes, I know what the word sequence means. It's a word that can be used in many different contexts and context may (or may not) have an impact on how things should be interpreted in the individual case. – Roark

Thank you everybody for the fast responses :-) Appreciate it! – Roark

@user1884325: The standard is quite clear about the meaning. – Cress