[Bug 15397] New: gdi32: path.c fails to build with gcc 4.2, -Werror -O3

http://bugs.winehq.org/show_bug.cgi?id=15397

Summary: gdi32: path.c fails to build with gcc 4.2, -Werror -O3
           Product: Wine
           Version: 1.1.5
          Platform: PC
        OS/Version: Linux
            Status: NEW
          Keywords: source
          Severity: enhancement
          Priority: P2
         Component: gdi32
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

ccache /usr/bin/gcc-4.2 -O3 -c -I. -I. -I../../include -I../../include
-I/usr/include/freetype2  -D__WINESRC__ -D_GDI32_ -D_REENTRANT -fPIC -Wall
-pipe -fno-strength-reduce -fno-strict-aliasing -Wdeclaration-after-statement
-Wwrite-strings -Wpointer-arith  -Werror  -o path.o path.c
cc1: warnings being treated as errors
path.c: In function ‘PATH_PolyPolygon’:
path.c:1079: warning: ‘startpt.y’ may be used uninitialized in this
function
path.c:1079: warning: ‘startpt.x’ may be used uninitialized in this
function
make: *** [path.o] Error 1

Relevant code:
BOOL PATH_PolyPolygon( DC *dc, const POINT* pts, const INT* counts,
                       UINT polygons )
{
   GdiPath     *pPath = &dc->path;
   POINT       pt, startpt;
   UINT        poly, i;
   INT         point;

/* Check that path is open */
   if(pPath->state!=PATH_Open)
      return FALSE;

for(i = 0, poly = 0; poly < polygons; poly++) {
       for(point = 0; point < counts[poly]; point++, i++) {
           pt = pts[i];
           if(!LPtoDP(dc->hSelf, &pt, 1))
               return FALSE;
           if(point == 0) startpt = pt;
           PATH_AddEntry(pPath, &pt, (point == 0) ? PT_MOVETO : PT_LINETO);
       }
       /* win98 adds an extra line to close the figure for some reason */
       PATH_AddEntry(pPath, &startpt, PT_LINETO | PT_CLOSEFIGURE);
   }
   return TRUE;
}

[Bug 15397] gdi32: path.c fails to build with gcc 4.2, -Werror -O3

http://bugs.winehq.org/show_bug.cgi?id=15397

--- Comment #1 from Austin English   2008-09-24 14:16:36 ---
Created an attachment (id=16258)
 --> (http://bugs.winehq.org/attachment.cgi?id=16258)
patch to initialize the variables

Haven't submitted, as I'm not sure it's correct. Comments appreciated.

[Bug 15397] gdi32: path.c fails to build with gcc 4.2, -Werror -O3

http://bugs.winehq.org/show_bug.cgi?id=15397

Andrey Turkin  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #2 from Andrey Turkin   2008-09-25 04:08:09 ---
I think better way would be do nothing if counts[poly]==0 as there aren't any
points to do anything useful. Also, startpt definition could be moved into a
loop where it belong.
Something like this:
   for(i = 0, poly = 0; poly < polygons; poly++) 
     if (counts[poly]>0) {
       POINT startpt;
       for(point = 0; point < counts[poly]; point++, i++) {
        ...
       }
       PATH_AddEntry(pPath, &startpt, PT_LINETO | PT_CLOSEFIGURE);
     }

[Bug 15397] gdi32: path.c fails to build with gcc 4.2, -Werror -O3

http://bugs.winehq.org/show_bug.cgi?id=15397

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #16258|0                           |1
        is obsolete|                            |

--- Comment #3 from Austin English   2008-09-29 21:14:28 ---
Created an attachment (id=16376)
 --> (http://bugs.winehq.org/attachment.cgi?id=16376)
attempted patch

Is this what you mean?

Still fails:
austin@austin-desktop:~/wine-git/dlls/gdi32$ make
/usr/bin/gcc-4.2 -O3 -c -I. -I. -I../../include -I../../include
-I/usr/include/freetype2  -D__WINESRC__ -D_GDI32_ -D_REENTRANT -fPIC -Wall
-pipe -fno-strength-reduce -fno-strict-aliasing -Wdeclaration-after-statement
-Wwrite-strings -Wpointer-arith  -Werror  -o path.o path.c
cc1: warnings being treated as errors
path.c: In function ‘PATH_PolyPolygon’:
path.c:1518: warning: ‘startpt.x’ is used uninitialized in this function
path.c:1079: note: ‘startpt.x’ was declared here
path.c:1518: warning: ‘startpt.y’ is used uninitialized in this function
path.c:1079: note: ‘startpt.y’ was declared here
make: *** [path.o] Error 1

[Bug 15397] gdi32: path.c fails to build with gcc 4.2, -Werror -O3

http://bugs.winehq.org/show_bug.cgi?id=15397

--- Comment #4 from Andrey Turkin   2008-09-30 00:59:01 ---
The patch looks mostly OK but you've mangled braces there so code is now really
really wrong. Also there is no need to check for counts (that is array so
comparing it with 0 looks wrong too), and you should move startpt definition,
not copy it

[Bug 15397] gdi32: path.c fails to build with gcc 4.2, -Werror -O3

http://bugs.winehq.org/show_bug.cgi?id=15397

--- Comment #5 from Austin English   2008-09-30 01:46:05 ---
(In reply to comment #4)
> The patch looks mostly OK but you've mangled braces there so code is now really
> really wrong. Also there is no need to check for counts (that is array so
> comparing it with 0 looks wrong too), and you should move startpt definition,
> not copy it

For the mangled braces, there's one extra in your original suggestion...

Moving the startpt definition gives an undeclared identifier error.

[Bug 15397] gdi32: path.c fails to build with gcc 4.2, -Werror -O3

http://bugs.winehq.org/show_bug.cgi?id=15397

--- Comment #6 from Andrey Turkin   2008-09-30 02:07:05 ---
(In reply to comment #5)
> For the mangled braces, there's one extra in your original suggestion...

No there isn't. There are two opening braces and two closing braces - one pair
surrounds code for drawing a polygon, and second pair surrounds code to convert
and draw another dot inside polygon. This is true for original code, this is
true for my snippet, but your patch removes two opening braces and instead adds
two opening braces, one closing braces and remove one closing brace to
rebalance.
> Moving the startpt definition gives an undeclared identifier error.

Yes it is because and this is clear sign that something wrong with patch. The
only consumer of startpt is final PATH_AddEntry which should belong to polygon
drawing block; if you move startpt inside that block and code does not compile
anymore - that means consumer moved somewhere it doesn't belong to.

Essentially patch should add one line (check for counts[poly]), possibly move
one line (startpt definition) and maybe change indentation. Any other changes
would probably be wrong.

[Bug 15397] gdi32: path.c fails to build with gcc 4.2, -Werror -O3

http://bugs.winehq.org/show_bug.cgi?id=15397

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #16376|0                           |1
        is obsolete|                            |

--- Comment #7 from Austin English   2008-09-30 02:27:09 ---
Created an attachment (id=16378)
 --> (http://bugs.winehq.org/attachment.cgi?id=16378)
new patch

I think this is what you're saying. Still fails though:

austin@austin-desktop:~/wine-git/dlls/gdi32$ make
/usr/bin/gcc-4.2 -O3 -c -I. -I. -I../../include -I../../include
-I/usr/include/freetype2  -D__WINESRC__ -D_GDI32_ -D_REENTRANT -fPIC -Wall
-pipe -fno-strength-reduce -fno-strict-aliasing -Wdeclaration-after-statement
-Wwrite-strings -Wpointer-arith  -Werror  -o path.o path.c
cc1: warnings being treated as errors
path.c: In function ‘PATH_PolyPolygon’:
path.c:1089: warning: ‘startpt.y’ may be used uninitialized in this
function
path.c:1089: warning: ‘startpt.x’ may be used uninitialized in this
function
make: *** [path.o] Error 1

Thanks for your patience!

[Bug 15397] gdi32: path.c fails to build with gcc 4.2, -Werror -O3

http://bugs.winehq.org/show_bug.cgi?id=15397

--- Comment #8 from Andrey Turkin   2008-09-30 03:38:31 ---
Patch looks good. Which gcc version do you use? I cannot reproduce with
Gentoo's 4.2.3 p1.0

[Bug 15397] gdi32: path.c fails to build with gcc 4.2, -Werror -O3

http://bugs.winehq.org/show_bug.cgi?id=15397

--- Comment #9 from Andrey Turkin   2008-09-30 05:55:56 ---
OK, I was able to reproduce and sort-of minimize this down to:

int a();
void b(int*out, const int in)
{
    *out += in;
}
void c(int* out, int count)
{
    if (count>0)
    {
        int point, startpt;
        for(point=0;point<count;point++)
        {
            int pt = a();
            if (!pt) return;
            if (point==0) startpt = pt;
        }
        b(out, startpt);
    }
}

While it is clear that startpt cannot be used uninitialized, GCC 3.4.4 and
4.2.3 both claim it can be. Commenting out line with return make this warning
go away for some reason. GCC bug or is there something I cannot see?

[Bug 15397] gdi32: path.c fails to build with gcc 4.2, -Werror -O3

http://bugs.winehq.org/show_bug.cgi?id=15397

--- Comment #10 from Austin English   2008-09-30 12:25:30 ---
(In reply to comment #8)
> Patch looks good. Which gcc version do you use? I cannot reproduce with
> Gentoo's 4.2.3 p1.0

gcc (GCC) 4.2.3 (Ubuntu 4.2.3-2ubuntu7)

[Bug 15397] gdi32: path.c fails to build with gcc 4.2, -Werror -O3

http://bugs.winehq.org/show_bug.cgi?id=15397

--- Comment #11 from Austin English   2008-12-08 03:47:49 ---
Still present in 1.1.10.

[Bug 15397] gdi32: path.c fails to build with gcc 4.2, -Werror -O3

http://bugs.winehq.org/show_bug.cgi?id=15397

André H.  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #12 from André H.   2010-01-20 06:49:39 ---
Still present in Wine-1.1.36

[Bug 15397] gdi32: path.c fails to build with gcc 4.2, -Werror -O3

http://bugs.winehq.org/show_bug.cgi?id=15397

Jerome Leclanche  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #13 from Jerome Leclanche  2012-02-22 12:11:47 CST ---
Austin, do you still have gcc-4.2?

If it's still a bug, is this a wontfix? (aka do we care about versions this
old?)

[Bug 15397] gdi32: path.c fails to build with gcc 4.2, -Werror -O3

http://bugs.winehq.org/show_bug.cgi?id=15397

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #14 from Austin English  2012-02-22 12:46:12 CST ---
That code was removed as part of the DIB engine rewrite, so yeah, fixed.

[Bug 15397] gdi32: path.c fails to build with gcc 4.2, -Werror -O3

http://bugs.winehq.org/show_bug.cgi?id=15397

Jerome Leclanche  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
      Fixed by SHA1|                            |d4889bef47331f556cc1bfdbecf
                   |                            |b8a65ccd513cb

[Bug 15397] gdi32: path.c fails to build with gcc 4.2, -Werror -O3

http://bugs.winehq.org/show_bug.cgi?id=15397

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #15 from Alexandre Julliard  2012-02-24 12:49:04 CST ---
Closing bugs fixed in 1.4-rc5.