ALLEGRO_NATIVE_PATH_SEP+PHYSFS bug on Windows?
kenmasters1976

I've just spent hours hunting for a bug and was finally able to isolate the code causing it. It seems that using ALLEGRO_NATIVE_PATH_SEP with PHYFS won't work on Windows. Consider the following code:

#SelectExpand
1#include <stdint.h> 2#include <string.h> 3#include <allegro5/allegro.h> 4#include <allegro5/allegro_image.h> 5#include <allegro5/allegro_physfs.h> 6 7ALLEGRO_DISPLAY *display = NULL; 8 9int main(int argc, char **argv) { 10 assert(al_init()); 11 assert(al_install_keyboard()); 12 al_set_new_display_flags(ALLEGRO_WINDOWED); 13 display = al_create_display(800, 600); 14 assert(display); 15 assert(al_init_image_addon()); 16
17 assert(PHYSFS_init(argv[0]));
18 assert(PHYSFS_mount("my-file.zip", NULL, 0));
20 21 ALLEGRO_PATH *path = al_create_path("data/"); 22 al_set_path_filename(path, "image.png"); 23 printf(al_path_cstr(path, ALLEGRO_NATIVE_PATH_SEP)); 24 ALLEGRO_BITMAP *bmp = al_load_bitmap(al_path_cstr(path, ALLEGRO_NATIVE_PATH_SEP)); 25 assert(bmp); 26 al_draw_bitmap(bmp, 0, 0, 0); 27 al_flip_display(); 28 al_rest(5); 29}

I'm working on Linux and the code works whether you use PHYSFS (load the file data/image.png from the my-file.zip archive) or if you comment the marked lines and don't use PHYSFS (load image.png from the data/ folder). Both work on Linux.

However, when building for Windows, the PHYSFS version won't work. The code will work fine if you don't use PHYSFS, though.

I tested this with wine and with a Windows XP VM, both with the same results.

Of course, you can use '/' instead of ALLEGRO_NATIVE_PATH_SEP and the problem disappears but it's still annoying trying to use ALLEGRO_NATIVE_PATH_SEP and figure out that it's the cause of your problems.

Elias

Personally I would be for deprecating ALLEGRO_NATIVE_PATH_SEP. There is no reason to ever use it (Windows filesystem functions all understand / in place of \). And when displaying a path to a user I doubt they really care if you show them C:\\GAMES or C://GAMES.

Having said that, I have not used a Windows in a very long time, so maybe there is cases where \ needs to be used?

l j

There are cases such as FOR loop variables being wrongly set in the windows command prompt when using "/" instead of "\". Some old command prompt commands also use "/" for named arguments and might potentially cause unexpected results.

In my experience it will generally not cause any issues and I use "/" all the time, except for batch scripts.

Audric
Elias said:

so maybe there is cases where \ needs to be used?

Off the top of my head, I can see two kinds of cases:
- As output : In order to show a path to a user, for example in order to display "You are editing C:\Data\Users\(...)\Image4.png"
- As input : If you need to parse a path produced as a string by a system command or third-party library. Or if you want to parse argv[0]

In the second case, it's actually better to expect both separators when you're on Windows OS. I remember slashes when running a program through sh or gdb, and antislashes otherwise.

kenmasters1976

Using '/' seems like the way to go as solving the issue might be more complicated than it seems. Any solution in Windows might fix the issue in some places but break things somewhere else.

That said, could this be a PHYSFS issue and not Allegro's? I mean, if you use data\image.png (as per the output of the printf() in line 23) with al_load_bitmap(), it will work fine if you're not using PHYSFS but fail with PHYSFS. A quick look at the PHYSFS documentation seems to indicate that PHYSFS usually expects its filenames to be written in platform-independent notation. So, that might likely be the cause of the problem.

[EDIT:] Indeed, the following minimal PHYSFS code fails when using '\\' as path separator:

#include <stdio.h>
#include <assert.h>
#include <physfs.h>

int main(int argc, char **argv) {
    assert(PHYSFS_init(argv[0]));
    assert(PHYSFS_mount("my-file.zip", NULL, 0));
    PHYSFS_File *file = PHYSFS_openRead("data\\image.png");
    assert(file);
}

Edgar Reynaldo

Uhm, I might be hallucinating, but PHYSFS expects /, just like CMake, and lots of other things.

To expand, physfs is a virtual file system. Its conventions are to use forward / slash. It's totally arbitrary, but you have to convert windows filenames to use forward slashes yourself if they are in the DOS format.

EDIT
And actually, I use ALLEGRO_NATIVE_PATH_SEP myself, but it's easily replaced by a couple ifdefs.

kenmasters1976

Yeah, I was trying to say that indeed, the error in my code was caused by using the Windows ALLEGRO_NATIVE_PATH_SEP ('\\') with PHYSFS. Knowing the PHYSFS convention makes it obvious that if you're going to use PHYSFS you have to use forward slashes but it's not something that's immediately obvious if you're not aware of the PHYSFS convention.

I guess it's not really an Allegro bug, nor a PHYSFS bug. Just something that you must be aware of when using both together.

Edgar Reynaldo

Yeah, just something to be aware of. ;)

And CMake-GUI, doesn't tell you anything is wrong with your paths until you hit configure. :/

Also, I have a SanitizePath() function for just this purpose. Turns everything into a forward slash path.

torhu

So: PhysFS uses forward slashes only? The way to deal with that seems to be to make a macro or function that returns the correct path separator, based on ALLEGRO_NATIVE_PATH_SEP and whether you are using PhysFS or not.

Edgar Reynaldo

There's no OS that doesn't understand forward slashes, at least when it comes to system calls, everything should just automatically be converted to / <//>

Thread #617884. Printed from Allegro.cc