[AD] [ alleg-Bugs-1890478 ] pack_fopen and file_size_ex not thread safe

["SourceForge.net" <noreply@xxxxxxxxxx>]

Bugs item #1890478, was opened at 2008-02-10 02:10
Message generated for change (Comment added) made by evouga
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105665&aid=1890478&group_id=5665

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Core Library
Group: 4.2.0rc2
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Etienne Vouga (evouga)
Assigned to: Nobody/Anonymous (nobody)
Summary: pack_fopen and file_size_ex not thread safe

Initial Comment:
pack_fopen() and file_size_ex() are not thread-safe, because they refer to a global password set by packfile_password(). This limitation makes using packfiles in multi-threaded applications painful. To get around the problem, I've patched Allegro 4.2.2's file.h, file.c, and aintern.h to include the following new methods:

pack_fopen_password()
file_size_ex_password()

which work just like the regular versions of these functions, but take in an explicit const char *password to use instead of the global password. Passing in NULL means to default to the global password (not thread-safe, obviously.)

I would love to see functionality similar to this patch integrated into Allegro, as the packfile password is the only reason Allegro packfile IO is not thread-safe.

----------------------------------------------------------------------

>Comment By: Etienne Vouga (evouga)
Date: 2008-02-18 19:19

Message:
Logged In: YES 
user_id=1722957
Originator: YES

File Added: file.c.diff

----------------------------------------------------------------------

Comment By: Etienne Vouga (evouga)
Date: 2008-02-18 19:18

Message:
Logged In: YES 
user_id=1722957
Originator: YES

File Added: aintern.h.diff

----------------------------------------------------------------------

Comment By: David A. Capello (dacap)
Date: 2008-02-10 14:31

Message:
Logged In: YES 
user_id=223055
Originator: NO

Wait... "allegro_errno" is a pointer to standard "errno"?
In that way I think it should work fine.

----------------------------------------------------------------------

Comment By: David A. Capello (dacap)
Date: 2008-02-10 14:21

Message:
Logged In: YES 
user_id=223055
Originator: NO

The "allegro_errno" global variable could be another big problem for
multithreaded applications. Mainly if you want to have two (or more)
threads loading files in parallel, and then you want to know which of them
failed.

----------------------------------------------------------------------

Comment By: Elias Pschernig (elias)
Date: 2008-02-10 07:05

Message:
Logged In: YES 
user_id=32894
Originator: NO

Thanks, it does sound like a good idea, and the 4.3.10+ branch would be a
place where we actually could still add this to a 4.x version right now.
However, it should be possible to fix this by using your own PACKFILE
vtable with encryption (Allegro's encryption is very very weak, as you
probably know after looking at the source - so not much point using the
builtin password functionality anyway).

Also, could you provide a patch (preferably against 4.3.10+ SVN) instead
of the complete files?

----------------------------------------------------------------------

Comment By: Etienne Vouga (evouga)
Date: 2008-02-10 02:55

Message:
Logged In: YES 
user_id=1722957
Originator: YES

File Added: fixed.zip

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105665&aid=1890478&group_id=5665