Re: [ft-devel] Re: meddlesome compiler warns against "for( ...; ...;...) ; "

[mpsuzuki]

Dear Werner,

I'm too stupid to close this issue, I'm quite sorry for
keeping this thread. I don't have good sample font to
test the logic is right /or not, so I want to be sure
about the original intention of the author.

Here I quote the current code in src/truetype/ttgxvar.c.

   907      {
   908        for ( i = 0;
   909              i < num_coords && blend->normalizedcoords[i] == coords[i];
   910              ++i );
   911          if ( i == num_coords )
   912            manageCvt = mcvt_retain;
   913          else
   914            manageCvt = mcvt_load;
   915  
   916        /* If we don't change the blend coords then we don't need to do  
*/
   917        /* anything to the cvt table.  It will be correct.  Otherwise we 
*/
   918        /* no longer have the original cvt (it was modified when we set  
*/
   919        /* the blend last time), so we must reload and then modify it.   
*/
   920      }


I think, your comment proposes to remove the ";" at the end
of line 910. It changes the current logic to another one.
The changed logic would be:

        for ( i = 0;
              i < num_coords && blend->normalizedcoords[i] == coords[i];
              ++i )
        {
          if ( i == num_coords )
            manageCvt = mcvt_retain;
          else
            manageCvt = mcvt_load;
        }

But, "if ( i == num_coords ) manageCvt = mcvt_retain; " would
not be executed, because the "for" loop already restricts "i"
as "i < num_coords". Therefore, the executed procedure would be
simplified aslike:

        for ( i = 0;
              i < num_coords && blend->normalizedcoords[i] == coords[i];
              ++i )
        {
          manageCvt = mcvt_load;
        }

During the loop, "manageCvt" is not used at all, and, the variable
"i" is not used in following part of the function, so, it would be
further simplified aslike:

        manageCvt = mcvt_load;

It looks like very strange. So, I guessed the original intention
by the author was

        for ( i = 0;
              i < num_coords && blend->normalizedcoords[i] == coords[i];
              ++i ) { }
        
        if ( i == num_coords )
          manageCvt = mcvt_retain;
        else
          manageCvt = mcvt_load;

Using "{ }" instead of ";" looks ugly, so I divided the looping
restriction into giving max of "i"  and "coods[i]" comparison,
aslike:

        for ( i = 0; i < num_coords; ++i )
          if ( blend->normalizedcoords[i] != coords[i] )
            break; 
        
        if ( i == num_coords )
          manageCvt = mcvt_retain;
        else
          manageCvt = mcvt_load;

Werner, my proposal in above is right?
Or, simple removing ";" from the end of "for" loop is right?

Regards,
mpsuzuki


On Thu, 09 Oct 2008 20:13:18 +0200 (CEST)
Werner LEMBERG <address@hidden> wrote:

>
>> However, the logic is correct, is it not? It looks as if the code is
>> checking whether all of the items in the array fulfil a condition.
>> So it should be fixed as originally suggested by mpsuzuki, without
>> changing the logic, but preventing the warning.
>
>I mean that the `;' in the same line as the `for' command is a
>mistake.  Sorry for being unclear.
>
>
>    Werner