latest defaultfile branch

Dirk Hohndel dirk at hohndel.org
Wed Sep 12 15:50:21 PDT 2012


Pushed to the defaultfile branch with minor changes.

/D

"Lubomir I. Ivanov" <neolit123 at gmail.com> writes:

> 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
> --

-- 
Dirk Hohndel
Intel Open Source Technology Center


More information about the subsurface mailing list