Re: [Gtkpod-devel] Segmentation fault when exporting files to the filesystem

[Chris J. <cdp...@gm...>]

It works great - thanks :-). I just compiled the latest code from git and it
no longer crashes when doing the export.

On 15 February 2011 18:02, phantomjinx <p.g...@ph...>wrote:

> 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 <p.g...@ph...<mailto:
>> p.g...@ph...>> 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 <cdp...@gm...
>>    <mailto:cdp...@gm...> <mailto:cdp...@gm...
>>
>>    <mailto:cdp...@gm...>>>
>>    > 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
>>
>>
>