al_save_config_file_f() returns true when it shouldn't
Bruce Pascoe

I have the following code to save out config files in my engine:

#SelectExpand
1next_buf_size = 4096; 2while (!is_aok) { 3 free(buffer); buffer = malloc(next_buf_size); 4 memfile = al_open_memfile(buffer, next_buf_size, "wb"); 5 next_buf_size *= 2; 6 is_aok = al_save_config_file_f(memfile, file->conf); 7 file_size = al_ftell(memfile); 8 al_fclose(memfile); 9} 10if (!sfs_fspew(g_fs, file->path, "save", buffer, file_size)) 11 goto on_error; 12free(buffer);

This is needed because games run in my engine may have their assets either on disk or compressed into a wad-like package and my sfs_*() routines abstract that so that everything works in both cases. So I figured, okay, I could use a memfile to stage everything first, then call sfs_fspew() to write it out to whereever. Except for one small problem: al_save_config_file_f() returns true even if the buffer is too small to hold the entire file (try setting next_buf_size to 1 before the loop, it still "succeeds"), so my code is broken.

Any way I could get around this, or if it could perhaps be fixed in Allegro?

SiegeLord

Depending on how this error manifests itself, this can go both ways. Allegro doesn't check the return value of the file writing functions (shame on it) but it does check al_ferror which you could set (EDIT: nvm, you're using the memfile addon). I'd say Allegro's at the fault here, however.

EDIT: Another thing you might be able to do in a pinch is call al_feof on the file. It'll return true if an attempt was made to write past its size.

Bruce Pascoe

Thanks for the tip about al_feof, I didn't think to do that. I generally don't use feof myself because fread/fwrite returning a size less than what was requested generally means the same thing. But for this case it's a useful workaround.

Regardless, al_save_config_file_f should really be returning false in this situation. ;)

beoran

Yes, it's a good idea. So why not contribute a patch? :)

Bruce Pascoe

I would, but I'm not sure where to submit it to. The GitHub repo is considered a mirror, so I don't know if it would be acceptable to send a pull request there.

Thomas Fjellstrom

I think its ok to do a github pull request, but you can just send a patch to the Development forum here, or the AD mailing list.

SiegeLord

I even prefer a pull request, as it'll run the TravisCI/AppVeyor checks.

Bruce Pascoe

I'll post the patch here because Git wants to commit all the CMake cruft and I don't feel like dealing with it at the moment. :P

See attached.

SiegeLord

Thanks, applied. It took me awhile to do so, as that file was encoded in UTF-16 :P.

Bruce Pascoe

Huh, wonder how that happened. All I did was:

git diff src/config.c > config.patch

Although that was done from PowerShell (the default shell for GitHub for Windows), maybe PS outputs UTF-16 when redirecting output? Pretty dumb of it if that's the case.

Sorry about that.

Thread #615452. Printed from Allegro.cc