tricky assignment statemenent

[Ike Naar]

Consider the following code:

#include <stdlib.h>

struct s
{
/* ... */
struct s * next;
};

void f(struct s * q)
{
struct s * p = malloc(sizeof *p);
if (p != 0)
{
p = p->next = q; /* is this safe? */
}
}

In the context given above, is the statement "p = p->next = q;"
safe and equivalent to "p->next = q; p = q;"
or does it invoke undefined behaviour because p is modified and
dereferenced at the same time?

Kind regards,
Ike

 

[Joona I Palaste]

Consider the following code:

#include <stdlib.h>

struct s
{
/* ... */
struct s * next;
};

void f(struct s * q)
{
struct s * p = malloc(sizeof *p);
if (p != 0)
{
p = p->next = q; /* is this safe? */
}
}

In the context given above, is the statement "p = p->next = q;"
safe and equivalent to "p->next = q; p = q;"
or does it invoke undefined behaviour because p is modified and
dereferenced at the same time?

I would consider it undefined behaviour, as there is no sequence point
between the operations "p=p->next" and "p->next=q" in the code.

 

[Ben Pfaff]

In the context given above, is the statement "p = p->next = q;"
safe and equivalent to "p->next = q; p = q;"
or does it invoke undefined behaviour because p is modified and
dereferenced at the same time?

We had a long, long thread about this years ago. I should know,
I started it You could probably find it with Google. For
what it's worth, the conclusion was that everyone disagreed about
the actual answer, but almost everyone concluded that it was
better to stay on the safe side by writing it as two statements.

 

[Andrey Tarasevich]

Consider the following code:

#include <stdlib.h>

struct s
{
/* ... */
struct s * next;
};

void f(struct s * q)
{
struct s * p = malloc(sizeof *p);
if (p != 0)
{
p = p->next = q; /* is this safe? */
}
}

In the context given above, is the statement "p = p->next = q;"
safe and equivalent to "p->next = q; p = q;"
or does it invoke undefined behaviour because p is modified and
dereferenced at the same time?
...

The real question here is whether the old value of 'p' is accessed for
the sole purpose of determining its new value. For example, assignment

p = p->next;

is perfectly legal even though "p is modified and dereferenced at the
same time".

I think it's pretty clear that assignment

p = p->next = q;

(which associates from right to left) does not satisfy this requirement.
Therefore, this assignment produces undefined behavior.

 

[Emmanuel Delahaye]

Ike Naar wrote on 31/12/04 :

struct s * p = malloc(sizeof *p);
if (p != 0)

Style preference:

if (p != NULL)

{
p = p->next = q; /* is this safe? */

Huh! Gurus know that. Simple programmers write simple things that show
clearly the intention:

p->next = q;
p = p->next;

(Well, I guess)

--
Emmanuel
The C-FAQ: http://www.eskimo.com/~scs/C-faq/faq.html
The C-library: http://www.dinkumware.com/refxc.html

"Mal nommer les choses c'est ajouter du malheur au
monde." -- Albert Camus.

 

[Chris Croughton]

We had a long, long thread about this years ago. I should know,
I started it You could probably find it with Google. For
what it's worth, the conclusion was that everyone disagreed about
the actual answer, but almost everyone concluded that it was
better to stay on the safe side by writing it as two statements.

If everyone disagrees, I'd call that "undefined" (as in the spec.
doesn't define it in an unambiguous way). It may, of course, then be a
defect in the specification which should be remedied at some point
(either to define the behaviour or to specifically state it as
'undefined').

It sounds like a job for comp.std.c.

Chris C

 

[Ben Pfaff]

If everyone disagrees, I'd call that "undefined" (as in the spec.
doesn't define it in an unambiguous way). It may, of course, then be a
defect in the specification which should be remedied at some point
(either to define the behaviour or to specifically state it as
'undefined').

It sounds like a job for comp.std.c.

We brought them into the picture and it didn't, if I recall
correctly, help much.

 

[Ike Naar]

: Ike Naar wrote on 31/12/04 :
:> struct s * p = malloc(sizeof *p);
:> if (p != 0)
: Style preference:
: if (p != NULL)

Not to mention "if (p)", "if( p!=0 )" or "if (NULL != p)" .
The discussion was not about style, though.

:> p = p->next = q; /* safe or UB ? */
: Huh! Gurus know that.

That's why I turned to this very group. To ask the gurus.

: Simple programmers write simple things that show
: clearly the intention:
: p->next = q;
: p = p->next;

And right they are. Do they also eat quiche ?

Kind regards,
Ike

 

[Ike Naar]

: [...] is the statement "p = p->next = q;"
: safe and equivalent to "p->next = q; p = q;"
: or does it invoke undefined behaviour because p is modified and
: dereferenced at the same time?

Thanks to all who responded. The tricky part appeared in existing code
and at first sight it looked a bit suspicious to me; your responses
have confirmed those suspicions.
That said, the code seems to compile and run correctly on every
platform/compiler combination I tried.

I would have written it as two statements, just to be on the safe side.

Thanks again,
Ike

 

[Lawrence Kirby]

....



I would consider it undefined behaviour, as there is no sequence point
between the operations "p=p->next" and "p->next=q" in the code.

However the standard says "Furthermore, the prior value shall be read only
to determine the value to be stored." In p = p->next = q the value
assigned to p is the result of the expression p->next = q. In the abstract
machine this expression must be evaluated fully in order to generate the
value to be assigned. The access to p in p->next is part of that
evaluation so it is very much being read in order to determine the new
value. Before you start formulting your objections to this consider
carefully the statements in the standard (5.1.2.3):

"The semantic descriptions in this International Standard describe the
behavior of an abstract machine in which issues of optimization are
irrelvant."

"In the abstract machine, all expressions are evaluated as specified by
the semantics."

Most counterarguments are based on the idea that you can determine the
value to be assigned to p without evaluating p->next explicitly. However
that is an argument that is based on optimization, not the application of
the semantics of assignment exactly as stated in the standard. The
standard explicitly forbids this line of reasoning.

Also consider that in the abstract machine operands must be evaluated
before the operation can be performed and a result generated.
Again, anything else would constitute an optimization and a departure
from stated semantics. (Noting short-circuit and conditional operators
whose stated semantics affect whether some operands are evaluated at all
depending on the value of another operand).

Lawrence

 

[Christian Bau]

However the standard says "Furthermore, the prior value shall be read only
to determine the value to be stored." In p = p->next = q the value
assigned to p is the result of the expression p->next = q. In the abstract
machine this expression must be evaluated fully in order to generate the
value to be assigned. The access to p in p->next is part of that
evaluation so it is very much being read in order to determine the new
value. Before you start formulting your objections to this consider
carefully the statements in the standard (5.1.2.3):

There are two assignments here. In one assignment, p is modified. The
prior value of p is arguably used to determine the new value stored into
p, as you explained, but it is _also_ read to determine at which memory
location the second assignment will happen, so it is not _only_ read to
determine the value stored, therefore undefined behavior.

 

[petermcmillan_uk]

: Ike Naar wrote on 31/12/04 :
:> struct s * p = malloc(sizeof *p);
:> if (p != 0)
: Style preference:
: if (p != NULL)

Not to mention "if (p)", "if( p!=0 )" or "if (NULL != p)" .
The discussion was not about style, though.

:> p = p->next = q; /* safe or UB ? */
: Huh! Gurus know that.

That's why I turned to this very group. To ask the gurus.

: Simple programmers write simple things that show
: clearly the intention:
: p->next = q;
: p = p->next;

And right they are. Do they also eat quiche ?

Kind regards,
Ike

I'm very interested in style, and personally I quite like :

if( p )

When p is a pointer I think it's clear what this means. What does
everybody else think about this?

 

[Old Wolf]

Consider the following code:

#include <stdlib.h>

struct s
{
/* ... */
struct s * next;
};

void f(struct s * q)
{
struct s * p = malloc(sizeof *p);
if (p != 0)
{
p = p->next = q; /* is this safe? */
}
}

In the context given above, is the statement "p = p->next = q;"
safe and equivalent to "p->next = q; p = q;"
or does it invoke undefined behaviour because p is modified and
dereferenced at the same time?

There's no sequence point; so it could be done in the order:
p = q;
p->next = q;
which doesn't give the result you intended. So the only
question is whether the behaviour is undefined, or
unspecified.
IMHO it's undefined: 'p->next = q' involves a read of p
which is NOT for the purpose of determining the value
to be written in 'p = q'.

 

[pete]

if( p )

When p is a pointer I think it's clear what this means. What does
everybody else think about this?

I know that p is a pointer,
only because you told me so in text.

if (k != NULL)

gives you a good clue that k is a pointer

 

[Tim Rentsch]

There are two assignments here. In one assignment, p is modified. The
prior value of p is arguably used to determine the new value stored into
p, as you explained, but it is _also_ read to determine at which memory
location the second assignment will happen, so it is not _only_ read to
determine the value stored, therefore undefined behavior.

Please consult Annex D. The formal model makes it clear (even if not
immediately obvious) that the semantics here are well defined.

Granted, Annex D is informative rather than normative. But the
question of whether "the prior value is read only to determine the
value to be stored" is at best open to debate. Even though the formal
model is informative and the statement about prior value is normative,
a clear statement from the formal model is a better indicator (at
least, IMO) than an unclear statement in informal prose.

 

[lawrence.jones]

Please consult Annex D. The formal model makes it clear (even if not
immediately obvious) that the semantics here are well defined.

There is no formal model in the published standard -- it was a work in
progress that was considered too preliminary to publish, even
informatively. It does provide some guidance as to how some fraction of
the committee (which may be as small as the author and is certainly
smaller than the entire committee since there were people who objected
to it) thinks sequence points should work (if you can understand it at
all, which is not trivial), but it should not be considered in any way
definitive.

-Larry Jones

At times like these, all Mom can think of is how long she was in
labor with me. -- Calvin

 

[Tim Rentsch]

There is no formal model in the published standard -- it was a work in
progress that was considered too preliminary to publish, even
informatively. It does provide some guidance as to how some fraction of
the committee (which may be as small as the author and is certainly
smaller than the entire committee since there were people who objected
to it) thinks sequence points should work (if you can understand it at
all, which is not trivial), but it should not be considered in any way
definitive.

A nice response posting. Informative; thoughtfully worded.

However, I don't think the comments above take away from the basic
conclusion. Even though the formal model was preliminary and is not
definitive, in the absence of any specific evidence to the contrary
it's more likely to provide an accurate interpretation for what the
standard intends than guessing what was intended based on one or two
sentences of informal prose.

One point seems fairly widely held: the language in the standard that
addresses this question is not as clear or as unambiguous as it should
be. So if I understand the differences between the two newsgroups,
it's appropriate at this point to bring up that topic in comp.std.c.

 

[S.Tobias]

Please consult Annex D.

May I ask which "Annex D" you're talkig about?
In C99 Annex D is "Universal character names for identifiers" and
in C89 - "Library summary".

 

[lawrence.jones]

A nice response posting. Informative; thoughtfully worded.

Sorry, I'll try not to let it happen again. ;-)

However, I don't think the comments above take away from the basic
conclusion. Even though the formal model was preliminary and is not
definitive, in the absence of any specific evidence to the contrary
it's more likely to provide an accurate interpretation for what the
standard intends than guessing what was intended based on one or two
sentences of informal prose.

I don't think that's a valid conclusion. As I said (but perhaps not
explicitly enough), some members of the committee vehemently disagreed
with some parts of the proposed model, so I don't think it is any more
accurate than guessing based on the prose.

-Larry Jones

It's not denial. I'm just very selective about the reality I accept.
-- Calvin

 

[Lawrence Kirby]

....


There are two assignments here. In one assignment, p is modified. The
prior value of p is arguably used to determine the new value stored into
p, as you explained, but it is _also_ read to determine at which memory
location the second assignment will happen, so it is not _only_ read to
determine the value stored, therefore undefined behavior.

OK, the issue here is the meaning of "only". It is in there to (un)cover
cases like this:

j = (i = 2*i) + i;

Here i is accessed in the expression in a way that is used to
determine the value to be stored by the assignment to i, it is also
accessed in a way that is not. As such it violates the "only" requirement.

In p=p->next=q it is *only* accessed in the expression that calculates the
new value to be assigned. The inner assignment is part of that evaluation,
the fact that it has another side-effect on a distinct object is neither
here nor there, why should it be relevant?

Lawrence