Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Macro leak with unity build #59

Open
Sinamore opened this issue May 12, 2023 · 4 comments
Open

Macro leak with unity build #59

Sinamore opened this issue May 12, 2023 · 4 comments

Comments

@Sinamore
Copy link

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.

@icculus
Copy link
Owner

icculus commented May 13, 2023

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?

@Sinamore
Copy link
Author

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?

@icculus
Copy link
Owner

icculus commented May 15, 2023

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.

@madebr
Copy link
Contributor

madebr commented Feb 22, 2024

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants