Allegro.cc - Online Community

Allegro.cc Forums » Allegro Development » al_save_config_file_f() returns true when it shouldn't

This thread is locked; no one can reply to it. rss feed Print
al_save_config_file_f() returns true when it shouldn't
Bruce Pascoe
Member #15,931
April 2015
avatar

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
Member #7,827
October 2006
avatar

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.

"For in much wisdom is much grief: and he that increases knowledge increases sorrow."-Ecclesiastes 1:18
[SiegeLord's Abode][Codes]:[DAllegro5]:[RustAllegro]

Bruce Pascoe
Member #15,931
April 2015
avatar

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
Member #12,636
March 2011

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

Bruce Pascoe
Member #15,931
April 2015
avatar

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
Member #476
June 2000
avatar

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.

--
Thomas Fjellstrom - [website] - [email] - [Allegro Wiki] - [Allegro TODO]
"If you can't think of a better solution, don't try to make a better solution." -- weapon_S
"The less evidence we have for what we believe is certain, the more violently we defend beliefs against those who don't agree" -- https://twitter.com/neiltyson/status/592870205409353730

SiegeLord
Member #7,827
October 2006
avatar

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

"For in much wisdom is much grief: and he that increases knowledge increases sorrow."-Ecclesiastes 1:18
[SiegeLord's Abode][Codes]:[DAllegro5]:[RustAllegro]

Bruce Pascoe
Member #15,931
April 2015
avatar

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
Member #7,827
October 2006
avatar

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

"For in much wisdom is much grief: and he that increases knowledge increases sorrow."-Ecclesiastes 1:18
[SiegeLord's Abode][Codes]:[DAllegro5]:[RustAllegro]

Bruce Pascoe
Member #15,931
April 2015
avatar

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.

Go to: