[comp.lang.c] Problem with post-incrementation

godin@ireq.hydro.qc.ca (Andre Godin 8926) (04/15/91)

	Every times I call this function, the value of 'str' 
	is always tmp_0 and 'i' never gets incremented. If
	we use the pre-increment operator, the function works 
	properly. Why post-incrementing doesn't work?

	(I'm using cc under SunOs 4.0.3.)

char *
fnc() /*
----- */
{
static	int	i = 0;
static	char	str[256];

	sprintf(str, "tmp_%d", i = (i++) % 3600);

	return (str);
}

jlg@cochiti.lanl.gov (Jim Giles) (04/16/91)

	sprintf(str, "tmp_%d", i = (i++) % 3600);

I have seen this bug in a number of C programs (including the _second_
edition of a published book on graphics in C).  It is one of the
drawbacks of side-effect operators that such errors are made and
remain undetected.

The problem is that i = (i++) etc.  assigns the value of the expression
to the variable i.  Depending on the order that the side-effects are
carried out, this may happen before or after the side-effect for the
incrementation operator.  If the assignment happens _after_ the increment,
then i will get zero every time.  What you want to do is this:

      sprintf(str, "tmp_%d", i = (i+1) % 3600);

You only need i to be assigned once anyway.

J. Giles

vegdahl@ogicse.ogi.edu (Steve Vegdahl) (04/16/91)

In article <6530@s3.ireq.hydro.qc.ca> godin@ireq.hydro.qc.ca (Andre Godin 8926) writes:
>
>	Every times I call this function, the value of 'str' 
>	is always tmp_0 and 'i' never gets incremented. If
>	we use the pre-increment operator, the function works 
>	properly. Why post-incrementing doesn't work?
>
>	(I'm using cc under SunOs 4.0.3.)
>
>char *
>fnc() /*
>----- */
>{
>static	int	i = 0;
>static	char	str[256];
>
>	sprintf(str, "tmp_%d", i = (i++) % 3600);
>
>	return (str);
>}

The problem is that the expression
  i = (i++) % 3600)
applies two side-effects to the variable i, without any intevening "sequence
points".  This results in "undefined behavior", according to the ANSI standard.
(Just be glad that it didn't erase your disk!)

Even disregarding ANSI, there are two natural interpretations of the above,
depending on the order chosen by the compiler for the application of the
side-effects to i:
  #1: evaluate "i % 3600"; assign %-result to i; increment i; yield %-result
      as value of expression
  #2: evaluate "i % 3600"; increment i; assign %-result to i; yield %-result
      as value of expression
In the second case, the value of i will remain at 0.

I think what you really want is
	sprintf(str, "tmp_%d", (i++) % 3600);
though this will still likely lead to unexpected behavior if/when i overflows.

		Steve Vegdahl
		Adaptive Solutions, Inc.

simonp@fulcrum.bt.co.uk (Simon Parsons) (04/17/91)

In article <6530@s3.ireq.hydro.qc.ca> godin@ireq.hydro.qc.ca (Andre Godin 8926) writes:
>
>	Every times I call this function, the value of 'str' 
>	is always tmp_0 and 'i' never gets incremented. If
>	we use the pre-increment operator, the function works 
>	properly. Why post-incrementing doesn't work?
>
>	(I'm using cc under SunOs 4.0.3.)
>
>char *
>fnc() /*
>----- */
>{
>static	int	i = 0;
>static	char	str[256];
>
>	sprintf(str, "tmp_%d", i = (i++) % 3600);
>
>	return (str);
>}

No suprises there the expression on the right-hand side of the assignment
is evaluated to zero, i is post-incremented to 1. Then the value of the
expression ( 0 ) is assigned to i. Hence i remains at zero for each call
of the function.

Simon


--
        "Hey girl, as I've always said, I prefer your lips red, 
         Not what the good lord made, but what he intended."
                                                         
          Simon Parsons, BT Fulcrum - simonp@fulcrum.bt.co.uk

rjohnson@shell.com (Roy Johnson) (04/17/91)

In article <20095@ogicse.ogi.edu> vegdahl@ogicse.ogi.edu (Steve Vegdahl) writes:
>>static	int	i = 0;
>>static	char	str[256];
>>
>>	sprintf(str, "tmp_%d", i = (i++) % 3600);

>The problem is that the expression
>  i = (i++) % 3600)
>applies two side-effects to the variable i, without any intevening "sequence
>points".  This results in "undefined behavior", according to the ANSI standard.
>(Just be glad that it didn't erase your disk!)
>I think what you really want is
	   sprintf(str, "tmp_%d", (i++) % 3600);
>though this will still likely lead to unexpected behavior if/when i overflows.

I think what he really wants is 
	   sprintf(str, "tmp_%d", i = (i+1) % 3600);
but only he can know for sure.

--
=============== !You!can't!get!here!from!there!rjohnson ===============
Feel free to correct me, but don't preface your correction with "BZZT!"
Roy Johnson, Shell Development Company

stetner@.bnr.ca (Douglas Stetner) (04/18/91)

In article <6530@s3.ireq.hydro.qc.ca> godin@ireq.hydro.qc.ca (Andre Godin 8926) writes:
>
>	Every times I call this function, the value of 'str' 
>	is always tmp_0 and 'i' never gets incremented. If
>	we use the pre-increment operator, the function works 
>	properly. Why post-incrementing doesn't work?
>
>	(I'm using cc under SunOs 4.0.3.)
>
>char *
>fnc() /*
>----- */
>{
>static	int	i = 0;
>static	char	str[256];
>
>	sprintf(str, "tmp_%d", i = (i++) % 3600);
>
>	return (str);
>}

Think about the way this will get compiled
i = (i++) % 3600
      |
      V
i = ( 0 ) % 3600   /* i has been incremented, i = 1 */
          |
	  V
i =       0        /* the assignment takes place and i = 0 */

Pre increment works because it evaluates this way...
i = (++i) % 3600
      |
      V
i = ( 1 ) % 3600   /* i has been incremented, i = 1 */
          |
	  V
i =       1        /* the assignment takes place and i = 1 */

It would be much better to either do the sprintf then twiddle i
or to twiddle i then do the sprintf, as this would be more readable
and will most likley make no difference to the size/speed of the
resulting executable.
not produce 'smaller|quick
--
--------------------------------------------------------------------------------
Douglas G. Stetner, 4S75  |  Bell-Northern Research  | My opinions are my own.
      stetner@bnr.ca      |  P.O. Box 3511, Stn. C   | (613) 828-6321  (home)
 ..!uunet!bnrgate!stetner |  Ottawa, ON, CANADA      | (613) 763-8396  (work)