latest defaultfile branch

Lubomir I. Ivanov neolit123 at gmail.com
Wed Sep 12 14:04:53 PDT 2012


On 12 September 2012 02:28, Dirk Hohndel <dirk at hohndel.org> wrote:
>
> Folks,
>
> After going back and forth about this a number of times I still think
> that this code is a major improvement compared to what we have in master
> today.
>
> Can I please get some testing on Mac and Windows (and of course also
> Linux), and especially Henrik, can you verify that you can still see the
> bug that you reported (because either I don't understand the steps that
> you sent, or I'm seriously confused... it seems to work as expected
> here).
>
>
> for you to fetch changes up to be941e00b20cc0b8b3b46e04200ce15c3023ad23:
>

hello,

here is a patch i did with some changes. please, let me know if i'm
doing / talking nonsense...sure explaining these is difficult enough.

the patch commit message broken down with comments:
------------
1) for safety reasons, the directory creation if "appdata/subsurace" is missing,
is permanent i.e. folder remains. otherwise it can result in an access error
(on windows) and also a couple of small exploits are possible.
------------

the error can be reproduced on windows if a default file name is
picked then "ok" is pressed (i don't think it happens with cancel). it
creates directory, shows in GTK, deletes directory, but when ok is
pressed i believe GTK checks for the file in the directory (unlike)
cancel and this i think fails the rmdir call and also the directory
somehow becomes locked and not accessible even to subsurface, while
_it_ is the process (according to a peace of software that i have)
keeping the directory locked o_O. i believe this could be some sort of
a async file system access issue.

some other issues are related to undefined behaviour, for example,
when the user browses and/or creates files in the folder in question,
while the "pick" dialog is running.

if you wish to removed the folder if it has remained empty, i suggest
to do that when closing the application, which is much safer.

------------
2) once default_filename is allocated, keep it, until the value has to change.
the value is released once the program is ready to close.
------------
i've removed some of the calls to free((void *)default_file). there
was a bug where a section of the code accessed the pointer, while it
was freed and this returned in junk chars in the "label" and
eventually a seg fault. i think that the pointer had to be set to NULL
after freeing, but why not we can keep the memory allocated until it
has to change or has to be freed completely.

------------
3) when picking a new default file, grab the new string directly from the
GSList.
------------
...


------------
4) when storing the new default file from the preferences dialog, make sure
we also update "existing_filename" if needed.
------------
can be reproduced if a default file name is picked. then "save" is
called, then if new default file name is picked and "save" is called
again it saves to the first picked one.
at least with my tests.

sorry for the TLDR
lubomir
--
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-various-small-fixes-to-the-defaultfile-model.patch
Type: application/octet-stream
Size: 4705 bytes
Desc: not available
URL: <http://lists.hohndel.org/pipermail/subsurface/attachments/20120913/f50666cf/attachment.obj>


More information about the subsurface mailing list