Bug 626199 - F12:gthumb core dumps at comments.c:749 when reading comment files from F13:gthumb
F12:gthumb core dumps at comments.c:749 when reading comment files from F13:g...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: gthumb (Show other bugs)
12
All Linux
low Severity medium
: ---
: ---
Assigned To: Christian Krause
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2010-08-22 13:26 EDT by Tim Taiwanese Liim
Modified: 2010-09-02 13:03 EDT (History)
1 user (show)

See Also:
Fixed In Version: gthumb-2.10.12-1.fc12
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-08-31 23:30:07 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Proposed patch, version 1, so F12:gthumb can use comment files from F13. (1.62 KB, patch)
2010-08-22 13:29 EDT, Tim Taiwanese Liim
no flags Details | Diff

  None (edit)
Description Tim Taiwanese Liim 2010-08-22 13:26:53 EDT
Description of problem:
    F13:gthumb uses different labels in comment files, which F12:gthumb
    does not expect and core dumps.  F12:gthumb should ignore labels that
    it does not understand.  Here is the difference between
    F12/F13:gthumb comment files:
       <Comment format="2.0">      # Comment file from F12:gthumb
       <comment version="3.0">     # Comment file from F13:gthumb
    In libgthumb/comments.c:748, we have
            format = xmlGetProp (root, FORMAT_TAG);  // FORMAT_TAG = "format"
            if (strcmp ((char*)format, "1.0") == 0)  <== core dump
    The string format would be NULL when the comment has anything
    other than "format" (eg. "version" from F13), thus core dump.

    gthumb should be protective against unknown labels, eg.
            if (format == NULL) {
                    xmlFreeDoc (doc);
                    g_free (data);
                    return NULL;
            }
            if (strcmp ((char*)format, "1.0") == 0)


    You might ask how realistic this scenario is.  I have several F12
    systems; I upgraded one of them to F13 first, for evaluation.
    After trying out F13:gthumb, I decided not to upgrade the rest of
    my systems to F13.  Yes, F13:gthumb is the show stopper for me to
    upgrade to F13.

    I understand that F13:gthumb is a complete rewrite, which is fine
    and good and noble endeavor in itself.  But it is not ready for
    prime time yet.  For example,
      - in slide show mode I can no longer show the comments.
      - in a folder with >100 photos it takes long time (much
        slower than F12:ghumb) to show the thumbnails.
      - the shortcut keys are missing (eg. B/N for back/next).  I
        have to use Backspace key to go back, which is next to Delete
        key on my notebook, good chance for error.
      - very slow to update photo.  eg. In thumbnail I moved from
        photo p1 to photo p2, then go full-screen mode, which still
        shows p1 (I expect p2).  It usually takes >10 seconds before
        p2 shows up.  F12:gthumb has same issue, but not as slow as
        F13:gthumb.

    I really think F13 should stick with gthumb-2.10.11 intead of
    moving to gthumb-2.11.5; at least wait till 2.11.x is more
    usable.

Version-Release number of selected component (if applicable):
    gthumb-2.10.11-9.fc12.x86_64

How reproducible:
    always

Steps to Reproduce:
    1. in an F13 system, use gthumb to write some comments and 
       save it.
    2. open the same folder in F12:gthumb (eg. after copying the
       same file to an F12 system, or sharing over NFS).
  
Actual results:
    F12:gthumb core dumps.

Expected results:
    F12:gthumb should be able to read the comments (at least the
    Note/Place/Time fields) from F13:gthumb.  At least F12:gthumb
    should not core dump.

Additional info:
    N/A
Comment 1 Tim Taiwanese Liim 2010-08-22 13:29:47 EDT
Created attachment 440234 [details]
Proposed patch, version 1, so F12:gthumb can use comment files from F13.

The fix in Comment #0 causes F12:gthumb to ignore comment files
from F13:gthumb.  But why should it?  Many of the comment fields
are similar, eg. place, note, time; but we have
  <Note>this is a test msg.</Note>      # F12
  <note>this is a test msg.</note>      # F13
The only difference is in "Note" and "note".  So F12:gthumb has
no good reason to drop Note/Place field from F13.
Comment 2 Christian Krause 2010-08-22 14:46:21 EDT
An update of gthumb to 2.10.12 in F12 is on its way. It will fix the issue that gthumb will crash when opening a file with comments generated by version 2.11.x.

Please note, that it will not make help to actually read the new comments generated by 2.11.x. If you really need this feature, please file a bug report upstream ( https://bugzilla.gnome.org/enter_bug.cgi?product=gthumb ).
Comment 3 Fedora Update System 2010-08-22 14:56:09 EDT
gthumb-2.10.12-1.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/gthumb-2.10.12-1.fc12
Comment 4 Tim Taiwanese Liim 2010-08-23 10:46:53 EDT
Christian,
Thanks for fast action!  That's good, the core dump fixed.  I'll file
a bug upstream for reading comments from 2.11.x.
Comment 5 Tim Taiwanese Liim 2010-08-23 12:25:16 EDT
Just checked the source code; the fix is good against core dump:
	if (format == NULL)
		return NULL;
But now we have memory leak on 'doc' and 'data'.  How about this?
            if (format == NULL) {
                    xmlFreeDoc (doc);
                    g_free (data);
                    return NULL;
            }
Thanks.
Comment 6 Fedora Update System 2010-08-23 18:06:47 EDT
gthumb-2.10.12-1.fc12 has been pushed to the Fedora 12 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update gthumb'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/gthumb-2.10.12-1.fc12
Comment 7 Fedora Update System 2010-08-31 23:30:02 EDT
gthumb-2.10.12-1.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 8 Tim Taiwanese Liim 2010-09-02 13:03:23 EDT
For the record: I filed two bugs upstream for Comment #5 (628587) and
Comment #1 (628626).

628587 memory leak at libgthumb/comments.c:750
628626 RFE: gthumb-2.10.x be able to read comments from 2.11.x

https://bugzilla.gnome.org/show_bug.cgi?id=628587
https://bugzilla.gnome.org/show_bug.cgi?id=628626

Also I want to confirm that gthumb-2.10.12-1.fc12 solved this core
dump (bug 626199).  
Christian, thanks!

Note You need to log in before you can comment on or make changes to this bug.