physfs icon indicating copy to clipboard operation
physfs copied to clipboard

Macro leak with unity build

Open Sinamore opened this issue 2 years ago • 4 comments

Unity builds glue together multiple files, so when using PhysFS in a bigger project, macros like

#define malloc(x) Do not use malloc() directly.

(physfs_internal.h:171) suddenly may become defined for an outer scope, effectively disabling malloc usage in this case.

Sinamore avatar May 12 '23 13:05 Sinamore

How about we add a check for PHYSFS_DONT_DEFINE_MALLOC, so a unity build can define that somewhere at the top of the includes?

icculus avatar May 13 '23 13:05 icculus

This is certainly a way to solve this. I wonder, however, why you need this at all, if you can just decline PRs that try to introduce a call to malloc? Are you trying to protect people from shooting their feet off?

Sinamore avatar May 15 '23 17:05 Sinamore

I'm trying to prevent me from shooting my own foot off. :)

I'll move this to the CMake project, actually, and out of the source code, which will solve the problem for both of us.

icculus avatar May 15 '23 21:05 icculus

When configuring with -DCMAKE_UNITY_BUILD=1, I see the following build error:

[3/5] Building C object CMakeFiles/physfs.dir/Unity/unity_2_c.c.o
FAILED: CMakeFiles/physfs.dir/Unity/unity_2_c.c.o 
/usr/lib64/ccache/cc -Dphysfs_EXPORTS -I/home/maarten/projects/physfs/include -g -fPIC -Wall -MD -MT CMakeFiles/physfs.dir/Unity/unity_2_c.c.o -MF CMakeFiles/physfs.dir/Unity/unity_2_c.c.o.d -o CMakeFiles/physfs.dir/Unity/unity_2_c.c.o -c /home/maarten/projects/physfs/cmake-build-debug/CMakeFiles/physfs.dir/Unity/unity_2_c.c
In file included from /home/maarten/projects/physfs/cmake-build-debug/CMakeFiles/physfs.dir/Unity/unity_2_c.c:22:
/home/maarten/projects/physfs/src/physfs_archiver_vdf.c:28:19: error: redefinition of ‘readui32’
   28 | static inline int readui32(PHYSFS_Io *io, PHYSFS_uint32 *val)
      |                   ^~~~~~~~
In file included from /home/maarten/projects/physfs/cmake-build-debug/CMakeFiles/physfs.dir/Unity/unity_2_c.c:13:
/home/maarten/projects/physfs/src/physfs_archiver_zip.c:286:12: note: previous definition of ‘readui32’ with type ‘int(PHYSFS_Io *, PHYSFS_uint32 *)’ {aka ‘int(PHYSFS_Io *, unsigned int *)’}
  286 | static int readui32(PHYSFS_Io *io, PHYSFS_uint32 *val)
      |      

Perhaps these read utility functions can be moved to a common header? Or renamed to something unique?

madebr avatar Feb 22 '24 23:02 madebr