#4846 closed defect (fixed)
TINSEL: Build with -O2 broken
| Reported by: | lordhoto | Owned by: | fingolfin |
|---|---|---|---|
| Priority: | normal | Component: | Engine: Tinsel |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
ScummVM trunk r48511 g++ (Debian 4.4.3-3) 4.4.3 Linux/amd64
Builds fails on the following lines:
engines/tinsel/play.cpp: In function ‘void Tinsel::ResSoundReel(Tinsel::CoroBaseContext*&, const void*)’: engines/tinsel/play.cpp:382: error: dereferencing type-punned pointer will break strict-aliasing rules
engines/tinsel/sched.cpp: In function ‘void Tinsel::RestoredProcessProcess(Tinsel::CoroBaseContext*&, const void*)’: engines/tinsel/sched.cpp:522: error: dereferencing type-punned pointer will break strict-aliasing rules
Ticket imported from: #2981788. Ticket imported from: bugs/4846.
Attachments (2)
Change History (10)
by , 14 years ago
| Attachment: | tinsel-sched.patch added |
|---|
comment:1 by , 14 years ago
comment:2 by , 14 years ago
That indeed fixes the build. Though looking a bit into it, that seems to cast the "const" away rather cruelly, though that does not give any errors/warnings for me. I'm not sure whether it would be nicer to use const_cast for that though.
Anyway I don't have time currently to play some Tinsel game to test whether everything works fine. I might do try DW2 a bit next week....
comment:3 by , 14 years ago
Ah, it's not necessary to cast away any const-ness: In ResSoundReel, we just want to read an int, so casting to "const int *" is fine. In the other two places, it wants to cast "param" to a pointer to a pointer, so once again it would be fine to leave the constness as-is.
comment:4 by , 14 years ago
I just updated your patch to include const in the casts. That looks a bit cleaner to me at least and I'm not sure if more strict compilers will complain about the "const-away" cast otherwise.
by , 14 years ago
| Attachment: | tinsel-sched-v2.patch added |
|---|
comment:5 by , 14 years ago
I just reviewed Lorhoto's latest patch, and it all seems fine. If it fixes the compiler errors when using strict mode, I'd say go ahead and commit it.
comment:6 by , 14 years ago
I committed my version of Fingolfin's patch, thus I'm assigning this to Fingolfin.
comment:7 by , 14 years ago
| Owner: | set to |
|---|---|
| Resolution: | → fixed |
| Status: | new → closed |
comment:8 by , 6 years ago
| Component: | → Engine: Tinsel |
|---|
The attached patch should fix the issue, I hope. But I haven't had a chance to try it out, and I am hesitant to commit it without any testing.