Re: Segmentation fault when exporting files to the filesystem

Hi Chris,

I have just pushed the fix for this so if you would like to test it out 
then it is now in git or you can hopefully download one of the unstable 
builds shortly.

Cheers

phantomjinx

On 14/02/11 04:49, Chris Jenkins wrote:
> Hi phantomjinx,
>
> Thanks for incorporating the patch :-). I was thinking that maybe a 
> better way of doing it would be for the worker thread to return a 
> status message (instead of popping up a dialog box each time) and then 
> to display all the status messages at the end, after all export 
> operations have ben attempted. Let me know if it would be useful for 
> me to do any work on this. Otherwise, I will wait to see what changes 
> you put in.
>
> Cheers,
>
> Chris
>
>
>
> On 13 February 2011 22:22, phantomjinx 
> <[email protected] 
> <mailto:[email protected]>> wrote:
>
>     On 13/02/11 16:51, Chris Jenkins wrote:
>     > Hi,
>     >
>     > I began using gtkpod recently and tried to use it to export some
>     files
>     > from my iPod to the filesystem, which resulted in a segmentation
>     > fault. After playing with the source code for a bit, it looks like I
>     > have found this bug:
>     >
>     > http://gtkpod.org/bugs/index.php?do=details&task_id=37
>     <http://gtkpod.org/bugs/index.php?do=details&task_id=37>
>     > <http://gtkpod.org/bugs/index.php?do=details&task_id=37
>     <http://gtkpod.org/bugs/index.php?do=details&task_id=37>>
>     >
>     > I think that what is happening is this: The file that I'm trying to
>     > export has the type .m4p and there is no template for this
>     filetype. A
>     > worker thread is created to do the export and, when it fails to
>     find a
>     > template for that filetype, it tries to create a warning dialog. It
>     > looks like this causes a race condition in gtk (though I'm not sure
>     > because I don't know much about gtk :-).
>     >
>     > Here is a patch which fixes the problem for me - it simply moves the
>     > code that works out the filename out of the worker thread. It's
>     quite
>     > hacky but I was wondering if it would be useful or if it would be
>     > worth me spending some more time trying to come up with a more
>     elegant
>     > solution.
>     >
>     > Cheers,
>     >
>     > Chris
>     >
>     >
>     > From f60f8dfcab1ec60e81460343bf8d1becb57b9ec0 Mon Sep 17
>     00:00:00 2001
>     > From: cdpjenkins <[email protected]
>     <mailto:[email protected]> <mailto:[email protected]
>     <mailto:[email protected]>>>
>     > Date: Sun, 13 Feb 2011 16:09:40 +0000
>     > Subject: [PATCH] Hacky fix for SEGV on file export
>     >
>     > ---
>     >  plugins/exporter/file_export.c |   14 ++++++++++++--
>     >  1 files changed, 12 insertions(+), 2 deletions(-)
>     >
>     > diff --git a/plugins/exporter/file_export.c
>     > b/plugins/exporter/file_export.c
>     > index 77a43a2..ec357b2 100644
>     > --- a/plugins/exporter/file_export.c
>     > +++ b/plugins/exporter/file_export.c
>     > @@ -55,6 +55,7 @@ struct fcd {
>     >      GList **filenames; /* pointer to GList to append the filenames
>     > used */
>     >      GtkBuilder *win_xml; /* Glade xml reference */
>     >      Track *track; /* current track to export */
>     > +    gchar *filename; /* filename for the current track to export */
>     >  };
>     >
>      /*------------------------------------------------------------------
>     > @@ -263,10 +264,12 @@ track_get_export_filename(Track *track) {
>     >   */
>     >  static gboolean write_track(struct fcd *fcd) {
>     >      gboolean result = FALSE;
>     > -    gchar *dest_file = track_get_export_filename(fcd->track);
>     > +    /*    gchar *dest_file =
>     track_get_export_filename(fcd->track); */
>     > +    gchar *dest_file = fcd->filename;
>     >      g_return_val_if_fail (fcd, FALSE);
>     >      g_return_val_if_fail (fcd->track, FALSE);
>     > +    g_return_val_if_fail (fcd->filename, FALSE);
>     >      g_return_val_if_fail (fcd->track->itdb, FALSE);
>     >      if (dest_file) {
>     > @@ -303,7 +306,7 @@ static gboolean write_track(struct fcd *fcd) {
>     >              gtkpod_warning(_("Could not find file for '%s' on the
>     > iPod\n"), buf);
>     >              g_free(buf);
>     >          }
>     > -        g_free(dest_file);
>     > + /*  g_free(dest_file); */
>     >      }
>     >      return (result);
>     >  }
>     > @@ -405,6 +408,9 @@ static void export_files_write(struct fcd
>     *fcd) {
>     >              fcd->track = tr;
>     >              copied += tr->size;
>     > +
>     > +    fcd->filename = track_get_export_filename(fcd->track);
>     > +
>     >  #ifdef G_THREADS_ENABLED
>     >              mutex_data = FALSE;
>     >              thread = g_thread_create (th_write_track, fcd,
>     TRUE, NULL);
>     > @@ -434,6 +440,10 @@ static void export_files_write(struct fcd
>     *fcd) {
>     >              while (widgets_blocked && gtk_events_pending ())
>     >              gtk_main_iteration ();
>     >  #endif
>     > +    if (fcd->filename) {
>     > +        g_free(fcd->filename);
>     > +        fcd->filename = 0;
>     > +    }
>     >              if (!resultWrite) {
>     >                  gtkpod_warning(_("Failed to write '%s-%s'\n"),
>     > tr->artist, tr->title);
>     >              }
>     > --
>     > 1.7.3.4
>     >
>     >
>
>     Thanks Chris,
>
>     I am incorporating your patch with a couple of changes of my own, eg.
>     don't really want massive numbers of dialogues displayed for each
>     track
>     that fails to export.
>
>     So should get it tidied up in the next couple of days.
>
>     Cheers
>
>     phantomjinx
>


------------------------------------------------------------------------------
The ultimate all-in-one performance toolkit: Intel(R) Parallel Studio XE:
Pinpoint memory and threading errors before they happen.
Find and fix more than 250 security defects in the development cycle.
Locate bottlenecks in serial and parallel code that limit performance.
http://p.sf.net/sfu/intel-dev2devfeb