Opened 10 years ago

Closed 10 years ago

Last modified 13 months ago

#4846 closed defect (fixed)

TINSEL: Build with -O2 broken

Reported by: lordhoto Owned by: fingolfin
Priority: normal Component: Engine: Tinsel
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)

tinsel-sched.patch (1.8 KB ) - added by fingolfin 10 years ago.
tinsel-sched-v2.patch (2.0 KB ) - added by lordhoto 10 years ago.

Download all attachments as: .zip

Change History (10)

by fingolfin, 10 years ago

Attachment: tinsel-sched.patch added

comment:1 by fingolfin, 10 years ago

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.

comment:2 by lordhoto, 10 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 fingolfin, 10 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 lordhoto, 10 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 lordhoto, 10 years ago

Attachment: tinsel-sched-v2.patch added

comment:5 by dreammaster, 10 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 lordhoto, 10 years ago

I committed my version of Fingolfin's patch, thus I'm assigning this to Fingolfin.

comment:7 by lordhoto, 10 years ago

Owner: set to fingolfin
Resolution: fixed
Status: newclosed

comment:8 by digitall, 13 months ago

Component: Engine: Tinsel
Note: See TracTickets for help on using tickets.