Preventing destroying of symbolic/hard links in Windows OS. #1270

ghost · 2016-10-21T23:40:59Z

Preventing destroying of symbolic/hard links to real documents on "document-save" in Windows OS.

Preventing destroying of symbolic/hard links to real documents on "document-save" on Windows OS.

codebrainz · 2016-10-22T00:06:43Z

src/document.c

@@ -1955,37 +1961,51 @@ static gchar *write_data_to_disk(const gchar *locale_filename,
 		/* Use POSIX API for unsafe saving (GVFS-unsafe) */
 		/* The error handling is taken from glib-2.26.0 gfileutils.c */
 		errno = 0;
-		fp = g_fopen(locale_filename, "wb");
+		fp = g_fopen(locale_filename, "w+b"); // we can not destroy  symbolic/hard links


Why does it need to open for reading and writing to write the file to disk?

if use "w" in case of appcrash will lost entire file.
if use "a" we can not seek to start of file and rewrite it.
if "w+" we can have significant part of file if appcrash, because truncate+write.
(fix) "r+" instead "w+"

Both "w" and "w+" truncate the file and discard its contents (see here). If you observe something else, it might be undefined behaviour. Or did I misunderstand?

@biv91 have you checked that remote filesystems like sshfs support this?

If you havn't tested it on Linux and OSX and with a large number of different filesystems I would be happier if this was limited to windows only, not imposed on everybody.

We need open for writing without truncation to empty, set write position to start, truncate to dataLength, write data to file.

The linked document says for "w+":

write/update: Create an empty file and open it for update (both for input and output). If a file with the same name already exists its contents are discarded and the file is treated as a new empty file.

And my man pages say:

Open for reading and writing. The file is created if it does not exist, otherwise it is truncated. The stream is positioned at the beginning of the file.

Both sources seem to agree that the file is truncated for both "w" and "w+". Am I being dense?

I found good option "r+", for overwriting file from start + truncate to writingDataLength, we got half-safe writing of document.

codebrainz · 2016-10-22T00:07:21Z

src/document.c

+	#if defined(_WIN32)
+	#elif defined(_WIN64)
+	#elif defined(__CYGWIN__)
+	#elif defined(__MSYS__)


I think _WIN32 is enough to cover all of the above. GLib has one defined though already, it's called G_OS_WIN32.

elextr · 2016-10-22T00:09:44Z

src/document.c

@@ -1934,7 +1934,12 @@ static gchar *write_data_to_disk(const gchar *locale_filename,
 		if (g_file_set_contents(locale_filename, data, len, &error))


Why does g_file_set_contents() not have the same problem?

elextr · 2016-10-22T00:13:45Z

I suspect your problem may be that the real path is not being determined correctly rather than how its written.

codebrainz · 2016-10-22T00:17:37Z

I suspect your problem may be that the real path is not being determined correctly rather than how its written.

I believe the "realpath" code in TagManager predates Windows having symbolic links (Vista/2007), so quite possibly.

elextr · 2016-10-22T01:45:11Z

src/document.c

-			bytes_written = fwrite(data, sizeof(gchar), len, fp);
-
-			if (len != bytes_written)
+			if(truncate(locale_filename,sizeof(gchar)*len)) // no can change file size error!


Glib makes locale_filename UTF8 on windows, you can't pass it to a clib function I don't think.

truncate() is POSIX 2008 and AFAIK not available on Windows

elextr · 2016-10-22T01:50:42Z

@biv91 The basic file write method is intended to be simple and fast, especially useful for slow remote filesystems. We regularly get complaints about other save methods being too slow. Your changes mean it now writes all zeros and then overwrites with the data, potentially doubling the transfers.

Also that change has nothing to do with overwriting links on windows. Please separate the changes.

elextr · 2016-10-22T02:19:38Z

This PR should be rejected, unconditionally removing GIO saving on windows removes functionality that is useful to other users. It can already be disabled at runtime with a pref if a user does not want to use it.

Note: fixing the travis errors, and separating the links overwriting from the truncate() changes in another PR might make it acceptable.

ghost · 2016-10-22T02:43:36Z

realpath() can not determine hard links, current stable Geany destroys them too. This means - all it doing Glib.

elextr · 2016-10-22T02:47:30Z

@biv91 what do you mean by "destroys" hard links?

ghost · 2016-10-22T02:49:29Z

Create 2 files hard linked. Edit & Save one with Gewany & we have 2 separate files. I means.

elextr · 2016-10-22T02:59:48Z

@biv91 on Linux a hardlinked file is updated when either plain file saving or GIO saving is used, and the links split when atomic saving is used. This is what is expected. If there is a different behaviour on windows then:

describe it clearly, describing the behaviour in all the saving settings
if you wish to make a change for windows please make code that is specific to windows, do not make untested changes to code that is common to all systems.
Do not compile out options that can be disabled at runtime (GIO saving)

ghost · 2016-10-22T03:00:38Z

@elextr "This PR should be rejected, unconditionally removing GIO saving"
Good idvise, I will do it in settings.

elextr · 2016-10-22T03:01:35Z

@biv91 GIO is already disablable in settings

ghost · 2016-10-22T03:10:18Z

@elextr , I am just now founf editor options. Maybe I will uncomment GIO-safe-write, but leave unsafe-write which I was turn into half-safe-writing, because it better.

elextr · 2016-10-22T03:17:10Z

@biv91 I say again do not change common code. The simple unsafe saving code suits specific use-cases, and your half safe saving adds overheads that may not be wanted. If you want another saving method, give it an option of its own and make it separate code.

If you just found the settings I suggest you read this before you go changing a critical function such as file saving.

ghost · 2016-10-22T04:16:18Z

Best solution - turn of Atomic-saving and GIO-unsafe-saving. This works perfect. No need code changes.

Preventing destroying of symbolic/hard links on Windows OS.

961af63

Preventing destroying of symbolic/hard links to real documents on "document-save" on Windows OS.

codebrainz reviewed Oct 22, 2016

View reviewed changes

elextr reviewed Oct 22, 2016

View reviewed changes

Add files via upload

0484b62

elextr added the windows label Oct 22, 2016

Update document.c

00d9218

error fixed

03cb91a

uncommented

47e78ee

elextr added the workaround available label Oct 22, 2016

elextr closed this Oct 22, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preventing destroying of symbolic/hard links in Windows OS. #1270

Preventing destroying of symbolic/hard links in Windows OS. #1270

ghost commented Oct 21, 2016

codebrainz Oct 22, 2016

ghost Oct 22, 2016 •

edited by ghost

codebrainz Oct 22, 2016 •

edited

elextr Oct 22, 2016

ghost Oct 22, 2016

codebrainz Oct 22, 2016

ghost Oct 22, 2016

codebrainz Oct 22, 2016

elextr Oct 22, 2016

elextr commented Oct 22, 2016

codebrainz commented Oct 22, 2016

elextr Oct 22, 2016

codebrainz Oct 22, 2016

elextr commented Oct 22, 2016

elextr commented Oct 22, 2016 •

edited

ghost commented Oct 22, 2016

elextr commented Oct 22, 2016

ghost commented Oct 22, 2016

elextr commented Oct 22, 2016

ghost commented Oct 22, 2016 •

edited by ghost

elextr commented Oct 22, 2016

ghost commented Oct 22, 2016

elextr commented Oct 22, 2016

ghost commented Oct 22, 2016

		@@ -1934,7 +1934,12 @@ static gchar write_data_to_disk(const gchar locale_filename,
		if (g_file_set_contents(locale_filename, data, len, &error))

Preventing destroying of symbolic/hard links in Windows OS. #1270

Preventing destroying of symbolic/hard links in Windows OS. #1270

Conversation

ghost commented Oct 21, 2016

Choose a reason for hiding this comment

ghost Oct 22, 2016 • edited by ghost

Choose a reason for hiding this comment

codebrainz Oct 22, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elextr commented Oct 22, 2016

codebrainz commented Oct 22, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elextr commented Oct 22, 2016

elextr commented Oct 22, 2016 • edited

ghost commented Oct 22, 2016

elextr commented Oct 22, 2016

ghost commented Oct 22, 2016

elextr commented Oct 22, 2016

ghost commented Oct 22, 2016 • edited by ghost

elextr commented Oct 22, 2016

ghost commented Oct 22, 2016

elextr commented Oct 22, 2016

ghost commented Oct 22, 2016

ghost Oct 22, 2016 •

edited by ghost

codebrainz Oct 22, 2016 •

edited

elextr commented Oct 22, 2016 •

edited

ghost commented Oct 22, 2016 •

edited by ghost