Function name free() in class EXIV2API DataBuf

[tester0077]

Ever since I have used the Exiv2 library in my Win based projects, I have had to work around the usage of the function name free() in class EXIV2API DataBuf as defined in types.hpp.
The underlying issue is caused by my usage of the MS recommended memory leakage detection code as outlined at:
https://docs.microsoft.com/en-us/visualstudio/debugger/finding-memory-leaks-using-the-crt-library?view=vs-2019

I have used this technique over many years in all of my apps and it has served me well.

When using the recommended code in any app which want to use the Exiv2 header in order to link to libexiv2, I get warnings and errors such as
8>D:\pkg\C++\MSVC2019\exiv2Git\exiv2\include\exiv2\types.hpp(247,14): warning C4003: not enough arguments for function-like macro invocation 'free'

In order for me to be able to use libexiv2 in my projects, I have found 2 ways to work around the issue:

1. modify the exiv2 source to rename the offending function to free_() in types.hpp
   This of course means I will always have to maintain my own exiv2 code base. This is not only time consuming, but also opens the door to other unintended consequences.
   It also means, I will not be able to use common distribution of Exiv2 code provided by such package managers as vcpkg.

2. rework the include sequence recommended by MS and make sure the line:
   #include "exiv2/exiv2.hpp"
   precedes any of the other memory leakage detection code.
   In some way, this circumvents, possibly defeats, the purpose of the leak detection code as well as forcing me to make using libexiv2 a special case when developing my app.

While 'free' is not a reserved keyword, the MS recommended code for memory leak detection clearly seems to assume as much.

Why no one else seems to have come forward and raised this issue, I have no idea. Possibly much development is done under non-Windows environment, possibly libexiv2 user under Windows don't the MS way of memory leak detection or use some other method.

In any case, I would plead with the developers to implement this small change as part of one of the current development builds and run the usual test of all the related code. If the change causes any errors, I will be quite happy to accept the verdict and keep using my 2nd work around
TIA for you consideration

[clanmills]

As you know, Arnold. I have never seen this issue and nobody has reported anything similar.

We released Exiv2 v0.27.4 RC2 yesterday and I hope that's v0.27.4 finished.

I've marked your issue for v1.00 which we expect to ship on 2021-12-15. I've asked Thomas to comment on your issue as he uses Windows and has his own Visual Studio vcxproj/sln files with which he builds the code.

[clanmills]

@tester0077 I don't think there is a case to answer here. free() is C-Runtime library reference. You can see that exiv2 must link that dynamically when it loads:

508 rmills@rmillsmm-local:~/gnu/github/exiv2/0.27-maintenance/build $ nm --demangle lib/libexiv2.dylib | grep free
0000000000106ec0 T Exiv2::DataBuf::free()
                 U ___cxa_free_exception
                 U _free
509 rmills@rmillsmm-local:~/gnu/github/exiv2/0.27-maintenance/build $ 

Equivalent evidence on Windows (Visual Studio) can be obtained with the utility dumpbin.exe which is on the PATH when you use the "Visual Studio Command Prompt".

---

I have used the Visual Studio Heap Debugging Magic in the past (at work, I don't recall using it with Exiv2). I believe it uses the C Preprocessor to redefine free() as free_dbg().

You can achieve this using CMake:

$ cmake .. -DEXIV2_ENABLE_BMFF=1 -DCMAKE_CXX_FLAGS=-Dfree=my_free
...
--  --- Compiler flags --- 
-- General:           -Dfree=my_free
	 -fcf-protection
	 -fstack-protector-strong
	 -Wp,-D_GLIBCXX_ASSERTIONS
	 -Wall
	 -Wcast-align
	 -Wpointer-arith
	 -Wformat-security
	 -Wmissing-format-attribute
	 -Woverloaded-virtual
	 -W
...
$ make 
...
[ 48%] Linking CXX shared library ../lib/libexiv2.dylib
Undefined symbols for architecture x86_64:
  "_my_free", referenced from:
      Exiv2::MemIo::~MemIo() in basicio.cpp.o
      Exiv2::MemIo::transfer(Exiv2::BasicIo&) in basicio.cpp.o
      Exiv2::RemoteIo::write(Exiv2::BasicIo&) in basicio.cpp.o
      Exiv2::RemoteIo::read(unsigned char*, long) in basicio.cpp.o
      Exiv2::BlockMap::~BlockMap() in basicio.cpp.o
      Exiv2::XmpProperties::unregisterNsUnsafe(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) in properties.cpp.o
      Exiv2::WebPImage::decodeChunks(long) in webpimage.cpp.o
      ...
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [lib/libexiv2.0.27.4.2.dylib] Error 1
make[1]: *** [src/CMakeFiles/exiv2lib.dir/all] Error 2
make: *** [all] Error 2
...
$

As you can see, we were unable to link the exiv2 library because the linker cannot find the function my_free()

---

As you know, Exiv2 recommends using cmake to create Visual Studio solution and project files. Please investigate using -DCMAKE_CXX_FLAGS=-Dfree=free_dbg You might have to also add something concerning malloc_dbg.

I haven't followed the details of your suggestion to change the order of our include files. Please submit a PR with your proposal. I will review it and ask other Team Members to add their comments.

---

@tbeu As you can see, I have assigned this issue to you. I'm only asking for your comments. I don't want you to dig into this unless you wish to do so.

[tester0077]

@clanmills Thank you for having another looks at this issue.
As you correctly state, " free() is C-Runtime library reference." and IMO, using 'free() as a function name within the class EXIV2API DataBuf hopelessly confuses the simple minded MS C preprocessor when it does its thing to redefine free() as free_dbg()., simply because the code is part of an *.h?? file.

By redefining free() from 'outside', you merely mimic the renaming done by the C Preprocessor and end up actually renaming the real free() C-Runtime function used within Exiv2 or any application and the compiler/linker properly rejects this.

My solution/work around merely renames the actual function as used within libexiv2's class EXIV2API DataBuf from free() to free_() - any alteration of the name will do the trick.

The changes to the exiv2 code necessary as a result of my rename are:

Find all "free_", Entire solution, "*.c;*.cpp;*.cxx;*.cc;*.tli;*.tlh;*.h;*.hh;*.hpp;*.hxx;*.hh;*.inl;*.ipp;*.rc;*.resx;*.idl;*.asm;*.inc"
D:\pkg\C++\MSVC2017\exiv2-master-0.27.1\exiv2\include\exiv2\types.hpp(243):        void free_();
D:\pkg\C++\MSVC2017\exiv2-master-0.27.1\exiv2\src\image.cpp(660):        iccProfile_.free_();
D:\pkg\C++\MSVC2017\exiv2-master-0.27.1\exiv2\src\pngimage.cpp(130):                result.free_();
D:\pkg\C++\MSVC2017\exiv2-master-0.27.1\exiv2\src\pngimage.cpp(137):                result.free_();
D:\pkg\C++\MSVC2017\exiv2-master-0.27.1\exiv2\src\pngimage.cpp(160):                result.free_();
D:\pkg\C++\MSVC2017\exiv2-master-0.27.1\exiv2\src\pngimage.cpp(163):                result.free_();
D:\pkg\C++\MSVC2017\exiv2-master-0.27.1\exiv2\src\types.cpp(206):    void DataBuf::free_()

I hope this clarifies things a bit more, but if questions/concerns remain, please let me know.

[tbeu]

@clanmills Would it be OK for you if we rename free in types.[ch]pp as proposed? I can file a PR, but I have no string opinion here.

[clanmills]

@tbeu and @tester0077

I haven't understood the proposed change. Could somebody write the solution down in code and it will be considered. I have not idea what the following means:

modify the exiv2 source to rename the offending function to free_() in types.hpp
This of course means I will always have to maintain my own exiv2 code base. This is not only time consuming, but also opens the door to other unintended consequences.
It also means, I will not be able to use common distribution of Exiv2 code provided by such package managers as vcpkg.

rework the include sequence recommended by MS and make sure the line:
#include "exiv2/exiv2.hpp"
precedes any of the other memory leakage detection code.
In some way, this circumvents, possibly defeats, the purpose of the leak detection code as well as forcing me to make using libexiv2 a special case when developing my app.

[tester0077]

In listing the places where I had to apply my 'fix' I thought I was providing "a solution in code", showing just what needs to be changed to resolve the issue.
Actually, my preferred fix would be to change the name of the Databuf:: free() to something much more descriptive, such as Databuf::FreeDataBuf
Along the same lines, the same class definition defines a function alloc(), For consistency, it might be desirable to rename it as well along the same lines and for the same reasons.
Even though, to this point, it has not caused any issues.
The underlying problem, as I see it, is that the MS preprocessor replaces the string free( with its own replacement, without considering or respecting any scope resolution modifiers.
If it did, there would be no issue, IMO

[clanmills]

Ah, right. I've now understood. You've probably explained this several times already and I haven't understood what you meant. Right. I understand. Rename the Databuf member functions to avoid names such as "free", "malloc" and "alloc".

Yes, that's simple for v1.00. We cannot change that in a "dot". When this is added to 'main' you can begin using it and let us know if you encounter issues. I think it'll be a couple of weeks before this will get done. I'm working on the project plan for v1.00 at the moment. Then we'll have a team meeting to discuss assignments. Code changes/PRs will begin mid-May.

[tester0077]

As I have clarified the request- I hope - on GitHub, I just want to address the question posed by @clanmills The 2 paragraphs below outline the 2 options I have to work-around the current use of the function free() in the class DataBuf. My original post also include code to clearly indicate where the proposed changes would need to be applied.

[tester0077]

Thank you, Robin. There is no rush at my end. I won't be updating my libraries until these changes are part of the library code because none of the other changes and updates seem to affect my needs.

[hassec]

@tester0077 we just merged #1687 which hopefully fixes this problem for you.

Please feel free to reopen this issue if this or a similar problem persists!