Bug 433182

I've done a full review, result below.

<humor>
Unfortunately I cannot approve this package as every package should have atleast
one patch. Luckily I found a 64 bit bug, causing the package to fail to work
(crash) on 64 bit. So I've written a patch fixing this so that we can meet the
every package should have atleast one patch rule.
</humor>

MUST FIX
--------
* Apply the to be attached patch fixing crashing on 64 bit. The patch may look 
  strange for a 64 bit patch, but it really fixes a 64 bit issue:
  -The header in question contains guards to protect it from multiple inclusion
  -These guards where copy pasted from another header without changing the
   define checked and set, therefor this header things its already included
   while its not
  -This causes various prototypes to not be declared, and the compiler to assume
   their return type is int (aka 32 bits)
  -However some return a pointer, which should be 64 bit and now gets truncated 
   to 32 bits.
  -Boom (aka crash)

Should Fix
----------
* Send the 64 bit patch upstream
* Ask upstream to use freedesktop.org icon installation location instead of
  obsolete /usr/share/pixmaps

Although there is a MUST FIX, I trust you to include the patch so I'm going to
set the approved flag, please include the patch before building the first time.

Like I said I'm going to set the approved flag, second try :)

Created attachment 295109 [details]
PATCH fixing 64 bit crash

Thank you for the patch! The upstream developer replied that he
applied the fix on svn.
Also upstream asked me:
--------------------------------------------------------------------------
Some debian users reported that mounting removable devices (usb disk, cdrom)
in pcmanfm 0.3.5.23 from the side pane (with --enable-hal turned on) will
cause crashes (seg. faults).  However, we cannot reproduce the bug on ubuntu
by any means, and from the gdb backtrace, the crash happened in libhal.
We have no idea how could this happens. Would you please get this
tested on Fedora, and I wish that good programmer can help reviewing
this part. It's mainly in src/vfs/vfs-volume-hal.c, and most of the source code
was taken from gnome-mount and exo-mount of xfce.
This is the main blocker of 0.3.6 release. We'll make this stable release
after this potential bug get resolved.
------------------------------------------------------------------------------
This bug seems reproducible to me although for now I am not sure
if this bug is caused really by hal side. For now I disabled to
mount removal devices by pcmanfm by a patch.

http://mtasaka.fedorapeople.org/Review_request/pcmanfm/pcmanfm.spec
http://mtasaka.fedorapeople.org/Review_request/pcmanfm/pcmanfm-0.3.5.23-2.fc8.src.rpm
* Mon Feb 18 2008 Mamoru Tasaka <mtasaka.u-tokyo.ac.jp> - 0.3.5.23-2
- Apply patch to fix crash on 64bits arch as suggested by Hans
  (bug 433182)
- Disable to mount removable devices for now

---------------------------------------------------------------------------
New Package CVS Request
=======================
Package Name:      pcmanfm
Short Description: Extremly fast and lightweight file manager
Owners:            mtasaka
Branches:          F-8 F-7
InitialCC: 
Cvsextras Commits: yes

cvs done.

Successfully built on all branches, requested on bodhi, closing.

Thank you for reviewing this package and for CVS procedure!