Re: [evolution-patches] Patch for attaching remote url



Nice patch, a few issues though:

- memory leaks
  update_remote_file should take a const char * as its message text
  where it is called you need to free the return of g_strdup_printf()
  you also need to free the result of g_path_get_basename

With the mkdtemp usage, it would be better if you only create 1 path
global to the composer and put all the downloaded files in the same
location:

+e_msg_composer_attachment_bar_attach_remote_file
(EMsgComposerAttachmentBar *bar,
+                                                 const gchar *url)
+{
+       EMsgComposerAttachment *attachment;
+       CamelException ex;
+       gchar *tmpfile;
+       gchar *path, *base;
+
+       base = g_path_get_basename (url);
+       path = e_mkdtemp("attach-XXXXXX");

Instead of having a whole new entry point for attaching a remote file,
probably easier just to add a boolean argument to say its remote - at
least from the patch it looks like thats the only difference in the
generated structure.

+EMsgComposerAttachment *
+e_msg_composer_attachment_new_remote_file (const char *file_name,
+                                          const char *disposition,
+                                          CamelException *ex)


I haven't had a really close look at all the detailed cases but the demo
was impressive :)


On Fri, 2005-05-20 at 05:06 -0600, Srinivasa Ragavan wrote:




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]