GNOME Bugzilla – Bug 711620
curlftpsink: Create a temporary file during FTP transfer/upload
Last modified: 2014-01-14 19:28:16 UTC
Created attachment 259195 [details] [review] This is the patch which will have this temporary file feature while uploading FTP files Use temporary file - Some FTP servers do not allow an existing file to be overwritten by a new file with the same name. When this option is enabled, the new file will be uploaded with a temporary name. The original file is then deleted and the new file is given the original file name. Problem can be observerd when using a bad 3G connection because: If you upload the file, overwriting the existing one directly and the connection is lost, you end up in a corrupted file. If you work with the temporary upload instead, the file is only overwritten in case of a successful upload. So no corrupted files at all.
Review of attachment 259195 [details] [review]: ::: ext/curl/gstcurlftpsink.c @@ +206,3 @@ + timeinfo = localtime (&rawtime); + strftime (timeinfo_buffer, TIMEINFO_BUFFERSIZE, "%d_%B_%Y_%I_%M_%S_%p", timeinfo); + rename_from = g_strdup_printf ("%s%s%s", RENAME_FROM, tmpfile_name, timeinfo_buffer); rename_from is only needed in the "if" part below, and leaked otherwise @@ +208,3 @@ + rename_from = g_strdup_printf ("%s%s%s", RENAME_FROM, tmpfile_name, timeinfo_buffer); + + last_slash = strrchr (basesink->file_name, G_DIR_SEPARATOR); You probably want to use "/" as a separator always here, instead of "\\" on Windows. But isn't the filename here guaranteed to be just a filename and the path is separate from that?
Created attachment 259266 [details] [review] I have modified the code such rename_from will be freed in the end. To answer your question, the basesink->file_name can contain either filename alone or with the pathname. It depends on the user needs. I have modified the code accordingly. rename_from is ensured that, it is cleared in the end. Eg. "filename.jpg" or "directory/filename.jpg".
Please see the attached patch.
Hi, Any inputs for me? Thanks, Hari.
Review of attachment 259266 [details] [review]: ::: ext/curl/gstcurlftpsink.c @@ +139,3 @@ + g_object_class_install_property (gobject_class, PROP_CREATE_TEMP_FILE, + g_param_spec_boolean ("create-tmpfile", "Enable or disable temporary file transfer", + "Creating a temporary file ensures a proper file transfer", Might benefit from something more descriptive like "Upload to a temporary file then replace the original file" @@ +197,3 @@ + tmpfile_name = g_strdup_printf ("%s", sink->tmpfile_name); + } else { + tmpfile_name = g_strdup_printf ("%s", UPLOAD_FILE_AS); Might be better to have something random as the default ? Along the files of rename_to + ".tmp.%04%04X", g_random_int(), g_random_int()
Created attachment 263654 [details] [review] Added comments given by Olivier Crete Hi, I have added commments given by Olivier Crete. kindly review my patch. Regards, Haridass.
Review of attachment 263654 [details] [review]: ::: ext/curl/gstcurlftpsink.h @@ +46,2 @@ /*< private > */ + struct curl_slist *headerlist; Do you need to keep the list alive after setting it on libcurl?
Hi, I did not get your point, can you please explain a bit more? What do you mean by keeping the list alive? Regards, Haridass.
(In reply to comment #9) > I did not get your point, can you please explain a bit more? What do you mean > by keeping the list alive? Why do you put this list in the pbject structure instead of keeping it local to the function ?
(In reply to comment #10) > (In reply to comment #9) > > I did not get your point, can you please explain a bit more? What do you mean > > by keeping the list alive? > > Why do you put this list in the pbject structure instead of keeping it local to > the function ? I got your point now. Yes, this list should be kept alive. We cannot keep this local to the function.
Comment on attachment 263654 [details] [review] Added comments given by Olivier Crete Now only attach this in "git format-patch" style with your real name and mail address and it can be included :)
Created attachment 264403 [details] [review] Patch with "git format-patch" Hi, Added the patch with "git format-patch". Regards, Haridass.
commit 23ab7d8c4979bff05e37e2e4aff5a74da2b36c3a Author: Haridass Selvaraj <haridasj@axis.com> Date: Tue Dec 17 12:06:13 2013 +0100 curlftpsink: Optionally create a temporary file during FTP transfer/upload https://bugzilla.gnome.org/show_bug.cgi?id=711620