Bug 183033 - Allegro crashes when required resources not available
Summary: Allegro crashes when required resources not available
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: allegro
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jindrich Novy
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard: bzcl34nup
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2006-02-25 17:48 UTC by Frank Arnold
Modified: 2013-07-02 23:14 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2008-04-04 08:52:08 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Patch as a result of a minor source code audit of allegro. (14.17 KB, patch)
2006-02-27 12:37 UTC, Jindrich Novy
no flags Details | Diff

Description Frank Arnold 2006-02-25 17:48:04 UTC
Description of problem:
I stumbled over this one by accident. When trying to start games using allegro
through a ssh terminal connection allegro will segfault. I know this isn't
supposed to work, but I would expect an error message to stderr rather then a crash.

Version-Release number of selected component (if applicable):
allegro-4.2.0-7
overgod-1.0-3.fc5
lacewing-1.10-4.fc5

How reproducible:
Everytime.

Steps to Reproduce:
1. Remote login to a machine with overgod or lacewing installed (not Xnest or
something, just plain shell)
2. Try to start a game.
3. Watch it crash.
  
Actual results:
Backtrace from overgod
#0  0x0feaa7f0 in _set_gfx_mode (card=-1, w=0, h=0, v_w=0, v_h=0,
allow_config=-1) at ./src/graphics.c:637
#1  0x0feaaf60 in set_gfx_mode (card=Variable "card" is not available.
) at ./src/graphics.c:609
#2  0x1004c674 in main ()

Backtrace from lacewing
#0  0x0febc5c8 in install_keyboard () at ./src/keyboard.c:658
#1  0x1004f0f8 in initialize ()
#2  0x10055cb4 in main ()

Expected results:
A small error message to stderr stating that keyboard, video card, or other
resources are not available.

Comment 1 Jindrich Novy 2006-02-27 09:19:45 UTC
This bug is caused by the fact that allegro lacks checks whether system_driver
isn't NULL before accessing system_driver members. For this reason it segfaults
at least while initializing keyboard on timer. Patch is coming soon.

Comment 2 Jindrich Novy 2006-02-27 11:29:58 UTC
There are two possible solutions to this issue.

1) The problem is that the checks are missing almost everywhere in allegro so
the solution might be to exit allegro when no valid system driver is found while
searching one in _install_allegro() (src/allegro.c).

2) patch various parts of install_keyboard(), install_timer(), _set_gfx_mode(),
etc. to deal with the NULL system_driver what it might make it more stable and
slower.

Personally I like the first idea as it doesn't have much sense to go on in
execution of an allegro app when no system driver is found.

Frank, Hans, what's your opinion?

Comment 3 Hans de Goede 2006-02-27 11:46:37 UTC
Actually I like 1, but instead of exit you could return a status code, because
according to:
http://www.talula.demon.co.uk/allegro/onlinedocs/en/api.html

install_allegro now has an exit code and programs should check it. Problem with
this is 4.2 is almost 100% compatible with 4.0 and under 4.0 apps didn't need to
check this as allegro_init indeed used to call exit in this case. Actually I
think this is not an allegro issue at all because reading the above page
install_allegro will already return in error in the described case, so the real
problem is probably in lacewing, overgod and likely other allegro users.


Comment 4 Jindrich Novy 2006-02-27 12:37:45 UTC
Created attachment 125312 [details]
Patch as a result of a minor source code audit of allegro.

Hans,

I had a look into the code and it seems like the the upstream code has checks
for non-NULL system_driver at many parts of the code but not everywhere. It
might show that an original intention was to make it bullet-proof also for the
case that the return value of the allegro_init() aka _install_allegro() isn't
properly checked and the program goes on. Maybe for keeping the backward
compatibility I'd apply this patch since any such application would crash at
keyboard/timer/graphics initialization immediatelly with segfault for lots of
legacy code not aware of checking the allegro_init() when no suitable system
driver is found. The checks for non-NULLness of system_driver doesn't slow down
allegro much as it's used mostly exclusively in initialization functions that
ale called once.

On many places I see ASSERT(system_driver) before accessing components of the
system_driver, however this check is bogus for the non-debug compilation of
allegro as the macro does nothing then.

Comment 5 Hans de Goede 2006-02-28 08:21:04 UTC
Jindrich,

I don't know if this is the best plan, applications will most probably still
behave unexpected (and likely just crash somewhere else) if there is no system
driver. I'm not saying you're patch is bad, but I do not believe its a complete
solution we need to either:
1) restore the exit() call when no system driver is found
2) get all the apps fixed

I would prefer 2, we could add a fprintf(stderr, ... with a warning to all the
checks in your patch with a request to bugzilla it.

Also it might be a good idea to take this issue upstream and let them choose the
preferred solution.


Comment 6 Bug Zapper 2008-04-03 17:01:43 UTC
Based on the date this bug was created, it appears to have been reported
against rawhide during the development of a Fedora release that is no
longer maintained. In order to refocus our efforts as a project we are
flagging all of the open bugs for releases which are no longer
maintained. If this bug remains in NEEDINFO thirty (30) days from now,
we will automatically close it.

If you can reproduce this bug in a maintained Fedora version (7, 8, or
rawhide), please change this bug to the respective version and change
the status to ASSIGNED. (If you're unable to change the bug's version
or status, add a comment to the bug and someone will change it for you.)

Thanks for your help, and we apologize again that we haven't handled
these issues to this point.

The process we're following is outlined here:
http://fedoraproject.org/wiki/BugZappers/F9CleanUp

We will be following the process here:
http://fedoraproject.org/wiki/BugZappers/HouseKeeping to ensure this
doesn't happen again.

Comment 7 Hans de Goede 2008-04-04 08:52:08 UTC
This bug is still valid, the problem is there is no clear way howto solve this.
The best solution would still be for allegro using applicaitions to check the
return value of install_allegro().

Adding an exit() call to install_allegro() would break the API, so thats not
really an option. Adding checks for a NULL sysdriver everywhere doesn't help
much for applications which assume things will just work, iow those which don't
check the return value of allegro_install in the first place.

Conclusion, the apps should be fixed, not allegro, closing.



Note You need to log in before you can comment on or make changes to this bug.