Opened 13 years ago
Closed 13 years ago
Last modified 4 years ago
#4846 closed defect (fixed)
TINSEL: Build with -O2 broken
|Reported by:||lordhoto||Owned by:||fingolfin|
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.
Change History (10)
by , 13 years ago
comment:1 by , 13 years ago
comment:2 by , 13 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 , 13 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 , 13 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 , 13 years ago
comment:5 by , 13 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 , 13 years ago
I committed my version of Fingolfin's patch, thus I'm assigning this to Fingolfin.
comment:7 by , 13 years ago
|Status:||new → closed|
comment:8 by , 4 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.