Bug #1077

MemIo calls msync but FileIo does not

Added by Thomas Beutlich about 2 years ago. Updated over 1 year ago.

Status:ClosedStart date:11 May 2015
Priority:NormalDue date:
Assignee:Andreas Huggel% Done:

100%

Category:basicio
Target version:0.26

Description

I revisited the commits bound to #1043, esp. r3630. I do no think that msync is necessary for MemIo. According to http://man7.org/linux/man-pages/man2/msync.2.html msync is only useful for memory mapped files which is not the case in MemIo. I would rather see it in FileIo where memory mapped files are used. Thus my questions in #1043 (28) and #1043 (29) are still not answered.

Associated revisions

Revision 3896
Added by Andreas Huggel almost 2 years ago

#1077: Removed msync() calls from MemIo.

History

#1 Updated by Robin Mills almost 2 years ago

  • Status changed from New to Assigned
  • Assignee set to Robin Mills
  • Target version set to 0.26

I'll revisit this.

#2 Updated by Andreas Huggel almost 2 years ago

Stumbled upon this because valgrind complains about msync() being called with uninitialized parameters.
After some reading, I agree with Thomas that msync() is not necessary for MemIo. I'd simply remove it. Also, according to this thread, it is not needed before munmap(), at least on POSIX systems.

Robin, I'm not making any changes since you plan to revisit this. If you're ok with my suggestion, you can assign this issue to me and I'll do it.

==4932== Syscall param msync(start) points to uninitialised byte(s)
==4932==    at 0x5D8BD80: __msync_nocancel (syscall-template.S:81)
==4932==    by 0x429972: Exiv2::MemIo::msync() (basicio.cpp:1297)
==4932==    by 0x42921D: Exiv2::MemIo::~MemIo() (basicio.cpp:1177)
==4932==    by 0x42929F: Exiv2::MemIo::~MemIo() (basicio.cpp:1181)
==4932==    by 0x42E1D8: std::auto_ptr<Exiv2::BasicIo>::~auto_ptr() (in /usr/local/bin/exiv2)
==4932==    by 0x4664D3: Exiv2::JpegBase::writeMetadata() (jpgimage.cpp:664)
==4932==    by 0x41CCFD: Action::Modify::run(std::string const&) (actions.cpp:1259)
==4932==    by 0x406D69: main (exiv2.cpp:171)
==4932==  Address 0x6093212 is 34 bytes inside a block of size 40 alloc'd
==4932==    at 0x4C2C7A7: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4932==    by 0x42913B: Exiv2::MemIo::MemIo() (basicio.cpp:1165)
==4932==    by 0x427190: Exiv2::FileIo::temporary() const (basicio.cpp:614)
==4932==    by 0x466404: Exiv2::JpegBase::writeMetadata() (jpgimage.cpp:659)
==4932==    by 0x41CCFD: Action::Modify::run(std::string const&) (actions.cpp:1259)
==4932==    by 0x406D69: main (exiv2.cpp:171)

#3 Updated by Robin Mills almost 2 years ago

  • Assignee changed from Robin Mills to Andreas Huggel

I'm more than happy to pass this over to you, Andreas. Thank You.

#4 Updated by Andreas Huggel almost 2 years ago

  • Status changed from Assigned to Resolved
  • % Done changed from 0 to 100

Removed msync() calls from MemIo. Valgrind is happy again. Not planning to make changes to FileIo.

#5 Updated by Thomas Beutlich over 1 year ago

Thanks for considering and fixing this.

#6 Updated by Robin Mills over 1 year ago

  • Status changed from Resolved to Closed

I'm going to set the status of this to closed. It has been 100% resolved for some time without further incident.

Also available in: Atom PDF

Redmine Appliance - Powered by TurnKey Linux