Page MenuHomePhabricator

elm_config: replace ecore_file_cp with ecore_file_mv
ClosedPublic

Authored by akanad on Oct 30 2019, 9:56 PM.

Details

Summary

ecore_file_cp can cause config data(eet file) invalid,
once multiple processes are trying to call elm_config_save.

this patch replaces it with ecore_file_mv to prevent the problem.

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
akanad created this revision.Oct 30 2019, 9:56 PM
akanad requested review of this revision.Oct 30 2019, 9:56 PM
zmike added a subscriber: zmike.

I don't recall if there was a specific reason for using cp instead of move here, but this seems reasonable given the issue cited.

ecore_file_mv try very hard to use an atomic rename operation, but if all attempt fails, it will fallback to ecore_file_cp. This most likely happen on non Linux OS (Maybe non Unix actually). If that fallback is acceptable, I am fine with this patch. If not, it would be best to have a lock file to prevent the override of the config. Also not saving when not necessary would be nice to avoid a slow shutdown when multiple process are trying to save the config.

akanad updated this revision to Diff 26650.Nov 3 2019, 5:43 PM

rebase onto the lastest.
and ping.

cedric accepted this revision.Nov 6 2019, 9:57 AM

Would be nice to have my comment taken into account, but it is still an improvement over current situation.

This revision is now accepted and ready to land.Nov 6 2019, 9:57 AM
This revision was automatically updated to reflect the committed changes.