Project

General

Profile

Bug #1077

MemIo calls msync but FileIo does not

Added by Thomas Beutlich over 6 years ago. Updated about 6 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Andreas Huggel
Category:
basicio
Target version:
0.26
Start date:
11 May 2015
Due date:
% Done:

100%

Estimated time:

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 (28) and #1043-29 (29) are still not answered.

Associated revisions

Revision 3896 (diff)
Added by Andreas Huggel about 6 years ago

#1077: Removed msync() calls from MemIo.

History

#1

Updated by Robin Mills about 6 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 about 6 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 about 6 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 about 6 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 about 6 years ago

Thanks for considering and fixing this.

#6

Updated by Robin Mills about 6 years 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